From 25d74d689d0801962f8897d9d05d4303974fc932 Mon Sep 17 00:00:00 2001 From: aaltshuler Date: Sat, 13 Jun 2026 17:44:23 +0300 Subject: [PATCH] refactor(cli): GraphClient enum + read verbs (RFC-009 Phase 3a) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The embedded-vs-remote split gets one home: a GraphClient enum (Embedded { uri } | Remote { http, base_url, token }) with a resolve() factory that absorbs the shared preamble (apply_server_flag -> token -> URI/remoteness) and a verb method per command. The five uniform read forks — branch list, commit list, commit show, schema show, snapshot — collapse from per-command if-graph-is-remote else to one line each (main.rs: -113/+47). Behavior identical per verb (local reads still open WITHOUT policy, as today); the Phase-1 parity matrix is the referee and passes textually unchanged. Enum, not the RFC trait: only two variants ever, and inherent async methods avoid async_trait boxing and the apply_schema closure that is not object-safe (3b) — same one-body-two-impls collapse, less ceremony. Scope: the uniform reads only. The query verb (policy-open + operator- alias early-return + param merge) joins the write verbs in 3b; export/streaming and graphs-list in 3c, where the now-shared execute_*_remote/execute_* pairs get retired. Co-Authored-By: Claude Fable 5 --- crates/omnigraph-cli/src/client.rs | 197 +++++++++++++++++++++++++++++ crates/omnigraph-cli/src/main.rs | 160 +++++++---------------- 2 files changed, 244 insertions(+), 113 deletions(-) create mode 100644 crates/omnigraph-cli/src/client.rs diff --git a/crates/omnigraph-cli/src/client.rs b/crates/omnigraph-cli/src/client.rs new file mode 100644 index 0000000..feeaf16 --- /dev/null +++ b/crates/omnigraph-cli/src/client.rs @@ -0,0 +1,197 @@ +//! `GraphClient` — the one place the embedded-vs-remote split lives +//! (RFC-009 Phase 3). A CLI command body calls a verb method; the +//! enum routes to the engine (local URI) or HTTP (remote URI). The +//! 15 per-command `if graph.is_remote { … } else { … }` forks collapse +//! into two arms here. +//! +//! Phase 3a scope: the factory + the uniform read verbs (snapshot, +//! schema show, branch list, commit list/show — all of which open the +//! local engine WITHOUT policy today, preserved exactly). Write verbs +//! and the policy-bearing `query`/`mutate` arrive in 3b (the Embedded +//! variant will grow the policy context then); export + graphs-list in +//! 3c. Behavior is unchanged per verb — the Phase-1 parity matrix is the +//! referee and stays textually unchanged. +//! +//! Enum, not a trait (RFC sketch said "trait"): only two variants ever, +//! and inherent async methods sidestep `async_trait` boxing plus the +//! `apply_schema` catalog-validator closure that is not object-safe. +//! Same one-body-two-impls collapse, less ceremony. + +use reqwest::Method; +use color_eyre::Result; +use omnigraph::db::{Omnigraph, ReadTarget}; +use omnigraph_api_types::{ + BranchListOutput, CommitListOutput, CommitOutput, SchemaOutput, SnapshotOutput, commit_output, + snapshot_payload, +}; + +use crate::helpers::{ + apply_server_flag, build_http_client, is_remote_uri, remote_json, remote_url, + resolve_remote_bearer_token, +}; +use omnigraph_server::config::OmnigraphConfig; + +pub(crate) enum GraphClient { + /// Local engine at `uri`. Reads open the dataset per call (no policy + /// attached — matches today's read behavior; the write verbs in 3b + /// add a policy-bearing context). + Embedded { uri: String }, + /// Remote HTTP server. The actor is resolved server-side from the + /// token; the client never sets identity. + Remote { + http: reqwest::Client, + base_url: String, + token: Option, + }, +} + +impl GraphClient { + /// Resolve the addressing (positional URI / `--target` / `--server`) + /// and credential once, then pick the variant by URI scheme — the + /// single branch point that replaces every per-command `is_remote` + /// fork. Mirrors the read verbs' current preamble (`resolve_uri` + /// path, not the policy-bearing `resolve_cli_graph`). + pub(crate) fn resolve( + config: &OmnigraphConfig, + server: Option<&str>, + graph: Option<&str>, + uri: Option, + target: Option<&str>, + ) -> Result { + let uri = apply_server_flag(server, graph, uri, target)?; + let token = resolve_remote_bearer_token(config, uri.as_deref(), target)?; + let uri = crate::helpers::resolve_uri(config, uri, target)?; + if is_remote_uri(&uri) { + Ok(GraphClient::Remote { + http: build_http_client()?, + base_url: uri, + token, + }) + } else { + Ok(GraphClient::Embedded { uri }) + } + } + + pub(crate) async fn branch_list(&self) -> Result { + match self { + GraphClient::Remote { + http, + base_url, + token, + } => { + remote_json( + http, + Method::GET, + remote_url(base_url, "/branches"), + None, + token.as_deref(), + ) + .await + } + GraphClient::Embedded { uri } => { + let db = Omnigraph::open(uri).await?; + let mut branches = db.branch_list().await?; + branches.sort(); + Ok(BranchListOutput { branches }) + } + } + } + + pub(crate) async fn snapshot(&self, branch: &str) -> Result { + match self { + GraphClient::Remote { + http, + base_url, + token, + } => { + remote_json( + http, + Method::GET, + format!("{}?branch={}", remote_url(base_url, "/snapshot"), branch), + None, + token.as_deref(), + ) + .await + } + GraphClient::Embedded { uri } => { + let db = Omnigraph::open(uri).await?; + let snapshot = db.snapshot_of(ReadTarget::branch(branch)).await?; + Ok(snapshot_payload(branch, &snapshot)) + } + } + } + + pub(crate) async fn schema_source(&self) -> Result { + match self { + GraphClient::Remote { + http, + base_url, + token, + } => { + remote_json( + http, + Method::GET, + remote_url(base_url, "/schema"), + None, + token.as_deref(), + ) + .await + } + GraphClient::Embedded { uri } => { + let db = Omnigraph::open(uri).await?; + Ok(SchemaOutput { + schema_source: db.schema_source().to_string(), + }) + } + } + } + + pub(crate) async fn list_commits(&self, branch: Option<&str>) -> Result { + match self { + GraphClient::Remote { + http, + base_url, + token, + } => { + let url = match branch { + Some(branch) => format!("{}?branch={}", remote_url(base_url, "/commits"), branch), + None => remote_url(base_url, "/commits"), + }; + remote_json(http, Method::GET, url, None, token.as_deref()).await + } + GraphClient::Embedded { uri } => { + let db = Omnigraph::open(uri).await?; + let commits = db + .list_commits(branch) + .await? + .iter() + .map(commit_output) + .collect::>(); + Ok(CommitListOutput { commits }) + } + } + } + + pub(crate) async fn get_commit(&self, commit_id: &str) -> Result { + match self { + GraphClient::Remote { + http, + base_url, + token, + } => { + remote_json( + http, + Method::GET, + remote_url(base_url, &format!("/commits/{commit_id}")), + None, + token.as_deref(), + ) + .await + } + GraphClient::Embedded { uri } => { + let db = Omnigraph::open(uri).await?; + Ok(commit_output(&db.get_commit(commit_id).await?)) + } + } + } +} diff --git a/crates/omnigraph-cli/src/main.rs b/crates/omnigraph-cli/src/main.rs index 0b518eb..e979622 100644 --- a/crates/omnigraph-cli/src/main.rs +++ b/crates/omnigraph-cli/src/main.rs @@ -23,12 +23,11 @@ use omnigraph_compiler::{ json_params_to_param_map, lint_query_file, }; use omnigraph_api_types::{ - BranchCreateOutput, BranchCreateRequest, BranchDeleteOutput, BranchListOutput, - BranchMergeOutput, BranchMergeRequest, ChangeOutput, CommitListOutput, CommitOutput, + BranchCreateOutput, BranchCreateRequest, BranchDeleteOutput, + BranchMergeOutput, BranchMergeRequest, ChangeOutput, CommitOutput, ErrorOutput, ExportRequest, GraphListResponse, IngestOutput, IngestRequest, ReadOutput, - ReadRequest, SchemaApplyOutput, SchemaApplyRequest, SchemaOutput, SnapshotOutput, - SnapshotTableOutput, commit_output, ingest_output, read_output, schema_apply_output, - snapshot_payload, + ReadRequest, SchemaApplyOutput, SchemaApplyRequest, + SnapshotTableOutput, ingest_output, read_output, schema_apply_output, }; use omnigraph_server::queries::{QueryRegistry, check, format_check_breakages}; use omnigraph_server::{ @@ -50,6 +49,7 @@ use embed::{EmbedArgs, EmbedOutput, execute_embed}; use read_format::{ReadRenderOptions, render_read}; mod cli; +mod client; mod helpers; mod output; use cli::*; @@ -325,27 +325,14 @@ async fn main() -> Result<()> { json, } => { let config = load_cli_config(config.as_ref())?; - let uri = - apply_server_flag(cli.server.as_deref(), cli.graph.as_deref(), uri, target.as_deref())?; - let bearer_token = - resolve_remote_bearer_token(&config, uri.as_deref(), target.as_deref())?; - let graph = resolve_cli_graph(&config, uri, target.as_deref())?; - let uri = graph.uri.clone(); - let payload = if graph.is_remote { - remote_json::( - &http_client, - Method::GET, - remote_url(&uri, "/branches"), - None, - bearer_token.as_deref(), - ) - .await? - } else { - let db = Omnigraph::open(&uri).await?; - let mut branches = db.branch_list().await?; - branches.sort(); - BranchListOutput { branches } - }; + let client = client::GraphClient::resolve( + &config, + cli.server.as_deref(), + cli.graph.as_deref(), + uri, + target.as_deref(), + )?; + let payload = client.branch_list().await?; if json { print_json(&payload)?; } else { @@ -455,37 +442,18 @@ async fn main() -> Result<()> { json, } => { let config = load_cli_config(config.as_ref())?; - let uri = - apply_server_flag(cli.server.as_deref(), cli.graph.as_deref(), uri, target.as_deref())?; - let bearer_token = - resolve_remote_bearer_token(&config, uri.as_deref(), target.as_deref())?; - let uri = resolve_uri(&config, uri, target.as_deref())?; - let commits = if is_remote_uri(&uri) { - remote_json::( - &http_client, - Method::GET, - if let Some(branch) = branch.as_deref() { - format!("{}?branch={}", remote_url(&uri, "/commits"), branch) - } else { - remote_url(&uri, "/commits") - }, - None, - bearer_token.as_deref(), - ) - .await? - .commits - } else { - let db = Omnigraph::open(&uri).await?; - db.list_commits(branch.as_deref()) - .await? - .iter() - .map(commit_output) - .collect::>() - }; + let client = client::GraphClient::resolve( + &config, + cli.server.as_deref(), + cli.graph.as_deref(), + uri, + target.as_deref(), + )?; + let payload = client.list_commits(branch.as_deref()).await?; if json { - print_json(&CommitListOutput { commits })?; + print_json(&payload)?; } else { - print_commit_list_human(&commits); + print_commit_list_human(&payload.commits); } } CommitCommand::Show { @@ -496,24 +464,14 @@ async fn main() -> Result<()> { json, } => { let config = load_cli_config(config.as_ref())?; - let uri = - apply_server_flag(cli.server.as_deref(), cli.graph.as_deref(), uri, target.as_deref())?; - let bearer_token = - resolve_remote_bearer_token(&config, uri.as_deref(), target.as_deref())?; - let uri = resolve_uri(&config, uri, target.as_deref())?; - let commit = if is_remote_uri(&uri) { - remote_json::( - &http_client, - Method::GET, - remote_url(&uri, &format!("/commits/{}", commit_id)), - None, - bearer_token.as_deref(), - ) - .await? - } else { - let db = Omnigraph::open(&uri).await?; - commit_output(&db.get_commit(&commit_id).await?) - }; + let client = client::GraphClient::resolve( + &config, + cli.server.as_deref(), + cli.graph.as_deref(), + uri, + target.as_deref(), + )?; + let commit = client.get_commit(&commit_id).await?; if json { print_json(&commit)?; } else { @@ -620,26 +578,14 @@ async fn main() -> Result<()> { json, } => { let config = load_cli_config(config.as_ref())?; - let uri = - apply_server_flag(cli.server.as_deref(), cli.graph.as_deref(), uri, target.as_deref())?; - let bearer_token = - resolve_remote_bearer_token(&config, uri.as_deref(), target.as_deref())?; - let uri = resolve_uri(&config, uri, target.as_deref())?; - let output = if is_remote_uri(&uri) { - remote_json::( - &http_client, - Method::GET, - remote_url(&uri, "/schema"), - None, - bearer_token.as_deref(), - ) - .await? - } else { - let db = Omnigraph::open(&uri).await?; - SchemaOutput { - schema_source: db.schema_source().to_string(), - } - }; + let client = client::GraphClient::resolve( + &config, + cli.server.as_deref(), + cli.graph.as_deref(), + uri, + target.as_deref(), + )?; + let output = client.schema_source().await?; if json { print_json(&output)?; } else { @@ -686,27 +632,15 @@ async fn main() -> Result<()> { json, } => { let config = load_cli_config(config.as_ref())?; - let uri = - apply_server_flag(cli.server.as_deref(), cli.graph.as_deref(), uri, target.as_deref())?; - let bearer_token = - resolve_remote_bearer_token(&config, uri.as_deref(), target.as_deref())?; - let uri = resolve_uri(&config, uri, target.as_deref())?; + let client = client::GraphClient::resolve( + &config, + cli.server.as_deref(), + cli.graph.as_deref(), + uri, + target.as_deref(), + )?; let branch = resolve_branch(&config, branch, None, "main"); - let payload = if is_remote_uri(&uri) { - remote_json::( - &http_client, - Method::GET, - format!("{}?branch={}", remote_url(&uri, "/snapshot"), branch), - None, - bearer_token.as_deref(), - ) - .await? - } else { - let db = Omnigraph::open(&uri).await?; - let snapshot = db.snapshot_of(ReadTarget::branch(branch.as_str())).await?; - snapshot_payload(&branch, &snapshot) - }; - + let payload = client.snapshot(&branch).await?; if json { print_json(&payload)?; } else {