mirror of
https://github.com/ModernRelay/omnigraph.git
synced 2026-06-18 02:24:27 +02:00
feat(cli): no-default-graph errors list candidate graphs (RFC-011 D7) (#245)
When a server/cluster scope resolves with no --graph and no default_graph, the CLI auto-uses a sole graph (cluster) or errors listing the candidate graph ids (cluster catalog; multi-graph server via best-effort GET /graphs), never a silent pick. GraphClient::resolve becomes async; flat/single-graph servers and happy paths are unaffected.
This commit is contained in:
parent
b395757e21
commit
1bc0ea6b51
10 changed files with 262 additions and 62 deletions
|
|
@ -42,7 +42,7 @@ use crate::helpers::{
|
|||
ResolvedCliGraph, apply_bearer_token, apply_server_flag, build_http_client, is_remote_uri,
|
||||
legacy_change_request_body, open_local_db_with_policy, query_params_from_json,
|
||||
remote_json, remote_url, resolve_cli_actor, resolve_cli_graph, resolve_remote_bearer_token,
|
||||
select_named_query,
|
||||
resolve_server_flag, select_named_query,
|
||||
};
|
||||
use crate::output::{LoadOutput, load_output_from_result, load_output_from_tables};
|
||||
use omnigraph_server::config::OmnigraphConfig;
|
||||
|
|
@ -66,6 +66,44 @@ pub(crate) enum GraphClient {
|
|||
},
|
||||
}
|
||||
|
||||
/// RFC-011 Decision 7: a server scope that selects no graph (no `--graph`, no
|
||||
/// `default_graph`) must not silently fall through to the bare server URL when
|
||||
/// the server is multi-graph. Best-effort probe `GET /graphs`: a populated list
|
||||
/// forces `--graph` (listing the candidates); a single-graph/flat server (405),
|
||||
/// a policy-gated `/graphs`, or an unreachable server all proceed — the bare URL
|
||||
/// is then correct, or the real request surfaces the failure. Only fires on the
|
||||
/// no-graph path, so a `--graph`/`default_graph` happy path does no extra I/O.
|
||||
async fn require_graph_for_multi_graph_server(
|
||||
config: &OmnigraphConfig,
|
||||
scope: &crate::scope::ResolvedScope,
|
||||
) -> Result<()> {
|
||||
let (Some(server), None) = (scope.server.as_deref(), scope.graph.as_deref()) else {
|
||||
return Ok(());
|
||||
};
|
||||
let Some(base) = resolve_server_flag(Some(server), None)? else {
|
||||
return Ok(());
|
||||
};
|
||||
let token = resolve_remote_bearer_token(config, Some(&base))?;
|
||||
let probe = GraphClient::Remote {
|
||||
http: build_http_client()?,
|
||||
base_url: base,
|
||||
token,
|
||||
};
|
||||
if let Ok(resp) = probe.list_graphs().await {
|
||||
if !resp.graphs.is_empty() {
|
||||
let ids: Vec<&str> = resp.graphs.iter().map(|g| g.graph_id.as_str()).collect();
|
||||
bail!(
|
||||
"server scope '{server}' has {} {}: [{}]; pass --graph <id> to select one \
|
||||
(or set `default_graph` in your operator config)",
|
||||
ids.len(),
|
||||
if ids.len() == 1 { "graph" } else { "graphs" },
|
||||
ids.join(", ")
|
||||
);
|
||||
}
|
||||
}
|
||||
Ok(())
|
||||
}
|
||||
|
||||
/// A remote graph must be addressed with `--server` (RFC-011): a positional or
|
||||
/// `--uri` `http(s)://` URL no longer auto-dispatches to a server. A remote URL
|
||||
/// produced by a server scope (`via_server`) is fine.
|
||||
|
|
@ -86,7 +124,7 @@ impl GraphClient {
|
|||
/// fork. Mirrors the read verbs' current preamble (`resolve_uri`
|
||||
/// path, not the policy-bearing `resolve_cli_graph`). Used by reads
|
||||
/// and `query` (which opens without policy, like the reads).
|
||||
pub(crate) fn resolve(
|
||||
pub(crate) async fn resolve(
|
||||
config: &OmnigraphConfig,
|
||||
server: Option<&str>,
|
||||
graph: Option<&str>,
|
||||
|
|
@ -102,6 +140,7 @@ impl GraphClient {
|
|||
crate::planes::Capability::Any,
|
||||
crate::scope::ScopeFlags { profile, store, server, cluster: None, graph, uri },
|
||||
)?;
|
||||
require_graph_for_multi_graph_server(config, &scope).await?;
|
||||
let (server, graph, uri) = (
|
||||
scope.server.as_deref(),
|
||||
scope.graph.as_deref(),
|
||||
|
|
@ -133,7 +172,7 @@ impl GraphClient {
|
|||
/// resolved up front. The embedded arm then opens WITH policy. The
|
||||
/// resolution order matches the write arms exactly: server flag →
|
||||
/// bearer token → graph.
|
||||
pub(crate) fn resolve_with_policy(
|
||||
pub(crate) async fn resolve_with_policy(
|
||||
config: &OmnigraphConfig,
|
||||
server: Option<&str>,
|
||||
graph: Option<&str>,
|
||||
|
|
@ -149,6 +188,7 @@ impl GraphClient {
|
|||
crate::planes::Capability::Any,
|
||||
crate::scope::ScopeFlags { profile, store, server, cluster: None, graph, uri },
|
||||
)?;
|
||||
require_graph_for_multi_graph_server(config, &scope).await?;
|
||||
let (server, graph, uri) = (
|
||||
scope.server.as_deref(),
|
||||
scope.graph.as_deref(),
|
||||
|
|
|
|||
|
|
@ -632,11 +632,12 @@ pub(crate) async fn resolve_maintenance_uri(
|
|||
}
|
||||
|
||||
/// Map a resolved direct address to a storage URI: a cluster scope
|
||||
/// (`--cluster <root> --graph <id>`, or a `--profile` cluster binding)
|
||||
/// resolves the graph's storage URI from the **served cluster state** (the
|
||||
/// truth a `--cluster` server serves); otherwise the ordinary positional-URI
|
||||
/// path. The scope resolver guarantees a cluster scope always carries a graph,
|
||||
/// so the mismatched arm is defensive.
|
||||
/// (`--cluster <root> --graph <id>`, or a `--profile` cluster binding) resolves
|
||||
/// the graph's storage URI from the **served cluster state**; otherwise the
|
||||
/// ordinary positional-URI path. When a cluster scope carries no graph
|
||||
/// selection (RFC-011 D7), enumerate the catalog: a sole graph is used
|
||||
/// automatically, otherwise error and list the candidates so the operator can
|
||||
/// pass `--graph <id>`.
|
||||
pub(crate) async fn resolve_storage_uri(
|
||||
config: &OmnigraphConfig,
|
||||
cli_uri: Option<String>,
|
||||
|
|
@ -646,8 +647,32 @@ pub(crate) async fn resolve_storage_uri(
|
|||
) -> Result<String> {
|
||||
match (cluster, cluster_graph) {
|
||||
(Some(cluster), Some(graph_id)) => resolve_cluster_graph_uri(cluster, graph_id).await,
|
||||
(Some(cluster), None) => {
|
||||
let graph_id = resolve_sole_cluster_graph(cluster).await?;
|
||||
resolve_cluster_graph_uri(cluster, &graph_id).await
|
||||
}
|
||||
(None, None) => resolve_local_uri(config, cli_uri, operation),
|
||||
_ => bail!("internal error: a cluster scope was resolved without a graph id"),
|
||||
(None, Some(_)) => {
|
||||
bail!("internal error: a graph was selected without a cluster scope")
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/// Pick the graph for a cluster scope that has no `--graph`/`default_graph`
|
||||
/// (RFC-011 D7): exactly one applied graph → use it; zero → error; more than
|
||||
/// one → error and list the candidates. Never auto-picks among several.
|
||||
async fn resolve_sole_cluster_graph(cluster: &str) -> Result<String> {
|
||||
let ids = omnigraph_cluster::cluster_graph_ids(cluster)
|
||||
.await
|
||||
.map_err(|diagnostic| color_eyre::eyre::eyre!("{}", diagnostic.message))?;
|
||||
match ids.as_slice() {
|
||||
[only] => Ok(only.clone()),
|
||||
[] => bail!("cluster `{cluster}` has no applied graphs; run `cluster apply` first"),
|
||||
many => bail!(
|
||||
"cluster `{cluster}` has {} graphs: [{}]; pass --graph <id> to select one",
|
||||
many.len(),
|
||||
many.join(", ")
|
||||
),
|
||||
}
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -186,7 +186,8 @@ async fn main() -> Result<()> {
|
|||
cli.as_actor.as_deref(),
|
||||
cli.profile.as_deref(),
|
||||
cli.store.as_deref(),
|
||||
)?;
|
||||
)
|
||||
.await?;
|
||||
let branch = resolve_branch(&config, branch, None, "main");
|
||||
if matches!(mode, CliLoadMode::Overwrite) {
|
||||
confirm_destructive("load --mode overwrite", client.uri(), cli.yes, json)?;
|
||||
|
|
@ -224,7 +225,8 @@ async fn main() -> Result<()> {
|
|||
cli.as_actor.as_deref(),
|
||||
cli.profile.as_deref(),
|
||||
cli.store.as_deref(),
|
||||
)?;
|
||||
)
|
||||
.await?;
|
||||
let branch = resolve_branch(&config, branch, None, "main");
|
||||
let from = resolve_branch(&config, from, None, "main");
|
||||
echo_write_target(cli.quiet, "ingest", client.uri(), client.is_remote());
|
||||
|
|
@ -254,7 +256,8 @@ async fn main() -> Result<()> {
|
|||
cli.as_actor.as_deref(),
|
||||
cli.profile.as_deref(),
|
||||
cli.store.as_deref(),
|
||||
)?;
|
||||
)
|
||||
.await?;
|
||||
let from = resolve_branch(&config, from, None, "main");
|
||||
echo_write_target(cli.quiet, "branch create", client.uri(), client.is_remote());
|
||||
let payload = client.branch_create_from(&from, &name).await?;
|
||||
|
|
@ -277,7 +280,8 @@ async fn main() -> Result<()> {
|
|||
uri,
|
||||
cli.profile.as_deref(),
|
||||
cli.store.as_deref(),
|
||||
)?;
|
||||
)
|
||||
.await?;
|
||||
let payload = client.branch_list().await?;
|
||||
if json {
|
||||
print_json(&payload)?;
|
||||
|
|
@ -302,7 +306,8 @@ async fn main() -> Result<()> {
|
|||
cli.as_actor.as_deref(),
|
||||
cli.profile.as_deref(),
|
||||
cli.store.as_deref(),
|
||||
)?;
|
||||
)
|
||||
.await?;
|
||||
confirm_destructive("branch delete", client.uri(), cli.yes, json)?;
|
||||
echo_write_target(cli.quiet, "branch delete", client.uri(), client.is_remote());
|
||||
let payload = client.branch_delete(&name).await?;
|
||||
|
|
@ -328,7 +333,8 @@ async fn main() -> Result<()> {
|
|||
cli.as_actor.as_deref(),
|
||||
cli.profile.as_deref(),
|
||||
cli.store.as_deref(),
|
||||
)?;
|
||||
)
|
||||
.await?;
|
||||
let into = resolve_branch(&config, into, None, "main");
|
||||
echo_write_target(cli.quiet, "branch merge", client.uri(), client.is_remote());
|
||||
let payload = client.branch_merge(&source, &into).await?;
|
||||
|
|
@ -359,7 +365,8 @@ async fn main() -> Result<()> {
|
|||
uri,
|
||||
cli.profile.as_deref(),
|
||||
cli.store.as_deref(),
|
||||
)?;
|
||||
)
|
||||
.await?;
|
||||
let payload = client.list_commits(branch.as_deref()).await?;
|
||||
if json {
|
||||
print_json(&payload)?;
|
||||
|
|
@ -381,7 +388,8 @@ async fn main() -> Result<()> {
|
|||
uri,
|
||||
cli.profile.as_deref(),
|
||||
cli.store.as_deref(),
|
||||
)?;
|
||||
)
|
||||
.await?;
|
||||
let commit = client.get_commit(&commit_id).await?;
|
||||
if json {
|
||||
print_json(&commit)?;
|
||||
|
|
@ -436,7 +444,8 @@ async fn main() -> Result<()> {
|
|||
cli.as_actor.as_deref(),
|
||||
cli.profile.as_deref(),
|
||||
cli.store.as_deref(),
|
||||
)?;
|
||||
)
|
||||
.await?;
|
||||
let schema_source = fs::read_to_string(&schema)?;
|
||||
// The stored-query registry check is an embedded-only concern
|
||||
// (the remote arm ignores the validator — the server runs its
|
||||
|
|
@ -477,7 +486,8 @@ async fn main() -> Result<()> {
|
|||
uri,
|
||||
cli.profile.as_deref(),
|
||||
cli.store.as_deref(),
|
||||
)?;
|
||||
)
|
||||
.await?;
|
||||
let output = client.schema_source().await?;
|
||||
if json {
|
||||
print_json(&output)?;
|
||||
|
|
@ -528,7 +538,8 @@ async fn main() -> Result<()> {
|
|||
uri,
|
||||
cli.profile.as_deref(),
|
||||
cli.store.as_deref(),
|
||||
)?;
|
||||
)
|
||||
.await?;
|
||||
let branch = resolve_branch(&config, branch, None, "main");
|
||||
let payload = client.snapshot(&branch).await?;
|
||||
if json {
|
||||
|
|
@ -553,7 +564,8 @@ async fn main() -> Result<()> {
|
|||
uri,
|
||||
cli.profile.as_deref(),
|
||||
cli.store.as_deref(),
|
||||
)?;
|
||||
)
|
||||
.await?;
|
||||
let branch = resolve_branch(&config, branch, None, "main");
|
||||
if jsonl {
|
||||
eprintln!("warning: --jsonl is deprecated; `omnigraph export` always emits JSONL");
|
||||
|
|
@ -590,7 +602,8 @@ async fn main() -> Result<()> {
|
|||
uri.or(legacy_uri),
|
||||
cli.profile.as_deref(),
|
||||
cli.store.as_deref(),
|
||||
)?;
|
||||
)
|
||||
.await?;
|
||||
let query_source =
|
||||
resolve_query_source(&config, query.as_ref(), query_string.as_deref(), None)?;
|
||||
let params_json = load_params_json(¶ms)?;
|
||||
|
|
@ -625,7 +638,8 @@ async fn main() -> Result<()> {
|
|||
cli.as_actor.as_deref(),
|
||||
cli.profile.as_deref(),
|
||||
cli.store.as_deref(),
|
||||
)?;
|
||||
)
|
||||
.await?;
|
||||
let query_source =
|
||||
resolve_query_source(&config, query.as_ref(), query_string.as_deref(), None)?;
|
||||
let params_json = load_params_json(¶ms)?;
|
||||
|
|
@ -1005,7 +1019,8 @@ async fn main() -> Result<()> {
|
|||
uri,
|
||||
cli.profile.as_deref(),
|
||||
cli.store.as_deref(),
|
||||
)?;
|
||||
)
|
||||
.await?;
|
||||
let payload = client.list_graphs().await?;
|
||||
if json {
|
||||
print_json(&payload)?;
|
||||
|
|
|
|||
|
|
@ -189,15 +189,13 @@ fn scope_from_binding(
|
|||
.map(str::to_string)
|
||||
.unwrap_or(cluster);
|
||||
// A cluster holds many graphs; maintenance addresses one at a time.
|
||||
let Some(graph) = graph else {
|
||||
bail!(
|
||||
"{source} resolves a cluster scope; pass --graph <id> to select which \
|
||||
graph to maintain"
|
||||
);
|
||||
};
|
||||
// When no `--graph`/`default_graph` is given, leave `cluster_graph`
|
||||
// empty and defer to the async storage-URI resolver (RFC-011 D7),
|
||||
// which enumerates the catalog: auto-use a sole graph, else error
|
||||
// and list the candidates.
|
||||
Ok(ResolvedScope {
|
||||
cluster: Some(root),
|
||||
cluster_graph: Some(graph),
|
||||
cluster_graph: graph,
|
||||
..Default::default()
|
||||
})
|
||||
}
|
||||
|
|
@ -334,9 +332,13 @@ mod tests {
|
|||
}
|
||||
|
||||
#[test]
|
||||
fn cluster_scope_without_a_graph_is_a_loud_error() {
|
||||
fn cluster_scope_without_a_graph_defers_to_catalog_enumeration() {
|
||||
// RFC-011 D7: with no `--graph`/`default_graph`, resolution no longer
|
||||
// bails here — it resolves the cluster root and leaves `cluster_graph`
|
||||
// empty, deferring to the async storage-URI resolver (which enumerates
|
||||
// the catalog: auto-use a sole graph, else error listing candidates).
|
||||
let op = cfg("clusters:\n brain:\n root: s3://acme/brain\n");
|
||||
let err = resolve_scope(
|
||||
let scope = resolve_scope(
|
||||
&op,
|
||||
Capability::Direct,
|
||||
ScopeFlags {
|
||||
|
|
@ -344,9 +346,9 @@ mod tests {
|
|||
..flags()
|
||||
},
|
||||
)
|
||||
.unwrap_err()
|
||||
.to_string();
|
||||
assert!(err.contains("--graph <id>"), "{err}");
|
||||
.unwrap();
|
||||
assert_eq!(scope.cluster.as_deref(), Some("s3://acme/brain"));
|
||||
assert_eq!(scope.cluster_graph, None);
|
||||
}
|
||||
|
||||
#[test]
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue