diff --git a/crates/omnigraph-cli/src/main.rs b/crates/omnigraph-cli/src/main.rs index 742c75c..a7c762b 100644 --- a/crates/omnigraph-cli/src/main.rs +++ b/crates/omnigraph-cli/src/main.rs @@ -18,7 +18,7 @@ use omnigraph_compiler::{ }; use omnigraph_server::api::{ BranchCreateOutput, BranchCreateRequest, BranchDeleteOutput, BranchListOutput, - BranchMergeOutput, BranchMergeRequest, ChangeOutput, ChangeRequest, CommitListOutput, + BranchMergeOutput, BranchMergeRequest, ChangeOutput, CommitListOutput, CommitOutput, ErrorOutput, ExportRequest, IngestOutput, IngestRequest, ReadOutput, ReadRequest, SchemaApplyOutput, SchemaApplyRequest, SchemaOutput, SnapshotOutput, SnapshotTableOutput, commit_output, ingest_output, read_output, schema_apply_output, snapshot_payload, @@ -1636,6 +1636,33 @@ async fn execute_change( }) } +/// Build the JSON body for `POST /change` using the legacy wire shape. +/// +/// `ChangeRequest`'s Rust field names are now `query` / `name` (the canonical +/// wire shape going forward), but old `omnigraph-server` builds still require +/// the legacy `query_source` / `query_name` keys on `/change`. Hand-rolling +/// the JSON with the legacy names keeps a newer CLI talking to an older +/// server intact -- the same byte-stability contract we apply to +/// `execute_read_remote` against `/read`. +fn legacy_change_request_body( + query_source: &str, + query_name: Option<&str>, + branch: &str, + params_json: Option<&Value>, +) -> Value { + let mut body = serde_json::json!({ + "query_source": query_source, + "branch": branch, + }); + if let Some(name) = query_name { + body["query_name"] = Value::String(name.to_string()); + } + if let Some(params) = params_json { + body["params"] = params.clone(); + } + body +} + async fn execute_change_remote( client: &reqwest::Client, uri: &str, @@ -1649,12 +1676,12 @@ async fn execute_change_remote( client, Method::POST, remote_url(uri, "/change"), - Some(serde_json::to_value(ChangeRequest { - query: query_source.to_string(), - name: query_name.map(ToOwned::to_owned), - params: params_json.cloned(), - branch: Some(branch.to_string()), - })?), + Some(legacy_change_request_body( + query_source, + query_name, + branch, + params_json, + )), bearer_token, ) .await @@ -2627,14 +2654,62 @@ mod tests { use std::fs; use super::{ - DEFAULT_BEARER_TOKEN_ENV, apply_bearer_token, bearer_token_from_env_file, load_cli_config, - load_env_file_into_process, normalize_bearer_token, parse_env_assignment, - resolve_remote_bearer_token, + 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, }; use omnigraph_server::load_config; use reqwest::header::AUTHORIZATION; + use serde_json::json; use tempfile::tempdir; + #[test] + fn legacy_change_request_body_uses_legacy_field_names() { + // `execute_change_remote` hits `POST /change`, which old + // `omnigraph-server` builds deserialize as `ChangeRequest` with + // **required** `query_source` and optional `query_name` keys. + // Newer servers accept both spellings via serde alias, but a + // newer CLI must still emit the legacy keys on the wire so it + // can talk to an old server during a rolling upgrade. + let body = legacy_change_request_body( + "query insert_person($n: String) { insert Person { name: $n } }", + Some("insert_person"), + "main", + Some(&json!({ "n": "Alice" })), + ); + assert_eq!( + body["query_source"].as_str(), + Some("query insert_person($n: String) { insert Person { name: $n } }"), + ); + assert_eq!(body["query_name"].as_str(), Some("insert_person")); + assert_eq!(body["branch"].as_str(), Some("main")); + assert_eq!(body["params"]["n"].as_str(), Some("Alice")); + // Crucially, the **new** field names must NOT appear -- old + // servers would silently treat them as unknown fields and then + // fail on missing required `query_source`. + assert!( + body.get("query").is_none(), + "legacy /change body must not carry the renamed `query` key; got {body}" + ); + assert!( + body.get("name").is_none(), + "legacy /change body must not carry the renamed `name` key; got {body}" + ); + } + + #[test] + fn legacy_change_request_body_omits_optional_fields_when_unset() { + let body = legacy_change_request_body( + "query find() { match { $p: Person } return { $p.name } }", + None, + "main", + None, + ); + assert_eq!(body["branch"].as_str(), Some("main")); + assert!(body.get("query_name").is_none()); + assert!(body.get("params").is_none()); + } + #[test] fn apply_bearer_token_adds_header_when_configured() { let client = reqwest::Client::new(); diff --git a/crates/omnigraph-server/src/api.rs b/crates/omnigraph-server/src/api.rs index 9498c38..6699a6b 100644 --- a/crates/omnigraph-server/src/api.rs +++ b/crates/omnigraph-server/src/api.rs @@ -252,13 +252,15 @@ pub struct ReadRequest { /// /// 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. +/// /mutate` (or its deprecated alias `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. + /// (`insert`/`update`/`delete`) get 400 — use `POST /mutate` (or its + /// deprecated alias `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 diff --git a/crates/omnigraph-server/src/lib.rs b/crates/omnigraph-server/src/lib.rs index c9cd9d0..618677f 100644 --- a/crates/omnigraph-server/src/lib.rs +++ b/crates/omnigraph-server/src/lib.rs @@ -1026,7 +1026,7 @@ async fn server_read( 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 = 400, description = "Bad request - also returned when the query body contains mutations; use POST /mutate (or its deprecated alias POST /change) for write queries", body = ErrorOutput), (status = 401, description = "Unauthorized", body = ErrorOutput), (status = 403, description = "Forbidden", body = ErrorOutput), ), @@ -1037,9 +1037,10 @@ async fn server_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. +/// -- use `POST /mutate` (or its deprecated alias `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>, @@ -1080,7 +1081,7 @@ async fn server_query( .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 '{}' contains mutations (insert/update/delete); use POST /mutate for write queries", query_decl.name ))); } diff --git a/crates/omnigraph-server/tests/openapi.rs b/crates/omnigraph-server/tests/openapi.rs index f6968d3..70824fb 100644 --- a/crates/omnigraph-server/tests/openapi.rs +++ b/crates/omnigraph-server/tests/openapi.rs @@ -485,7 +485,7 @@ fn query_endpoint_documents_mutation_400() { 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"), + description.contains("mutations") || description.contains("POST /mutate"), "expected /query 400 response to mention mutation rejection, got: {description}" ); } diff --git a/crates/omnigraph-server/tests/server.rs b/crates/omnigraph-server/tests/server.rs index bbf6019..0b25840 100644 --- a/crates/omnigraph-server/tests/server.rs +++ b/crates/omnigraph-server/tests/server.rs @@ -2060,8 +2060,8 @@ async fn query_endpoint_rejects_mutation_with_400() { 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}" + err.contains("contains mutations") && err.contains("POST /mutate"), + "expected mutation-rejection message pointing at canonical /mutate, got: {err}" ); } diff --git a/openapi.json b/openapi.json index 0f335bb..8e36231 100644 --- a/openapi.json +++ b/openapi.json @@ -778,7 +778,7 @@ "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.", + "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 /mutate` (or its deprecated alias `POST /change`) for\nwrite queries. Otherwise behaves identically to `POST /read`: same\ntarget semantics (branch xor snapshot), same Cedar action (Read),\nsame response shape.", "operationId": "query", "requestBody": { "content": { @@ -802,7 +802,7 @@ } }, "400": { - "description": "Bad request - also returned when the query body contains mutations; use POST /change for write queries", + "description": "Bad request - also returned when the query body contains mutations; use POST /mutate (or its deprecated alias POST /change) for write queries", "content": { "application/json": { "schema": { @@ -1611,7 +1611,7 @@ }, "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.", + "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/mutate` (or its deprecated alias `POST /change`) for write queries.\nField names are deliberately short (`query`, `name`) to match the GQ\nkeyword and the CLI `-e` flag.", "required": [ "query" ], @@ -1635,7 +1635,7 @@ }, "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.", + "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 /mutate` (or its\ndeprecated alias `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": {