From 4152d9d5dc5e5fa000ae75ecef371bf3dc892cb4 Mon Sep 17 00:00:00 2001 From: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> Date: Fri, 22 May 2026 17:54:26 +0000 Subject: [PATCH] feat(MR-656): inline query strings in CLI and HTTP server CLI: - Add -e / --query-string to omnigraph read and omnigraph change - Exactly one of --query, --query-string, --alias is required (3-way XOR) - Empty --query-string is rejected with a clear error HTTP: - New POST /query (read-only, clean field names: query/name/params/branch/snapshot) - Mutations on /query are rejected with 400 -- use POST /change instead - ChangeRequest fields polished: query (alias query_source), name (alias query_name) - POST /read and POST /change remain byte-compatible for existing clients Tests: - cli.rs: -e happy-path on read/change, mutex error vs --query, empty -e rejected - system_local.rs: inline -e read and -e change exercise the local flow - system_remote.rs: inline -e read/change over HTTP plus direct /query 200/400 - server.rs: /query 200, /query 400 on mutation, /change legacy field alias - openapi.rs: new /query path, QueryRequest schema, ChangeRequest field-name polish Docs: cli.md (-e examples), cli-reference.md (read/change rows), server.md (/query) Co-Authored-By: Ragnor Comerford --- crates/omnigraph-cli/src/main.rs | 41 +++-- crates/omnigraph-cli/tests/cli.rs | 96 ++++++++++ crates/omnigraph-cli/tests/system_local.rs | 31 ++++ crates/omnigraph-cli/tests/system_remote.rs | 61 +++++++ .../examples/bench_actor_isolation.rs | 4 +- .../examples/bench_concurrent_http.rs | 4 +- crates/omnigraph-server/src/api.rs | 40 +++- crates/omnigraph-server/src/lib.rs | 102 ++++++++++- crates/omnigraph-server/tests/openapi.rs | 58 +++++- crates/omnigraph-server/tests/server.rs | 172 ++++++++++++++---- docs/user/cli-reference.md | 4 +- docs/user/cli.md | 18 ++ docs/user/server.md | 31 +++- openapi.json | 121 +++++++++++- 14 files changed, 708 insertions(+), 75 deletions(-) diff --git a/crates/omnigraph-cli/src/main.rs b/crates/omnigraph-cli/src/main.rs index ac21e7b..7585b10 100644 --- a/crates/omnigraph-cli/src/main.rs +++ b/crates/omnigraph-cli/src/main.rs @@ -170,10 +170,13 @@ enum Command { target: Option, #[arg(long)] config: Option, - #[arg(long)] + #[arg(long, conflicts_with_all = ["query", "query_string"])] alias: Option, - #[arg(long)] + #[arg(long, conflicts_with_all = ["alias", "query_string"])] query: Option, + /// Inline GQ source — alternative to `--query ` and `--alias `. + #[arg(short = 'e', long = "query-string", value_name = "GQ", conflicts_with_all = ["query", "alias"])] + query_string: Option, #[arg(long)] name: Option, #[command(flatten)] @@ -200,10 +203,13 @@ enum Command { target: Option, #[arg(long)] config: Option, - #[arg(long)] + #[arg(long, conflicts_with_all = ["query", "query_string"])] alias: Option, - #[arg(long)] + #[arg(long, conflicts_with_all = ["alias", "query_string"])] query: Option, + /// Inline GQ source — alternative to `--query ` and `--alias `. + #[arg(short = 'e', long = "query-string", value_name = "GQ", conflicts_with_all = ["query", "alias"])] + query_string: Option, #[arg(long)] name: Option, #[command(flatten)] @@ -906,7 +912,9 @@ fn resolve_query_path( .map(PathBuf::from) .or_else(|| alias_query.map(PathBuf::from)) .ok_or_else(|| { - color_eyre::eyre::eyre!("exactly one of --query or --alias must be provided") + color_eyre::eyre::eyre!( + "exactly one of --query, --query-string, or --alias must be provided" + ) }) .and_then(|query_path| config.resolve_query_path(&query_path)) } @@ -914,8 +922,15 @@ fn resolve_query_path( fn resolve_query_source( config: &OmnigraphConfig, explicit_query: Option<&PathBuf>, + inline_query: Option<&str>, alias_query: Option<&str>, ) -> Result { + if let Some(inline) = inline_query { + if inline.trim().is_empty() { + bail!("--query-string must not be empty"); + } + return Ok(inline.to_string()); + } Ok(fs::read_to_string(resolve_query_path( config, explicit_query, @@ -1629,8 +1644,8 @@ async fn execute_change_remote( Method::POST, remote_url(uri, "/change"), Some(serde_json::to_value(ChangeRequest { - query_source: query_source.to_string(), - query_name: query_name.map(ToOwned::to_owned), + query: query_source.to_string(), + name: query_name.map(ToOwned::to_owned), params: params_json.cloned(), branch: Some(branch.to_string()), })?), @@ -2249,6 +2264,7 @@ async fn main() -> Result<()> { config, alias, query, + query_string, name, params, branch, @@ -2257,8 +2273,8 @@ async fn main() -> Result<()> { json, alias_args, } => { - if alias.is_some() == query.is_some() { - bail!("exactly one of --alias or --query must be provided"); + if alias.is_none() && query.is_none() && query_string.is_none() { + bail!("exactly one of --query, --query-string, or --alias must be provided"); } let config = load_cli_config(config.as_ref())?; @@ -2281,6 +2297,7 @@ async fn main() -> Result<()> { let query_source = resolve_query_source( &config, query.as_ref(), + query_string.as_deref(), alias_config.map(|a| a.query.as_str()), )?; let params_json = merged_params_json( @@ -2334,14 +2351,15 @@ async fn main() -> Result<()> { config, alias, query, + query_string, name, params, branch, json, alias_args, } => { - if alias.is_some() == query.is_some() { - bail!("exactly one of --alias or --query must be provided"); + if alias.is_none() && query.is_none() && query_string.is_none() { + bail!("exactly one of --query, --query-string, or --alias must be provided"); } let config = load_cli_config(config.as_ref())?; @@ -2364,6 +2382,7 @@ async fn main() -> Result<()> { let query_source = resolve_query_source( &config, query.as_ref(), + query_string.as_deref(), alias_config.map(|a| a.query.as_str()), )?; let params_json = merged_params_json( diff --git a/crates/omnigraph-cli/tests/cli.rs b/crates/omnigraph-cli/tests/cli.rs index 578d1bd..63abd5a 100644 --- a/crates/omnigraph-cli/tests/cli.rs +++ b/crates/omnigraph-cli/tests/cli.rs @@ -1422,6 +1422,102 @@ fn read_requires_name_for_multi_query_files() { assert!(stderr.contains("multiple queries")); } +#[test] +fn read_supports_inline_query_string() { + let temp = tempdir().unwrap(); + let repo = repo_path(temp.path()); + init_repo(&repo); + load_fixture(&repo); + + let output = output_success( + cli() + .arg("read") + .arg(&repo) + .arg("-e") + .arg("query find($name: String) { match { $p: Person { name: $name } } return { $p.name, $p.age } }") + .arg("--params") + .arg(r#"{"name":"Alice"}"#) + .arg("--json"), + ); + let payload: Value = serde_json::from_slice(&output.stdout).unwrap(); + assert_eq!(payload["query_name"], "find"); + assert_eq!(payload["row_count"], 1); + assert_eq!(payload["rows"][0]["p.name"], "Alice"); +} + +#[test] +fn change_supports_inline_query_string() { + let temp = tempdir().unwrap(); + let repo = repo_path(temp.path()); + init_repo(&repo); + load_fixture(&repo); + + let output = output_success( + cli() + .arg("change") + .arg(&repo) + .arg("--query-string") + .arg("query add($name: String, $age: I32) { insert Person { name: $name, age: $age } }") + .arg("--params") + .arg(r#"{"name":"Inline","age":42}"#) + .arg("--json"), + ); + let payload: Value = serde_json::from_slice(&output.stdout).unwrap(); + assert_eq!(payload["query_name"], "add"); + assert_eq!(payload["affected_nodes"], 1); + + let verify = output_success( + cli() + .arg("read") + .arg(&repo) + .arg("-e") + .arg("query find($name: String) { match { $p: Person { name: $name } } return { $p.name } }") + .arg("--params") + .arg(r#"{"name":"Inline"}"#) + .arg("--json"), + ); + let verify_payload: Value = serde_json::from_slice(&verify.stdout).unwrap(); + assert_eq!(verify_payload["row_count"], 1); +} + +#[test] +fn read_rejects_query_string_combined_with_query() { + let temp = tempdir().unwrap(); + let repo = repo_path(temp.path()); + init_repo(&repo); + load_fixture(&repo); + + let output = output_failure( + cli() + .arg("read") + .arg(&repo) + .arg("--query") + .arg(fixture("test.gq")) + .arg("-e") + .arg("query whatever() { match { $p: Person } return { $p.name } }"), + ); + let stderr = String::from_utf8(output.stderr).unwrap(); + assert!( + stderr.contains("cannot be used") || stderr.contains("conflict"), + "expected clap conflict error, got: {stderr}" + ); +} + +#[test] +fn read_rejects_empty_query_string() { + let temp = tempdir().unwrap(); + let repo = repo_path(temp.path()); + init_repo(&repo); + load_fixture(&repo); + + let output = output_failure(cli().arg("read").arg(&repo).arg("-e").arg("")); + let stderr = String::from_utf8(output.stderr).unwrap(); + assert!( + stderr.contains("must not be empty"), + "expected empty-string rejection, got: {stderr}" + ); +} + #[test] fn branch_create_json_outputs_source_and_name() { let temp = tempdir().unwrap(); diff --git a/crates/omnigraph-cli/tests/system_local.rs b/crates/omnigraph-cli/tests/system_local.rs index 3d3e9bf..882221f 100644 --- a/crates/omnigraph-cli/tests/system_local.rs +++ b/crates/omnigraph-cli/tests/system_local.rs @@ -246,6 +246,37 @@ fn local_cli_end_to_end_init_load_read_change_read_flow() { )); assert_eq!(read_after["row_count"], 1); assert_eq!(read_after["rows"][0]["p.name"], "Eve"); + + // Inline-source variants of the same read/change flow (CLI `-e` / + // `--query-string`). Confirms that file-less invocations reach the + // engine identically, including param binding and `branch=main` defaults. + let inline_change = parse_stdout_json(&output_success( + cli() + .arg("change") + .arg(repo.path()) + .arg("-e") + .arg("query add($name: String, $age: I32) { insert Person { name: $name, age: $age } }") + .arg("--params") + .arg(r#"{"name":"Inline","age":42}"#) + .arg("--json"), + )); + assert_eq!(inline_change["branch"], "main"); + assert_eq!(inline_change["query_name"], "add"); + assert_eq!(inline_change["affected_nodes"], 1); + + let inline_read = parse_stdout_json(&output_success( + cli() + .arg("read") + .arg(repo.path()) + .arg("--query-string") + .arg("query find($name: String) { match { $p: Person { name: $name } } return { $p.name, $p.age } }") + .arg("--params") + .arg(r#"{"name":"Inline"}"#) + .arg("--json"), + )); + assert_eq!(inline_read["row_count"], 1); + assert_eq!(inline_read["rows"][0]["p.name"], "Inline"); + assert_eq!(inline_read["rows"][0]["p.age"], 42); } #[test] diff --git a/crates/omnigraph-cli/tests/system_remote.rs b/crates/omnigraph-cli/tests/system_remote.rs index 15f3a6f..48f50ab 100644 --- a/crates/omnigraph-cli/tests/system_remote.rs +++ b/crates/omnigraph-cli/tests/system_remote.rs @@ -192,6 +192,67 @@ query insert_person($name: String, $age: I32) { assert_eq!(local_verify["row_count"], 1); assert_eq!(local_verify["rows"][0]["p.name"], "Mina"); + // CLI `-e` over the HTTP transport (--config points at remote server). + // Confirms inline source survives the remote-execution path identically + // to file-based queries, and exercises `POST /query` end-to-end via the + // change-then-read round trip we just established. + let inline_remote_read = parse_stdout_json(&output_success( + cli() + .arg("read") + .arg("--config") + .arg(&config) + .arg("-e") + .arg("query find($name: String) { match { $p: Person { name: $name } } return { $p.name, $p.age } }") + .arg("--params") + .arg(r#"{"name":"Mina"}"#) + .arg("--json"), + )); + assert_eq!(inline_remote_read["row_count"], 1); + assert_eq!(inline_remote_read["rows"][0]["p.name"], "Mina"); + + let inline_remote_change = parse_stdout_json(&output_success( + cli() + .arg("change") + .arg("--config") + .arg(&config) + .arg("--query-string") + .arg("query add($name: String, $age: I32) { insert Person { name: $name, age: $age } }") + .arg("--params") + .arg(r#"{"name":"Inline","age":42}"#) + .arg("--json"), + )); + assert_eq!(inline_remote_change["affected_nodes"], 1); + + // `POST /query` happy path directly: a hand-rolled HTTP body using the + // new clean field names. + let http_query = client + .post(format!("{}/query", server.base_url)) + .json(&json!({ + "branch": "main", + "query": "query find($name: String) { match { $p: Person { name: $name } } return { $p.name } }", + "params": { "name": "Inline" } + })) + .send() + .unwrap() + .error_for_status() + .unwrap() + .json::() + .unwrap(); + assert_eq!(http_query["row_count"], 1); + assert_eq!(http_query["rows"][0]["p.name"], "Inline"); + + // `POST /query` rejects mutations with 400. + let http_query_mutation = client + .post(format!("{}/query", server.base_url)) + .json(&json!({ + "branch": "main", + "query": "query bad($name: String, $age: I32) { insert Person { name: $name, age: $age } }", + "params": { "name": "Nope", "age": 1 } + })) + .send() + .unwrap(); + assert_eq!(http_query_mutation.status(), reqwest::StatusCode::BAD_REQUEST); + // `run publish` / `run list` removed. Direct-to-target writes // already landed via the change call above; the commit graph is now // the audit surface (verified separately by `commit list`). diff --git a/crates/omnigraph-server/examples/bench_actor_isolation.rs b/crates/omnigraph-server/examples/bench_actor_isolation.rs index 1eca032..4e3299c 100644 --- a/crates/omnigraph-server/examples/bench_actor_isolation.rs +++ b/crates/omnigraph-server/examples/bench_actor_isolation.rs @@ -199,8 +199,8 @@ async fn drive_light_actor( let mut other = 0usize; for op_idx in 0..ops { let request_body = ChangeRequest { - query_source: "query insert_person($name: String, $age: I32) {\n insert Person { name: $name, age: $age }\n}".to_string(), - query_name: Some("insert_person".to_string()), + query: "query insert_person($name: String, $age: I32) {\n insert Person { name: $name, age: $age }\n}".to_string(), + name: Some("insert_person".to_string()), params: Some(serde_json::json!({ "name": format!("light-{actor_idx}-{op_idx}"), "age": op_idx as i32, diff --git a/crates/omnigraph-server/examples/bench_concurrent_http.rs b/crates/omnigraph-server/examples/bench_concurrent_http.rs index 11505e7..c114dfb 100644 --- a/crates/omnigraph-server/examples/bench_concurrent_http.rs +++ b/crates/omnigraph-server/examples/bench_concurrent_http.rs @@ -121,8 +121,8 @@ async fn drive_actor( for op_idx in 0..ops { let table_idx = pick_table(actor_idx, op_idx, mode, num_tables); let request_body = ChangeRequest { - query_source: build_query_source(table_idx), - query_name: Some("insert_item".to_string()), + query: build_query_source(table_idx), + name: Some("insert_item".to_string()), params: Some(serde_json::json!({ "name": format!("a{actor_idx}_o{op_idx}"), "value": op_idx as i32, diff --git a/crates/omnigraph-server/src/api.rs b/crates/omnigraph-server/src/api.rs index 1195f12..9498c38 100644 --- a/crates/omnigraph-server/src/api.rs +++ b/crates/omnigraph-server/src/api.rs @@ -248,17 +248,49 @@ pub struct ReadRequest { pub snapshot: Option, } +/// Inline read-query request for `POST /query`. +/// +/// Friendlier-named alternative to [`ReadRequest`] for ad-hoc reads and +/// AI-agent integration. Mutations are rejected with 400 — use `POST +/// /change` for write queries. Field names are deliberately short +/// (`query`, `name`) to match the GQ keyword and the CLI `-e` flag. +#[derive(Debug, Clone, Serialize, Deserialize, ToSchema)] +pub struct QueryRequest { + /// GQ read-query source. May declare one or more named queries; pick one + /// with `name` when more than one is declared. Mutations + /// (`insert`/`update`/`delete`) get 400 — use `POST /change` instead. + #[schema(example = "query get_person($name: String) {\n match {\n $p: Person { name: $name }\n }\n return { $p.name, $p.age }\n}")] + pub query: String, + /// Name of the query to run when `query` declares multiple. Optional when + /// only one query is declared. + pub name: Option, + /// JSON object whose keys match the query's declared parameters. + pub params: Option, + /// Branch to read from. Mutually exclusive with `snapshot`. Defaults to `main`. + pub branch: Option, + /// Snapshot id to read from. Mutually exclusive with `branch`. + pub snapshot: Option, +} + #[derive(Debug, Clone, Serialize, Deserialize, ToSchema)] pub struct ChangeRequest { /// GQ mutation source containing `insert`, `update`, or `delete` statements. - /// May declare multiple named mutations; pick one with `query_name`. + /// May declare multiple named mutations; pick one with `name`. + /// + /// Accepts the legacy field name `query_source` as a deserialization alias. #[schema(example = "query insert_person($name: String, $age: I32) {\n insert Person { name: $name, age: $age }\n}")] - pub query_source: String, - /// Name of the mutation to run when `query_source` declares multiple. - pub query_name: Option, + #[serde(alias = "query_source")] + pub query: String, + /// Name of the mutation to run when `query` declares multiple. + /// + /// Accepts the legacy field name `query_name` as a deserialization alias. + #[serde(default, alias = "query_name")] + pub name: Option, /// JSON object whose keys match the mutation's declared parameters. + #[serde(default)] pub params: Option, /// Target branch. Defaults to `main`. + #[serde(default)] pub branch: Option, } diff --git a/crates/omnigraph-server/src/lib.rs b/crates/omnigraph-server/src/lib.rs index 0ab2249..031d450 100644 --- a/crates/omnigraph-server/src/lib.rs +++ b/crates/omnigraph-server/src/lib.rs @@ -15,8 +15,8 @@ use api::{ BranchCreateOutput, BranchCreateRequest, BranchDeleteOutput, BranchListOutput, BranchMergeOutput, BranchMergeRequest, ChangeOutput, ChangeRequest, CommitListOutput, CommitListQuery, ErrorCode, ErrorOutput, ExportRequest, HealthOutput, IngestOutput, - IngestRequest, ReadOutput, ReadRequest, SchemaApplyOutput, SchemaApplyRequest, SchemaOutput, - SnapshotQuery, ingest_output, schema_apply_output, snapshot_payload, + IngestRequest, QueryRequest, ReadOutput, ReadRequest, SchemaApplyOutput, SchemaApplyRequest, + SchemaOutput, SnapshotQuery, ingest_output, schema_apply_output, snapshot_payload, }; use axum::body::{Body, Bytes}; use axum::extract::DefaultBodyLimit; @@ -74,6 +74,7 @@ fn hash_bearer_token(token: &str) -> BearerTokenHash { server_health, server_snapshot, server_read, + server_query, server_export, server_change, server_schema_apply, @@ -631,6 +632,7 @@ pub fn build_app(state: AppState) -> Router { .route("/snapshot", get(server_snapshot)) .route("/export", post(server_export)) .route("/read", post(server_read)) + .route("/query", post(server_query)) .route("/change", post(server_change)) .route("/schema", get(server_schema_get)) .route("/schema/apply", post(server_schema_apply)) @@ -980,6 +982,85 @@ async fn server_read( Ok(Json(api::read_output(selected_name, &target, result))) } +#[utoipa::path( + post, + path = "/query", + tag = "queries", + operation_id = "query", + request_body = QueryRequest, + responses( + (status = 200, description = "Query results", body = ReadOutput), + (status = 400, description = "Bad request - also returned when the query body contains mutations; use POST /change for write queries", body = ErrorOutput), + (status = 401, description = "Unauthorized", body = ErrorOutput), + (status = 403, description = "Forbidden", body = ErrorOutput), + ), + security(("bearer_token" = [])), +)] +/// Execute an inline read query (friendlier-named alternative to `POST /read`). +/// +/// Designed for ad-hoc exploration and AI-agent tool-use: short field +/// names (`query`, `name`) match the CLI `-e` flag and the GQ `query` +/// keyword. Mutations (`insert`/`update`/`delete`) are rejected with 400 +/// -- use `POST /change` for write queries. Otherwise behaves +/// identically to `POST /read`: same target semantics (branch xor +/// snapshot), same Cedar action (Read), same response shape. +async fn server_query( + State(state): State, + actor: Option>, + Json(request): Json, +) -> std::result::Result, ApiError> { + if request.branch.is_some() && request.snapshot.is_some() { + return Err(ApiError::bad_request( + "query request may specify branch or snapshot, not both", + )); + } + + let target = read_target_from_request(request.branch, request.snapshot); + let policy_branch = match &target { + ReadTarget::Branch(branch) => Some(branch.clone()), + ReadTarget::Snapshot(_) if state.policy_engine().is_some() && actor.is_some() => { + let db = &state.engine; + db.resolved_branch_of(target.clone()) + .await + .map(|branch| branch.or_else(|| Some("main".to_string()))) + .map_err(ApiError::from_omni)? + } + ReadTarget::Snapshot(_) => None, + }; + authorize_request( + &state, + actor.as_ref().map(|Extension(actor)| actor), + PolicyRequest { + actor_id: actor + .as_ref() + .map(|Extension(actor)| actor.as_str().to_string()) + .unwrap_or_default(), + action: PolicyAction::Read, + branch: policy_branch, + target_branch: None, + }, + )?; + let query_decl = select_named_query_decl(&request.query, request.name.as_deref()) + .map_err(|err| ApiError::bad_request(err.to_string()))?; + if !query_decl.mutations.is_empty() { + return Err(ApiError::bad_request(format!( + "query '{}' contains mutations (insert/update/delete); use POST /change for write queries", + query_decl.name + ))); + } + let selected_name = query_decl.name.clone(); + let params = query_params_from_json(&query_decl.params, request.params.as_ref()) + .map_err(|err| ApiError::bad_request(err.to_string()))?; + + let result = { + let db = &state.engine; + db.query(target.clone(), &request.query, &selected_name, ¶ms) + .await + .map_err(ApiError::from_omni)? + }; + Ok(Json(api::read_output(selected_name, &target, result))) +} + #[utoipa::path( post, path = "/export", @@ -1092,7 +1173,7 @@ async fn server_change( // estimated bytes per actor. Cedar runs FIRST so denied requests // don't consume admission slots. Estimate uses the request body // size as a coarse proxy; engine memory pressure can run higher. - let est_bytes = request.query_source.len() as u64 + let est_bytes = request.query.len() as u64 + request .params .as_ref() @@ -1103,7 +1184,7 @@ async fn server_change( .try_admit(&actor_arc, est_bytes) .map_err(ApiError::from_workload_reject)?; let (selected_name, query_params) = - select_named_query(&request.query_source, request.query_name.as_deref()) + select_named_query(&request.query, request.name.as_deref()) .map_err(|err| ApiError::bad_request(err.to_string()))?; let params = query_params_from_json(&query_params, request.params.as_ref()) .map_err(|err| ApiError::bad_request(err.to_string()))?; @@ -1112,7 +1193,7 @@ async fn server_change( let db = &state.engine; db.mutate_as( &branch, - &request.query_source, + &request.query, &selected_name, ¶ms, actor_id, @@ -1658,10 +1739,10 @@ fn read_target_from_request(branch: Option, snapshot: Option) -> } } -fn select_named_query( +fn select_named_query_decl( query_source: &str, requested_name: Option<&str>, -) -> Result<(String, Vec)> { +) -> Result { let parsed = parse_query(query_source)?; let query = if let Some(name) = requested_name { parsed @@ -1674,7 +1755,14 @@ fn select_named_query( } else { bail!("query file contains multiple queries; pass --name"); }; + Ok(query) +} +fn select_named_query( + query_source: &str, + requested_name: Option<&str>, +) -> Result<(String, Vec)> { + let query = select_named_query_decl(query_source, requested_name)?; Ok((query.name, query.params)) } diff --git a/crates/omnigraph-server/tests/openapi.rs b/crates/omnigraph-server/tests/openapi.rs index 86a124d..8b922b1 100644 --- a/crates/omnigraph-server/tests/openapi.rs +++ b/crates/omnigraph-server/tests/openapi.rs @@ -159,6 +159,7 @@ const EXPECTED_PATHS: &[&str] = &[ "/healthz", "/snapshot", "/read", + "/query", "/export", "/change", "/schema", @@ -278,6 +279,7 @@ const EXPECTED_SCHEMAS: &[&str] = &[ "BranchMergeRequest", "ChangeOutput", "ChangeRequest", + "QueryRequest", "CommitListOutput", "CommitOutput", "ErrorCode", @@ -368,13 +370,65 @@ fn read_output_schema_has_expected_fields() { #[test] fn change_request_schema_has_expected_fields() { + // Canonical field names on the wire are now `query` and `name`. The + // schema descriptions document `query_source` and `query_name` as + // legacy deserialization aliases for backward compatibility. let doc = openapi_json(); let schema = &doc["components"]["schemas"]["ChangeRequest"]; let props = schema["properties"].as_object().unwrap(); - assert!(props.contains_key("query_source")); - assert!(props.contains_key("query_name")); + assert!(props.contains_key("query")); + assert!(props.contains_key("name")); assert!(props.contains_key("params")); assert!(props.contains_key("branch")); + let query_desc = schema["properties"]["query"]["description"] + .as_str() + .unwrap_or_default(); + assert!( + query_desc.contains("query_source"), + "expected `query` description to mention the legacy `query_source` alias, got: {query_desc}" + ); +} + +#[test] +fn query_request_schema_has_expected_fields() { + let doc = openapi_json(); + let schema = &doc["components"]["schemas"]["QueryRequest"]; + let props = schema["properties"].as_object().unwrap(); + assert!(props.contains_key("query")); + assert!(props.contains_key("name")); + assert!(props.contains_key("params")); + assert!(props.contains_key("branch")); + assert!(props.contains_key("snapshot")); +} + +#[test] +fn query_request_query_is_required() { + let doc = openapi_json(); + let schema = &doc["components"]["schemas"]["QueryRequest"]; + let required: Vec<&str> = schema["required"] + .as_array() + .unwrap() + .iter() + .map(|v| v.as_str().unwrap()) + .collect(); + assert!(required.contains(&"query")); +} + +#[test] +fn openapi_query_is_post() { + let doc = openapi_json(); + assert!(doc["paths"]["/query"]["post"].is_object()); +} + +#[test] +fn query_endpoint_documents_mutation_400() { + let doc = openapi_json(); + let four_hundred = &doc["paths"]["/query"]["post"]["responses"]["400"]; + let description = four_hundred["description"].as_str().unwrap_or_default(); + assert!( + description.contains("mutations") || description.contains("POST /change"), + "expected /query 400 response to mention mutation rejection, got: {description}" + ); } #[test] diff --git a/crates/omnigraph-server/tests/server.rs b/crates/omnigraph-server/tests/server.rs index e7b4458..d0e10b1 100644 --- a/crates/omnigraph-server/tests/server.rs +++ b/crates/omnigraph-server/tests/server.rs @@ -14,7 +14,7 @@ use omnigraph::loader::{LoadMode, load_jsonl}; use omnigraph_policy::{PolicyChecker, PolicyEngine}; use omnigraph_server::api::{ BranchCreateRequest, BranchMergeRequest, ChangeRequest, ErrorOutput, ExportRequest, - IngestRequest, ReadRequest, SchemaApplyRequest, SchemaOutput, + IngestRequest, QueryRequest, ReadRequest, SchemaApplyRequest, SchemaOutput, }; use omnigraph_server::{AppState, build_app}; use serde_json::{Value, json}; @@ -831,8 +831,8 @@ async fn schema_drift_returns_conflict_for_snapshot_read_and_change() { ); let change = ChangeRequest { - query_source: MUTATION_QUERIES.to_string(), - query_name: Some("insert_person".to_string()), + query: MUTATION_QUERIES.to_string(), + name: Some("insert_person".to_string()), params: Some(json!({ "name": "Mina", "age": 28 })), branch: Some("main".to_string()), }; @@ -1470,8 +1470,8 @@ async fn policy_blocks_change_on_protected_main_but_allows_unprotected_branch() let app = build_app(state); let main_change = ChangeRequest { - query_source: MUTATION_QUERIES.to_string(), - query_name: Some("insert_person".to_string()), + query: MUTATION_QUERIES.to_string(), + name: Some("insert_person".to_string()), params: Some(json!({ "name": "Mina", "age": 28 })), branch: Some("main".to_string()), }; @@ -1494,8 +1494,8 @@ async fn policy_blocks_change_on_protected_main_but_allows_unprotected_branch() ); let feature_change = ChangeRequest { - query_source: MUTATION_QUERIES.to_string(), - query_name: Some("insert_person".to_string()), + query: MUTATION_QUERIES.to_string(), + name: Some("insert_person".to_string()), params: Some(json!({ "name": "Mina", "age": 28 })), branch: Some("feature".to_string()), }; @@ -1590,8 +1590,8 @@ async fn authenticated_change_stamps_actor_on_commits() { let (_temp, app) = app_for_loaded_repo_with_auth_tokens(&[("act-andrew", "token-one")]).await; let change = ChangeRequest { - query_source: MUTATION_QUERIES.to_string(), - query_name: Some("insert_person".to_string()), + query: MUTATION_QUERIES.to_string(), + name: Some("insert_person".to_string()), params: Some(json!({ "name": "Mina", "age": 28 })), branch: Some("main".to_string()), }; @@ -1839,8 +1839,8 @@ async fn authenticated_branch_merge_stamps_merge_actor_on_head_commit() { assert_eq!(create_status, StatusCode::OK); let change = ChangeRequest { - query_source: MUTATION_QUERIES.to_string(), - query_name: Some("insert_person".to_string()), + query: MUTATION_QUERIES.to_string(), + name: Some("insert_person".to_string()), params: Some(json!({ "name": "Zoe", "age": 33 })), branch: Some("feature".to_string()), }; @@ -1969,8 +1969,8 @@ async fn repeated_read_after_change_sees_updated_state_from_same_app() { let (_temp, app) = app_for_loaded_repo().await; let change = ChangeRequest { - query_source: MUTATION_QUERIES.to_string(), - query_name: Some("insert_person".to_string()), + query: MUTATION_QUERIES.to_string(), + name: Some("insert_person".to_string()), params: Some(json!({ "name": "Mina", "age": 28 })), branch: Some("main".to_string()), }; @@ -2009,6 +2009,108 @@ async fn repeated_read_after_change_sees_updated_state_from_same_app() { assert_eq!(read_body["rows"][0]["p.name"], "Mina"); } +#[tokio::test(flavor = "multi_thread")] +async fn query_endpoint_runs_inline_read() { + let (_temp, app) = app_for_loaded_repo().await; + + let query = QueryRequest { + query: fs::read_to_string(fixture("test.gq")).unwrap(), + name: Some("get_person".to_string()), + params: Some(json!({ "name": "Alice" })), + branch: Some("main".to_string()), + snapshot: None, + }; + let (status, body) = json_response( + &app, + Request::builder() + .uri("/query") + .method(Method::POST) + .header("content-type", "application/json") + .body(Body::from(serde_json::to_vec(&query).unwrap())) + .unwrap(), + ) + .await; + assert_eq!(status, StatusCode::OK); + assert_eq!(body["query_name"], "get_person"); + assert_eq!(body["row_count"], 1); + assert_eq!(body["rows"][0]["p.name"], "Alice"); +} + +#[tokio::test(flavor = "multi_thread")] +async fn query_endpoint_rejects_mutation_with_400() { + let (_temp, app) = app_for_loaded_repo().await; + + let query = QueryRequest { + query: MUTATION_QUERIES.to_string(), + name: Some("insert_person".to_string()), + params: Some(json!({ "name": "Should", "age": 1 })), + branch: Some("main".to_string()), + snapshot: None, + }; + let (status, body) = json_response( + &app, + Request::builder() + .uri("/query") + .method(Method::POST) + .header("content-type", "application/json") + .body(Body::from(serde_json::to_vec(&query).unwrap())) + .unwrap(), + ) + .await; + assert_eq!(status, StatusCode::BAD_REQUEST); + let err = body["error"].as_str().unwrap_or_default(); + assert!( + err.contains("contains mutations") && err.contains("POST /change"), + "expected mutation-rejection message, got: {err}" + ); +} + +#[tokio::test(flavor = "multi_thread")] +async fn change_endpoint_accepts_legacy_field_names() { + // The canonical wire field names on /change are `query` and `name`, but + // serde aliases keep the legacy `query_source`/`query_name` payload + // shape working for clients that haven't migrated yet. Pin both shapes. + let (_temp, app) = app_for_loaded_repo().await; + + let legacy_body = json!({ + "query_source": MUTATION_QUERIES, + "query_name": "insert_person", + "params": { "name": "Legacy", "age": 21 }, + "branch": "main", + }); + let (status, body) = json_response( + &app, + Request::builder() + .uri("/change") + .method(Method::POST) + .header("content-type", "application/json") + .body(Body::from(serde_json::to_vec(&legacy_body).unwrap())) + .unwrap(), + ) + .await; + assert_eq!(status, StatusCode::OK); + assert_eq!(body["affected_nodes"], 1); + + let canonical_body = json!({ + "query": MUTATION_QUERIES, + "name": "insert_person", + "params": { "name": "Canonical", "age": 22 }, + "branch": "main", + }); + let (status, body) = json_response( + &app, + Request::builder() + .uri("/change") + .method(Method::POST) + .header("content-type", "application/json") + .body(Body::from(serde_json::to_vec(&canonical_body).unwrap())) + .unwrap(), + ) + .await; + assert_eq!(status, StatusCode::OK); + assert_eq!(body["affected_nodes"], 1); +} + #[tokio::test(flavor = "multi_thread")] async fn remote_branch_list_create_merge_flow_works() { let (_temp, app) = app_for_loaded_repo().await; @@ -2056,8 +2158,8 @@ async fn remote_branch_list_create_merge_flow_works() { assert_eq!(list_body["branches"], json!(["feature", "main"])); let change = ChangeRequest { - query_source: MUTATION_QUERIES.to_string(), - query_name: Some("insert_person".to_string()), + query: MUTATION_QUERIES.to_string(), + name: Some("insert_person".to_string()), params: Some(json!({ "name": "Zoe", "age": 33 })), branch: Some("feature".to_string()), }; @@ -2390,8 +2492,8 @@ async fn change_conflict_returns_manifest_conflict_409() { .header("content-type", "application/json") .body(Body::from( serde_json::to_vec(&ChangeRequest { - query_source: MUTATION_QUERIES.to_string(), - query_name: Some("set_age".to_string()), + query: MUTATION_QUERIES.to_string(), + name: Some("set_age".to_string()), params: Some(json!({ "name": "Alice", "age": 33 })), branch: Some("main".to_string()), }) @@ -2450,8 +2552,8 @@ async fn change_concurrent_inserts_same_key_serialize_without_409() { let app = app.clone(); handles.push(tokio::spawn(async move { let body = serde_json::to_vec(&ChangeRequest { - query_source: MUTATION_QUERIES.to_string(), - query_name: Some("insert_person".to_string()), + query: MUTATION_QUERIES.to_string(), + name: Some("insert_person".to_string()), params: Some(json!({ "name": format!("racer-{i}"), "age": i as i32 })), branch: Some("main".to_string()), }) @@ -2563,8 +2665,8 @@ async fn change_concurrent_updates_same_key_serialize_via_publisher_cas() { let target_age = 100 + i as i32; handles.push(tokio::spawn(async move { let body = serde_json::to_vec(&ChangeRequest { - query_source: MUTATION_QUERIES.to_string(), - query_name: Some("set_age".to_string()), + query: MUTATION_QUERIES.to_string(), + name: Some("set_age".to_string()), params: Some(json!({ "name": "Alice", "age": target_age })), branch: Some("main".to_string()), }) @@ -2738,8 +2840,8 @@ mod matrix { pub async fn insert_person(&self, branch: &str, name: &str, age: i32) { let body = serde_json::to_vec(&ChangeRequest { - query_source: MUTATION_QUERIES.to_string(), - query_name: Some("insert_person".to_string()), + query: MUTATION_QUERIES.to_string(), + name: Some("insert_person".to_string()), params: Some(json!({ "name": name, "age": age })), branch: Some(branch.to_string()), }) @@ -2893,8 +2995,8 @@ mod matrix { /// /change either deadlocks or returns a non-200. pub async fn assert_post_op_sentinel(&self, cell: &str, sentinel: &str) { let body = serde_json::to_vec(&ChangeRequest { - query_source: MUTATION_QUERIES.to_string(), - query_name: Some("insert_person".to_string()), + query: MUTATION_QUERIES.to_string(), + name: Some("insert_person".to_string()), params: Some(json!({ "name": sentinel, "age": 99 })), branch: Some("main".to_string()), }) @@ -2972,8 +3074,8 @@ mod matrix { tokio::spawn(async move { barrier.wait().await; let body = serde_json::to_vec(&ChangeRequest { - query_source: MUTATION_QUERIES.to_string(), - query_name: Some("insert_person".to_string()), + query: MUTATION_QUERIES.to_string(), + name: Some("insert_person".to_string()), params: Some(json!({ "name": name, "age": age })), branch: Some(branch), }) @@ -3484,8 +3586,8 @@ query insert_c($name: String) { let app_p = app.clone(); handles.push(tokio::spawn(async move { let body = serde_json::to_vec(&ChangeRequest { - query_source: PERSON_QUERY.to_string(), - query_name: Some("insert_p".to_string()), + query: PERSON_QUERY.to_string(), + name: Some("insert_p".to_string()), params: Some(json!({ "name": format!("p-{i}"), "age": i as i32 })), branch: Some("main".to_string()), }) @@ -3501,8 +3603,8 @@ query insert_c($name: String) { let app_c = app.clone(); handles.push(tokio::spawn(async move { let body = serde_json::to_vec(&ChangeRequest { - query_source: COMPANY_QUERY.to_string(), - query_name: Some("insert_c".to_string()), + query: COMPANY_QUERY.to_string(), + name: Some("insert_c".to_string()), params: Some(json!({ "name": format!("c-{i}") })), branch: Some("main".to_string()), }) @@ -3767,8 +3869,8 @@ async fn default_deny_mode_rejects_change_with_forbidden() { .await; let change = ChangeRequest { - query_source: MUTATION_QUERIES.to_string(), - query_name: Some("insert_person".to_string()), + query: MUTATION_QUERIES.to_string(), + name: Some("insert_person".to_string()), params: Some(json!({ "name": "DefaultDeny", "age": 1 })), branch: Some("main".to_string()), }; @@ -3925,8 +4027,8 @@ async fn http_change_decision( .unwrap(); let app = build_app(state); let req = ChangeRequest { - query_source: MUTATION_QUERIES.to_string(), - query_name: Some("insert_person".to_string()), + query: MUTATION_QUERIES.to_string(), + name: Some("insert_person".to_string()), params: Some(json!({ "name": "ParityCharlie", "age": 30 })), branch: Some("main".to_string()), }; diff --git a/docs/user/cli-reference.md b/docs/user/cli-reference.md index 599ee13..bacfdf8 100644 --- a/docs/user/cli-reference.md +++ b/docs/user/cli-reference.md @@ -11,8 +11,8 @@ A reference for the `omnigraph` binary's command surface and `omnigraph.yaml` sc | `init` | `--schema ` → initialize a repo (also scaffolds `omnigraph.yaml` if missing) | | `load` | bulk load a branch (`--mode overwrite\|append\|merge`) | | `ingest` | branch-creating transactional load (`--from `) | -| `read` | run named query (params via `--params`, `--params-file`, or alias args) | -| `change` | run mutation query | +| `read` | run named query; source via `--query `, `-e`/`--query-string `, or `--alias ` (exactly one) | +| `change` | run mutation query; same `--query` / `-e` / `--alias` mutual-exclusion as `read` | | `snapshot` | print current snapshot (per-table version + row count) | | `export` | dump to JSONL on stdout (`--type T`, `--table K` filters) | | `branch create \| list \| delete \| merge` | branching ops | diff --git a/docs/user/cli.md b/docs/user/cli.md index ae8c152..1ecc2bb 100644 --- a/docs/user/cli.md +++ b/docs/user/cli.md @@ -10,6 +10,24 @@ omnigraph read --uri ./repo.omni --query ./queries.gq --name get_person --params omnigraph change --uri ./repo.omni --query ./queries.gq --name insert_person --params '{"name":"Mina","age":28}' ``` +For ad-hoc reads and mutations (REPLs, AI agents, one-off scripts), pass the +GQ source inline with `-e` / `--query-string` instead of a file path: + +```bash +omnigraph read --uri ./repo.omni \ + -e 'query find($name: String) { match { $p: Person { name: $name } } return { $p.name, $p.age } }' \ + --params '{"name":"Alice"}' + +omnigraph change --uri ./repo.omni \ + -e 'query add($name: String, $age: I32) { insert Person { name: $name, age: $age } }' \ + --params '{"name":"Inline","age":42}' +``` + +`-e` is mutually exclusive with `--query ` and `--alias `; exactly +one of the three must be provided. The inline source travels through the same +parser, lint, params binding, and commit machinery as a file-based query — +only the source loader changes. + ## Branching And Reviewable Data Flows ```bash diff --git a/docs/user/server.md b/docs/user/server.md index 6904e99..c8acfd1 100644 --- a/docs/user/server.md +++ b/docs/user/server.md @@ -9,9 +9,10 @@ Axum 0.8 + tokio + utoipa-generated OpenAPI. Single repo per process; deploy mul | GET | `/healthz` | none | — | `server_health` | | GET | `/openapi.json` | none | — | `server_openapi` (strips security if auth disabled) | | GET | `/snapshot?branch=` | bearer + `read` | snapshot of branch | `server_snapshot` | -| POST | `/read` | bearer + `read` | run named query | `server_read` | +| POST | `/read` | bearer + `read` | run named query (legacy field names `query_source`/`query_name`) | `server_read` | +| POST | `/query` | bearer + `read` | run inline read query (clean field names `query`/`name`; mutations → 400) | `server_query` | | POST | `/export` | bearer + `export` | NDJSON stream | `server_export` | -| POST | `/change` | bearer + `change` | mutation | `server_change` | +| POST | `/change` | bearer + `change` | mutation (`query`/`name`; accepts legacy `query_source`/`query_name` as serde aliases) | `server_change` | | GET | `/schema` | bearer + `read` | get current `.pg` source | `server_schema_get` | | POST | `/schema/apply` | bearer + `schema_apply` (target=`main`) | migrate | `server_schema_apply` | | POST | `/ingest` | bearer + `branch_create` (if new) + `change` | bulk load | `server_ingest` (32 MB body limit) | @@ -22,6 +23,32 @@ Axum 0.8 + tokio + utoipa-generated OpenAPI. Single repo per process; deploy mul | GET | `/commits?branch=` | bearer + `read` | list | `server_commit_list` | | GET | `/commits/{commit_id}` | bearer + `read` | show | `server_commit_show` | +## Inline read queries (`POST /query`) + +`POST /query` is the read-only, agent-friendly twin of `POST /read`. The +request body uses clean field names that match the CLI `-e` flag and the GQ +`query` keyword: + +```json +{ + "query": "query find($n: String) { match { $p: Person { name: $n } } return { $p.name } }", + "name": "find", + "params": { "n": "Alice" }, + "branch": "main", + "snapshot": null +} +``` + +Response shape is identical to `/read` (`ReadOutput`). If the inline source +contains mutations (`insert` / `update` / `delete`), the request is rejected +with HTTP 400 and an error pointing the caller at `POST /change` — the +read-only contract is enforced at the URL. + +`POST /change` accepts the same clean field names (`query`, `name`); the +legacy field names `query_source` and `query_name` continue to deserialize as +serde aliases so existing clients keep working without changes. `POST /read` +is byte-stable and unchanged. + ## Streaming Only `/export` streams (`application/x-ndjson`, MPSC channel + `Body::from_stream`). Everything else is buffered JSON. diff --git a/openapi.json b/openapi.json index b0ed1f2..c9fab6f 100644 --- a/openapi.json +++ b/openapi.json @@ -684,6 +684,73 @@ ] } }, + "/query": { + "post": { + "tags": [ + "queries" + ], + "summary": "Execute an inline read query (friendlier-named alternative to `POST /read`).", + "description": "Designed for ad-hoc exploration and AI-agent tool-use: short field\nnames (`query`, `name`) match the CLI `-e` flag and the GQ `query`\nkeyword. Mutations (`insert`/`update`/`delete`) are rejected with 400\n-- use `POST /change` for write queries. Otherwise behaves\nidentically to `POST /read`: same target semantics (branch xor\nsnapshot), same Cedar action (Read), same response shape.", + "operationId": "query", + "requestBody": { + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/QueryRequest" + } + } + }, + "required": true + }, + "responses": { + "200": { + "description": "Query results", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/ReadOutput" + } + } + } + }, + "400": { + "description": "Bad request - also returned when the query body contains mutations; use POST /change for write queries", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/ErrorOutput" + } + } + } + }, + "401": { + "description": "Unauthorized", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/ErrorOutput" + } + } + } + }, + "403": { + "description": "Forbidden", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/ErrorOutput" + } + } + } + } + }, + "security": [ + { + "bearer_token": [] + } + ] + } + }, "/read": { "post": { "tags": [ @@ -1103,7 +1170,7 @@ "ChangeRequest": { "type": "object", "required": [ - "query_source" + "query" ], "properties": { "branch": { @@ -1113,19 +1180,19 @@ ], "description": "Target branch. Defaults to `main`." }, - "params": { - "description": "JSON object whose keys match the mutation's declared parameters." - }, - "query_name": { + "name": { "type": [ "string", "null" ], - "description": "Name of the mutation to run when `query_source` declares multiple." + "description": "Name of the mutation to run when `query` declares multiple.\n\nAccepts the legacy field name `query_name` as a deserialization alias." }, - "query_source": { + "params": { + "description": "JSON object whose keys match the mutation's declared parameters." + }, + "query": { "type": "string", - "description": "GQ mutation source containing `insert`, `update`, or `delete` statements.\nMay declare multiple named mutations; pick one with `query_name`.", + "description": "GQ mutation source containing `insert`, `update`, or `delete` statements.\nMay declare multiple named mutations; pick one with `name`.\n\nAccepts the legacy field name `query_source` as a deserialization alias.", "example": "query insert_person($name: String, $age: I32) {\n insert Person { name: $name, age: $age }\n}" } } @@ -1453,6 +1520,44 @@ } } }, + "QueryRequest": { + "type": "object", + "description": "Inline read-query request for `POST /query`.\n\nFriendlier-named alternative to [`ReadRequest`] for ad-hoc reads and\nAI-agent integration. Mutations are rejected with 400 — use `POST\n/change` for write queries. Field names are deliberately short\n(`query`, `name`) to match the GQ keyword and the CLI `-e` flag.", + "required": [ + "query" + ], + "properties": { + "branch": { + "type": [ + "string", + "null" + ], + "description": "Branch to read from. Mutually exclusive with `snapshot`. Defaults to `main`." + }, + "name": { + "type": [ + "string", + "null" + ], + "description": "Name of the query to run when `query` declares multiple. Optional when\nonly one query is declared." + }, + "params": { + "description": "JSON object whose keys match the query's declared parameters." + }, + "query": { + "type": "string", + "description": "GQ read-query source. May declare one or more named queries; pick one\nwith `name` when more than one is declared. Mutations\n(`insert`/`update`/`delete`) get 400 — use `POST /change` instead.", + "example": "query get_person($name: String) {\n match {\n $p: Person { name: $name }\n }\n return { $p.name, $p.age }\n}" + }, + "snapshot": { + "type": [ + "string", + "null" + ], + "description": "Snapshot id to read from. Mutually exclusive with `branch`." + } + } + }, "ReadOutput": { "type": "object", "required": [