diff --git a/crates/omnigraph-cli/src/main.rs b/crates/omnigraph-cli/src/main.rs index 7585b10..742c75c 100644 --- a/crates/omnigraph-cli/src/main.rs +++ b/crates/omnigraph-cli/src/main.rs @@ -1,3 +1,4 @@ +use std::ffi::OsString; use std::fs; use std::io::{self, Write}; use std::path::Path; @@ -119,10 +120,25 @@ enum Command { #[command(subcommand)] command: SchemaCommand, }, - /// Query validation and linting - Query { - #[command(subcommand)] - command: QueryCommand, + /// Validate queries against a schema (offline) or repo (repo-backed). + /// + /// Replaces `omnigraph query lint` / `omnigraph query check`, which + /// are kept as deprecated argv-level shims (a one-line warning is + /// printed and the invocation is rewritten to `omnigraph lint`). + #[command(visible_alias = "check")] + Lint { + /// Repo URI + uri: Option, + #[arg(long)] + target: Option, + #[arg(long)] + config: Option, + #[arg(long)] + query: PathBuf, + #[arg(long)] + schema: Option, + #[arg(long)] + json: bool, }, /// Show repo snapshot Snapshot { @@ -159,8 +175,13 @@ enum Command { #[command(subcommand)] command: CommitCommand, }, - /// Execute a read query against a branch or snapshot - Read { + /// Execute a read query against a branch or snapshot. + /// + /// Canonical read endpoint. The previous name `omnigraph read` is + /// kept as a visible alias and prints a one-line deprecation warning + /// when used. Pairs with `omnigraph mutate` on the write side. + #[command(visible_alias = "read")] + Query { /// Repo URI #[arg(long)] uri: Option, @@ -192,8 +213,13 @@ enum Command { #[arg()] alias_args: Vec, }, - /// Execute a graph change query against a branch - Change { + /// Execute a graph mutation query against a branch. + /// + /// Canonical mutation endpoint. The previous name `omnigraph change` + /// is kept as a visible alias and prints a one-line deprecation + /// warning when used. Pairs with `omnigraph query` on the read side. + #[command(visible_alias = "change")] + Mutate { /// Repo URI #[arg(long)] uri: Option, @@ -378,26 +404,6 @@ enum SchemaCommand { }, } -#[derive(Debug, Subcommand)] -enum QueryCommand { - /// Validate queries and report higher-level drift warnings - #[command(visible_alias = "check")] - Lint { - /// Repo URI - uri: Option, - #[arg(long)] - target: Option, - #[arg(long)] - config: Option, - #[arg(long)] - query: PathBuf, - #[arg(long)] - schema: Option, - #[arg(long)] - json: bool, - }, -} - #[derive(Debug, Subcommand)] enum CommitCommand { /// List graph commits @@ -1703,10 +1709,50 @@ async fn execute_export_remote_to_writer( Ok(()) } +/// Rewrite deprecated CLI invocations into their canonical form. +/// +/// The current rename pass moves three subcommands: +/// - `omnigraph read` -> `omnigraph query` (visible_alias handles parsing; we just warn) +/// - `omnigraph change` -> `omnigraph mutate` (visible_alias handles parsing; we just warn) +/// - `omnigraph query lint` -> `omnigraph lint` (rewrite required; `query` is now the read-runner) +/// - `omnigraph query check` -> `omnigraph lint` (`check` is still a visible alias on `lint`) +/// +/// Returns the (possibly rewritten) argv that clap should parse. +fn rewrite_deprecated_argv(args: Vec) -> Vec { + if args.len() >= 3 { + let sub = args[1].to_str(); + let sub2 = args[2].to_str(); + if sub == Some("query") && matches!(sub2, Some("lint") | Some("check")) { + let suffix = sub2.unwrap(); + eprintln!( + "warning: `omnigraph query {suffix}` is deprecated; use `omnigraph lint` (alias: `omnigraph check`) instead" + ); + // Drop the leading `query` token, leaving e.g. `lint --query ./foo.gq`. + let mut out = Vec::with_capacity(args.len() - 1); + out.push(args[0].clone()); + out.extend(args[2..].iter().cloned()); + return out; + } + } + if let Some(sub) = args.get(1).and_then(|s| s.to_str()) { + match sub { + "read" => eprintln!( + "warning: `omnigraph read` is deprecated; use `omnigraph query` instead" + ), + "change" => eprintln!( + "warning: `omnigraph change` is deprecated; use `omnigraph mutate` instead" + ), + _ => {} + } + } + args +} + #[tokio::main] async fn main() -> Result<()> { color_eyre::install()?; let cli = { + let raw_args = rewrite_deprecated_argv(std::env::args_os().collect()); let matches = Cli::command() .arg( Arg::new("version") @@ -1715,7 +1761,7 @@ async fn main() -> Result<()> { .action(ArgAction::Version) .help("Print version"), ) - .get_matches(); + .get_matches_from(raw_args); Cli::from_arg_matches(&matches)? }; let http_client = build_http_client()?; @@ -2172,22 +2218,20 @@ async fn main() -> Result<()> { } } }, - Command::Query { command } => match command { - QueryCommand::Lint { - uri, - target, - config, - query, - schema, - json, - } => { - let config = load_cli_config(config.as_ref())?; - let output = - execute_query_lint(&config, uri, target.as_deref(), schema.as_ref(), &query) - .await?; - finish_query_lint(&output, json)?; - } - }, + Command::Lint { + uri, + target, + config, + query, + schema, + json, + } => { + let config = load_cli_config(config.as_ref())?; + let output = + execute_query_lint(&config, uri, target.as_deref(), schema.as_ref(), &query) + .await?; + finish_query_lint(&output, json)?; + } Command::Snapshot { uri, target, @@ -2257,7 +2301,7 @@ async fn main() -> Result<()> { .await?; } } - Command::Read { + Command::Query { uri, legacy_uri, target, @@ -2344,7 +2388,7 @@ async fn main() -> Result<()> { ); print_read_output(&output, format, &config)?; } - Command::Change { + Command::Mutate { uri, legacy_uri, target, diff --git a/crates/omnigraph-cli/tests/cli.rs b/crates/omnigraph-cli/tests/cli.rs index 63abd5a..d44fd32 100644 --- a/crates/omnigraph-cli/tests/cli.rs +++ b/crates/omnigraph-cli/tests/cli.rs @@ -631,6 +631,131 @@ query list_people() { assert_eq!(stdout_string(&lint_output), stdout_string(&check_output)); } +/// `omnigraph lint` is the canonical top-level lint command after the +/// query/mutate rename. `omnigraph query lint` and `omnigraph query check` +/// are kept as deprecated argv shims (warning + rewrite). All three must +/// produce identical stdout output. +#[test] +fn lint_top_level_matches_deprecated_query_lint_output() { + let temp = tempdir().unwrap(); + let schema_path = temp.path().join("schema.pg"); + let query_path = temp.path().join("queries.gq"); + write_file( + &schema_path, + r#" +node Person { + name: String +} +"#, + ); + write_query_file( + &query_path, + r#" +query list_people() { + match { $p: Person } + return { $p.name } +} +"#, + ); + + let canonical = output_success( + cli() + .arg("lint") + .arg("--query") + .arg(&query_path) + .arg("--schema") + .arg(&schema_path) + .arg("--json"), + ); + let deprecated_lint = output_success( + cli() + .arg("query") + .arg("lint") + .arg("--query") + .arg(&query_path) + .arg("--schema") + .arg(&schema_path) + .arg("--json"), + ); + let deprecated_check = output_success( + cli() + .arg("query") + .arg("check") + .arg("--query") + .arg(&query_path) + .arg("--schema") + .arg(&schema_path) + .arg("--json"), + ); + + assert_eq!(stdout_string(&canonical), stdout_string(&deprecated_lint)); + assert_eq!(stdout_string(&canonical), stdout_string(&deprecated_check)); + + // Canonical form must NOT emit the deprecation warning. + let canonical_stderr = String::from_utf8(canonical.stderr).unwrap(); + assert!( + !canonical_stderr.contains("deprecated"), + "`omnigraph lint` is canonical and must not warn; got stderr: {canonical_stderr}" + ); + + // Deprecated forms MUST emit the one-line warning, pointing at the + // new top-level `omnigraph lint`. + let lint_stderr = String::from_utf8(deprecated_lint.stderr).unwrap(); + assert!( + lint_stderr.contains("`omnigraph query lint` is deprecated") + && lint_stderr.contains("`omnigraph lint`"), + "expected deprecation warning pointing at `omnigraph lint`; got: {lint_stderr}" + ); + let check_stderr = String::from_utf8(deprecated_check.stderr).unwrap(); + assert!( + check_stderr.contains("`omnigraph query check` is deprecated") + && check_stderr.contains("`omnigraph lint`"), + "expected deprecation warning pointing at `omnigraph lint`; got: {check_stderr}" + ); +} + +/// `omnigraph read` and `omnigraph change` are kept as visible clap +/// aliases for the new canonical `query` / `mutate` subcommands, plus an +/// argv-level deprecation warning. The warning is emitted to stderr; the +/// command otherwise behaves identically to the canonical form. +#[test] +fn deprecated_read_and_change_subcommands_emit_warnings() { + // Both subcommands require `--query`/`--query-string`/`--alias`, so + // invoking them with no args will exit non-zero. That's fine -- + // we only care that the deprecation warning is printed before the + // argument-required error. + let output = cli().arg("read").output().unwrap(); + let stderr = String::from_utf8(output.stderr).unwrap(); + assert!( + stderr.contains("`omnigraph read` is deprecated") + && stderr.contains("`omnigraph query`"), + "expected `omnigraph read` deprecation warning; got: {stderr}" + ); + + let output = cli().arg("change").output().unwrap(); + let stderr = String::from_utf8(output.stderr).unwrap(); + assert!( + stderr.contains("`omnigraph change` is deprecated") + && stderr.contains("`omnigraph mutate`"), + "expected `omnigraph change` deprecation warning; got: {stderr}" + ); + + // Sanity check the inverse: the canonical names must NOT print the + // deprecation banner. + let output = cli().arg("query").arg("--help").output().unwrap(); + let stderr = String::from_utf8(output.stderr).unwrap(); + assert!( + !stderr.contains("deprecated"), + "`omnigraph query` is canonical and must not warn; got: {stderr}" + ); + let output = cli().arg("mutate").arg("--help").output().unwrap(); + let stderr = String::from_utf8(output.stderr).unwrap(); + assert!( + !stderr.contains("deprecated"), + "`omnigraph mutate` is canonical and must not warn; got: {stderr}" + ); +} + #[test] fn query_lint_can_use_local_repo_via_positional_uri() { let temp = tempdir().unwrap(); diff --git a/crates/omnigraph-server/src/config.rs b/crates/omnigraph-server/src/config.rs index 7145ff2..767abbd 100644 --- a/crates/omnigraph-server/src/config.rs +++ b/crates/omnigraph-server/src/config.rs @@ -80,7 +80,16 @@ pub struct PolicySettings { #[derive(Debug, Clone, Copy, Eq, PartialEq, Serialize, Deserialize)] #[serde(rename_all = "snake_case")] pub enum AliasCommand { + /// Read alias (canonical: `query`). The legacy spelling `read` is + /// kept as the variant name for back-compat with serialized configs + /// and external SDK callers; `query` is accepted on the wire via the + /// serde alias. + #[serde(alias = "query")] Read, + /// Mutation alias (canonical: `mutate`). The legacy spelling `change` + /// is kept as the variant name for back-compat; `mutate` is accepted + /// on the wire via the serde alias. + #[serde(alias = "mutate")] Change, } diff --git a/crates/omnigraph-server/src/lib.rs b/crates/omnigraph-server/src/lib.rs index 031d450..c9cd9d0 100644 --- a/crates/omnigraph-server/src/lib.rs +++ b/crates/omnigraph-server/src/lib.rs @@ -22,7 +22,7 @@ use axum::body::{Body, Bytes}; use axum::extract::DefaultBodyLimit; use axum::extract::{Extension, Path, Query, Request, State}; use axum::http::StatusCode; -use axum::http::header::{AUTHORIZATION, CONTENT_TYPE}; +use axum::http::header::{AUTHORIZATION, CONTENT_TYPE, HeaderName, HeaderValue}; use axum::middleware::{self, Next}; use axum::response::{IntoResponse, Response}; use axum::routing::{delete, get, post}; @@ -73,10 +73,13 @@ fn hash_bearer_token(token: &str) -> BearerTokenHash { paths( server_health, server_snapshot, - server_read, + // deprecated; the #[deprecated] attribute on the handler + // surfaces as `deprecated: true` on the OpenAPI operation. + #[allow(deprecated)] server_read, server_query, server_export, - server_change, + #[allow(deprecated)] server_change, + server_mutate, server_schema_apply, server_schema_get, server_ingest, @@ -631,9 +634,21 @@ pub fn build_app(state: AppState) -> Router { let protected = Router::new() .route("/snapshot", get(server_snapshot)) .route("/export", post(server_export)) - .route("/read", post(server_read)) + // /read and /change are kept indefinitely for back-compat; + // their handlers carry #[deprecated] so the OpenAPI operation is + // flagged and their responses include RFC 9745 Deprecation + + // RFC 8288 Link headers. Suppress the call-site warning for the + // route registration itself. + .route("/read", post({ + #[allow(deprecated)] + server_read + })) .route("/query", post(server_query)) - .route("/change", post(server_change)) + .route("/change", post({ + #[allow(deprecated)] + server_change + })) + .route("/mutate", post(server_mutate)) .route("/schema", get(server_schema_get)) .route("/schema/apply", post(server_schema_apply)) .route( @@ -905,6 +920,21 @@ async fn server_snapshot( Ok(Json(snapshot_payload(&branch, &snapshot))) } +/// Header values that flag a response as coming from a deprecated route +/// (RFC 9745 / RFC 8288) and point at the canonical successor. +fn deprecation_headers(successor_link: &'static str) -> [(HeaderName, HeaderValue); 2] { + [ + ( + HeaderName::from_static("deprecation"), + HeaderValue::from_static("true"), + ), + ( + HeaderName::from_static("link"), + HeaderValue::from_static(successor_link), + ), + ] +} + #[utoipa::path( post, path = "/read", @@ -912,25 +942,28 @@ async fn server_snapshot( operation_id = "read", request_body = ReadRequest, responses( - (status = 200, description = "Query results", body = ReadOutput), + (status = 200, description = "Query results (response includes `Deprecation: true` + `Link: ; rel=\"successor-version\"`)", body = ReadOutput), (status = 400, description = "Bad request", body = ErrorOutput), (status = 401, description = "Unauthorized", body = ErrorOutput), (status = 403, description = "Forbidden", body = ErrorOutput), ), security(("bearer_token" = [])), )] -/// Execute a GQ read query. +#[deprecated(note = "use POST /query instead; /read is kept indefinitely for byte-stable back-compat")] +/// **Deprecated** — use [`POST /query`](#tag/queries/operation/query) instead. /// -/// Runs the query in `query_source` against either a branch or a frozen -/// snapshot (mutually exclusive). When `query_source` defines multiple named -/// queries, pick one with `query_name`. `params` is a JSON object whose keys -/// match the parameters declared by the query. Returns rows as a JSON array -/// plus a `columns` list. Read-only. +/// Execute a GQ read query. Behavior is unchanged from prior releases; the +/// route is kept indefinitely for byte-stable back-compat. New integrations +/// should target `POST /query`, which has clean field names (`query` / +/// `name`) and a 400-on-mutation guard. Responses from this route include +/// `Deprecation: true` and `Link: ; rel="successor-version"` +/// headers per RFC 9745 / RFC 8288 so SDKs and proxies can surface the +/// signal. async fn server_read( State(state): State, actor: Option>, Json(request): Json, -) -> std::result::Result, ApiError> { +) -> std::result::Result<([(HeaderName, HeaderValue); 2], Json), ApiError> { if request.branch.is_some() && request.snapshot.is_some() { return Err(ApiError::bad_request( "read request may specify branch or snapshot, not both", @@ -979,7 +1012,10 @@ async fn server_read( .await .map_err(ApiError::from_omni)? }; - Ok(Json(api::read_output(selected_name, &target, result))) + Ok(( + deprecation_headers("; rel=\"successor-version\""), + Json(api::read_output(selected_name, &target, result)), + )) } #[utoipa::path( @@ -1126,33 +1162,15 @@ async fn server_export( .into_response()) } -#[utoipa::path( - post, - path = "/change", - tag = "mutations", - operation_id = "change", - request_body = ChangeRequest, - responses( - (status = 200, description = "Mutation results", body = ChangeOutput), - (status = 400, description = "Bad request", body = ErrorOutput), - (status = 401, description = "Unauthorized", body = ErrorOutput), - (status = 403, description = "Forbidden", body = ErrorOutput), - (status = 409, description = "Merge conflict", body = ErrorOutput), - (status = 429, description = "Per-actor admission cap exceeded; honor `Retry-After` header", body = ErrorOutput), - ), - security(("bearer_token" = [])), -)] -/// Apply a GQ mutation to a branch. -/// -/// Writes to the named `branch` (defaults to `main`). Mutations are atomic -/// per call and produce a new commit. Returns counts of nodes and edges -/// affected. **Destructive**: on success the branch is updated; rejected -/// mutations may still acquire locks briefly. Returns 409 on merge conflict. -async fn server_change( - State(state): State, +/// Shared implementation behind `POST /mutate` (canonical) and +/// `POST /change` (deprecated alias). Returns the bare `ChangeOutput`; +/// each route handler wraps it (the alias also attaches Deprecation +/// headers). +async fn run_mutate( + state: AppState, actor: Option>, - Json(request): Json, -) -> std::result::Result, ApiError> { + request: ChangeRequest, +) -> std::result::Result { let branch = request.branch.unwrap_or_else(|| "main".to_string()); let actor_arc = actor .as_ref() @@ -1201,13 +1219,84 @@ async fn server_change( .await .map_err(ApiError::from_omni)? }; - Ok(Json(ChangeOutput { + Ok(ChangeOutput { branch, query_name: selected_name, affected_nodes: result.affected_nodes, affected_edges: result.affected_edges, actor_id: actor_id.map(str::to_string), - })) + }) +} + +#[utoipa::path( + post, + path = "/change", + tag = "mutations", + operation_id = "change", + request_body = ChangeRequest, + responses( + (status = 200, description = "Mutation results (response includes `Deprecation: true` + `Link: ; rel=\"successor-version\"`)", body = ChangeOutput), + (status = 400, description = "Bad request", body = ErrorOutput), + (status = 401, description = "Unauthorized", body = ErrorOutput), + (status = 403, description = "Forbidden", body = ErrorOutput), + (status = 409, description = "Merge conflict", body = ErrorOutput), + (status = 429, description = "Per-actor admission cap exceeded; honor `Retry-After` header", body = ErrorOutput), + ), + security(("bearer_token" = [])), +)] +#[deprecated(note = "use POST /mutate instead; /change is kept indefinitely for back-compat")] +/// **Deprecated** — use [`POST /mutate`](#tag/mutations/operation/mutate) instead. +/// +/// Apply a GQ mutation to a branch. Behavior is unchanged; the route is +/// kept indefinitely for back-compat. New integrations should target +/// `POST /mutate`, which has identical semantics and a name that pairs +/// cleanly with `POST /query`. Responses from this route include +/// `Deprecation: true` and `Link: ; rel="successor-version"` +/// headers per RFC 9745 / RFC 8288 so SDKs and proxies can surface the +/// signal. +async fn server_change( + State(state): State, + actor: Option>, + Json(request): Json, +) -> std::result::Result<([(HeaderName, HeaderValue); 2], Json), ApiError> { + let output = run_mutate(state, actor, request).await?; + Ok(( + deprecation_headers("; rel=\"successor-version\""), + Json(output), + )) +} + +#[utoipa::path( + post, + path = "/mutate", + tag = "mutations", + operation_id = "mutate", + request_body = ChangeRequest, + responses( + (status = 200, description = "Mutation results", body = ChangeOutput), + (status = 400, description = "Bad request", body = ErrorOutput), + (status = 401, description = "Unauthorized", body = ErrorOutput), + (status = 403, description = "Forbidden", body = ErrorOutput), + (status = 409, description = "Merge conflict", body = ErrorOutput), + (status = 429, description = "Per-actor admission cap exceeded; honor `Retry-After` header", body = ErrorOutput), + ), + security(("bearer_token" = [])), +)] +/// Apply a GQ mutation to a branch (canonical mutation endpoint). +/// +/// Writes to the named `branch` (defaults to `main`). Mutations are atomic +/// per call and produce a new commit. Returns counts of nodes and edges +/// affected. **Destructive**: on success the branch is updated; rejected +/// mutations may still acquire locks briefly. Returns 409 on merge conflict. +/// +/// Pairs with `POST /query` (read-only). The legacy `POST /change` route +/// has identical semantics and is kept as a deprecated alias. +async fn server_mutate( + State(state): State, + actor: Option>, + Json(request): Json, +) -> std::result::Result, ApiError> { + Ok(Json(run_mutate(state, actor, request).await?)) } #[utoipa::path( diff --git a/crates/omnigraph-server/tests/openapi.rs b/crates/omnigraph-server/tests/openapi.rs index 8b922b1..f6968d3 100644 --- a/crates/omnigraph-server/tests/openapi.rs +++ b/crates/omnigraph-server/tests/openapi.rs @@ -162,6 +162,7 @@ const EXPECTED_PATHS: &[&str] = &[ "/query", "/export", "/change", + "/mutate", "/schema", "/schema/apply", "/ingest", @@ -228,6 +229,64 @@ fn openapi_change_is_post() { assert!(doc["paths"]["/change"]["post"].is_object()); } +#[test] +fn openapi_mutate_is_post() { + let doc = openapi_json(); + assert!(doc["paths"]["/mutate"]["post"].is_object()); +} + +// Deprecation flagging — `/read` and `/change` are kept indefinitely for +// back-compat but are flagged so OpenAPI codegens (typescript-fetch, +// openapi-generator, oapi-codegen, etc.) emit @deprecated on the generated +// SDK methods. The canonical successors `/query` and `/mutate` are not +// flagged. See `deprecation_headers` in `omnigraph-server/src/lib.rs` for +// the matching runtime signal (RFC 9745 + RFC 8288 headers). +#[test] +fn openapi_read_is_deprecated() { + let doc = openapi_json(); + assert_eq!( + doc["paths"]["/read"]["post"]["deprecated"], + serde_json::Value::Bool(true), + "/read must be flagged deprecated in OpenAPI; use /query instead" + ); +} + +#[test] +fn openapi_change_is_deprecated() { + let doc = openapi_json(); + assert_eq!( + doc["paths"]["/change"]["post"]["deprecated"], + serde_json::Value::Bool(true), + "/change must be flagged deprecated in OpenAPI; use /mutate instead" + ); +} + +#[test] +fn openapi_query_is_not_deprecated() { + let doc = openapi_json(); + let deprecated = doc["paths"]["/query"]["post"] + .get("deprecated") + .and_then(serde_json::Value::as_bool) + .unwrap_or(false); + assert!( + !deprecated, + "/query is the canonical read endpoint and must not be deprecated" + ); +} + +#[test] +fn openapi_mutate_is_not_deprecated() { + let doc = openapi_json(); + let deprecated = doc["paths"]["/mutate"]["post"] + .get("deprecated") + .and_then(serde_json::Value::as_bool) + .unwrap_or(false); + assert!( + !deprecated, + "/mutate is the canonical mutation endpoint and must not be deprecated" + ); +} + #[test] fn openapi_ingest_is_post() { let doc = openapi_json(); diff --git a/crates/omnigraph-server/tests/server.rs b/crates/omnigraph-server/tests/server.rs index d0e10b1..bbf6019 100644 --- a/crates/omnigraph-server/tests/server.rs +++ b/crates/omnigraph-server/tests/server.rs @@ -2065,6 +2065,163 @@ async fn query_endpoint_rejects_mutation_with_400() { ); } +#[tokio::test(flavor = "multi_thread")] +async fn mutate_endpoint_runs_inline_mutation() { + // Canonical mutation endpoint. Pairs with `/query` on the read side. + // Same wire shape as `/change`, no deprecation signal. + let (_temp, app) = app_for_loaded_repo().await; + + let request = json!({ + "query": MUTATION_QUERIES, + "name": "insert_person", + "params": { "name": "Mutie", "age": 30 }, + "branch": "main", + }); + let response = app + .clone() + .oneshot( + Request::builder() + .uri("/mutate") + .method(Method::POST) + .header("content-type", "application/json") + .body(Body::from(serde_json::to_vec(&request).unwrap())) + .unwrap(), + ) + .await + .unwrap(); + + assert_eq!(response.status(), StatusCode::OK); + // Canonical route is NOT deprecated; no Deprecation header expected. + assert!( + response.headers().get("deprecation").is_none(), + "POST /mutate must not advertise itself as deprecated" + ); + let body_bytes = to_bytes(response.into_body(), usize::MAX).await.unwrap(); + let body: Value = serde_json::from_slice(&body_bytes).unwrap(); + assert_eq!(body["affected_nodes"], 1); + assert_eq!(body["query_name"], "insert_person"); + assert_eq!(body["branch"], "main"); +} + +#[tokio::test(flavor = "multi_thread")] +async fn change_endpoint_emits_deprecation_headers() { + // `/change` is kept indefinitely for back-compat but flagged at runtime + // per RFC 9745 (`Deprecation: true`) + RFC 8288 (`Link: ; + // rel="successor-version"`). The OpenAPI side is covered by + // `openapi_change_is_deprecated` in tests/openapi.rs. + let (_temp, app) = app_for_loaded_repo().await; + + let request = json!({ + "query": MUTATION_QUERIES, + "name": "insert_person", + "params": { "name": "Legacyer", "age": 33 }, + "branch": "main", + }); + let response = app + .clone() + .oneshot( + Request::builder() + .uri("/change") + .method(Method::POST) + .header("content-type", "application/json") + .body(Body::from(serde_json::to_vec(&request).unwrap())) + .unwrap(), + ) + .await + .unwrap(); + + assert_eq!(response.status(), StatusCode::OK); + assert_eq!( + response + .headers() + .get("deprecation") + .and_then(|v| v.to_str().ok()), + Some("true"), + "POST /change must advertise `Deprecation: true` (RFC 9745)" + ); + assert_eq!( + response.headers().get("link").and_then(|v| v.to_str().ok()), + Some("; rel=\"successor-version\""), + "POST /change must point at /mutate via `Link` rel=successor-version (RFC 8288)" + ); +} + +#[tokio::test(flavor = "multi_thread")] +async fn read_endpoint_emits_deprecation_headers() { + // `/read` is kept indefinitely for byte-stable back-compat but flagged + // at runtime per RFC 9745 + RFC 8288. Successor is `/query`. + let (_temp, app) = app_for_loaded_repo().await; + + let request = ReadRequest { + query_source: fs::read_to_string(fixture("test.gq")).unwrap(), + query_name: Some("get_person".to_string()), + params: Some(json!({ "name": "Alice" })), + branch: Some("main".to_string()), + snapshot: None, + }; + let response = app + .clone() + .oneshot( + Request::builder() + .uri("/read") + .method(Method::POST) + .header("content-type", "application/json") + .body(Body::from(serde_json::to_vec(&request).unwrap())) + .unwrap(), + ) + .await + .unwrap(); + + assert_eq!(response.status(), StatusCode::OK); + assert_eq!( + response + .headers() + .get("deprecation") + .and_then(|v| v.to_str().ok()), + Some("true"), + "POST /read must advertise `Deprecation: true` (RFC 9745)" + ); + assert_eq!( + response.headers().get("link").and_then(|v| v.to_str().ok()), + Some("; rel=\"successor-version\""), + "POST /read must point at /query via `Link` rel=successor-version (RFC 8288)" + ); +} + +#[tokio::test(flavor = "multi_thread")] +async fn query_endpoint_does_not_emit_deprecation_headers() { + // Sanity check the inverse: the canonical `/query` endpoint must not + // carry deprecation signaling, so SDK codegens don't propagate a + // bogus `@deprecated` marker. + let (_temp, app) = app_for_loaded_repo().await; + + let request = 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 response = app + .clone() + .oneshot( + Request::builder() + .uri("/query") + .method(Method::POST) + .header("content-type", "application/json") + .body(Body::from(serde_json::to_vec(&request).unwrap())) + .unwrap(), + ) + .await + .unwrap(); + + assert_eq!(response.status(), StatusCode::OK); + assert!( + response.headers().get("deprecation").is_none(), + "POST /query is canonical and must not advertise itself as deprecated" + ); +} + #[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 diff --git a/docs/user/cli-reference.md b/docs/user/cli-reference.md index bacfdf8..a4e3dad 100644 --- a/docs/user/cli-reference.md +++ b/docs/user/cli-reference.md @@ -11,15 +11,15 @@ 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; source via `--query `, `-e`/`--query-string `, or `--alias ` (exactly one) | -| `change` | run mutation query; same `--query` / `-e` / `--alias` mutual-exclusion as `read` | +| `query` (alias: `read`) | run named read query; source via `--query `, `-e`/`--query-string `, or `--alias ` (exactly one). `read` is the deprecated previous name and prints a one-line warning to stderr | +| `mutate` (alias: `change`) | run mutation query; same `--query` / `-e` / `--alias` mutual-exclusion as `query`. `change` is the deprecated previous name and prints a one-line warning to stderr | | `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 | | `commit list \| show` | inspect commit graph | | `run list \| show \| publish \| abort` | transactional run ops | | `schema plan \| apply \| show (alias: get)` | migrations | -| `query lint \| check` | offline / repo-backed validation | +| `lint` (alias: `check`) | offline / repo-backed query validation. Replaces `query lint` / `query check`, which are kept as deprecated argv-level shims that print a one-line warning and rewrite to `omnigraph lint` | | `optimize` | non-destructive Lance compaction | | `cleanup --keep N --older-than 7d --confirm` | destructive version GC | | `embed` | offline JSONL embedding pipeline | @@ -49,7 +49,10 @@ auth: env_file: ./.env.omni aliases: : - command: read|change + # accepted values: `read` / `query` (read alias), `change` / `mutate` + # (write alias). `query` and `mutate` are recommended; `read` and + # `change` remain accepted forever for back-compat. + command: read|change|query|mutate query: name: args: [, …] @@ -60,7 +63,7 @@ policy: file: ./policy.yaml ``` -## Output formats (read command) +## Output formats (`query` command, alias: `read`) - `json` — pretty-printed object with metadata + rows - `jsonl` — one metadata line then one JSON object per row diff --git a/docs/user/cli.md b/docs/user/cli.md index 1ecc2bb..d680cf6 100644 --- a/docs/user/cli.md +++ b/docs/user/cli.md @@ -6,19 +6,26 @@ omnigraph init --schema ./schema.pg ./repo.omni omnigraph load --data ./data.jsonl --mode overwrite ./repo.omni omnigraph snapshot ./repo.omni --branch main --json -omnigraph read --uri ./repo.omni --query ./queries.gq --name get_person --params '{"name":"Alice"}' -omnigraph change --uri ./repo.omni --query ./queries.gq --name insert_person --params '{"name":"Mina","age":28}' +omnigraph query --uri ./repo.omni --query ./queries.gq --name get_person --params '{"name":"Alice"}' +omnigraph mutate --uri ./repo.omni --query ./queries.gq --name insert_person --params '{"name":"Mina","age":28}' ``` +`omnigraph query` is the canonical read command (pairs with `POST /query`); +`omnigraph mutate` is the canonical write command (pairs with `POST /mutate`). +The previous names `omnigraph read` and `omnigraph change` keep working as +visible aliases — invocations emit a one-line deprecation warning to stderr +and otherwise behave identically. See [Deprecated names](#deprecated-names) +for the migration table. + 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 \ +omnigraph query --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 \ +omnigraph mutate --uri ./repo.omni \ -e 'query add($name: String, $age: I32) { insert Person { name: $name, age: $age } }' \ --params '{"name":"Inline","age":42}' ``` @@ -52,7 +59,7 @@ omnigraph-server ./repo.omni --bind 127.0.0.1:8080 Read through the HTTP API: ```bash -omnigraph read \ +omnigraph query \ --target http://127.0.0.1:8080 \ --query ./queries.gq \ --name get_person \ @@ -65,8 +72,8 @@ and configure the matching `bearer_token_env` in `omnigraph.yaml`. ## Runs, Policy, And Diagnostics ```bash -omnigraph query lint --query ./queries.gq --schema ./schema.pg --json -omnigraph query check --query ./queries.gq ./repo.omni --json +omnigraph lint --query ./queries.gq --schema ./schema.pg --json +omnigraph check --query ./queries.gq ./repo.omni --json omnigraph schema plan --schema ./next.pg ./repo.omni --json omnigraph schema apply --schema ./next.pg ./repo.omni --json @@ -116,3 +123,21 @@ The config file can also define: When policy is enabled, `schema apply` is authorized through the `schema_apply` action and is typically limited to admins on protected `main`. + +## Deprecated names + +The CLI was renamed to align with the HTTP server's canonical endpoint +names (`POST /query`, `POST /mutate`) and the `query` keyword in the GQ +language. The previous spellings keep working forever; invocations emit a +one-line warning to stderr and otherwise behave identically. + +| Old (deprecated) | New (canonical) | Migration | +|--------------------------|---------------------|----------------------------------------------------------| +| `omnigraph read` | `omnigraph query` | Same flags and behavior. `read` is a visible clap alias. | +| `omnigraph change` | `omnigraph mutate` | Same flags and behavior. `change` is a visible clap alias. | +| `omnigraph query lint` | `omnigraph lint` | Same flags. The argv-level shim rewrites `query lint` to `lint`. | +| `omnigraph query check` | `omnigraph check` | `check` is a visible alias of `omnigraph lint`. | + +The `command:` field in `aliases.` in `omnigraph.yaml` accepts both +`read` / `change` (legacy) and `query` / `mutate` (canonical); the two +spellings are interchangeable on the wire via serde aliases. diff --git a/docs/user/server.md b/docs/user/server.md index c8acfd1..e0db78f 100644 --- a/docs/user/server.md +++ b/docs/user/server.md @@ -9,10 +9,11 @@ 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 (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 | `/query` | bearer + `read` | run inline read query (canonical; clean field names `query`/`name`; mutations → 400) | `server_query` | +| POST | `/read` | bearer + `read` | **deprecated** alias of `/query` for legacy clients (legacy field names `query_source`/`query_name`, byte-stable response); response carries `Deprecation: true` + `Link: ; rel="successor-version"` | `server_read` | | POST | `/export` | bearer + `export` | NDJSON stream | `server_export` | -| POST | `/change` | bearer + `change` | mutation (`query`/`name`; accepts legacy `query_source`/`query_name` as serde aliases) | `server_change` | +| POST | `/mutate` | bearer + `change` | mutation query (canonical; `query`/`name`; accepts legacy `query_source`/`query_name` as serde aliases) | `server_mutate` | +| POST | `/change` | bearer + `change` | **deprecated** alias of `/mutate` for legacy clients; response carries `Deprecation: true` + `Link: ; rel="successor-version"` | `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) | @@ -41,13 +42,33 @@ request body uses clean field names that match the CLI `-e` flag and the GQ 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 +with HTTP 400 and an error pointing the caller at `POST /mutate` — 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. +`POST /mutate` is the canonical mutation endpoint. It 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. + +## Deprecated names (`/read`, `/change`) + +`POST /read` and `POST /change` are kept for back-compat indefinitely — they +are byte-stable on the request side and otherwise behave identically to +`/query` / `/mutate`. They are flagged as deprecated through three independent +channels: + +- **OpenAPI**: the operations carry `deprecated: true` in `openapi.json`, so + every OpenAPI codegen (typescript-fetch, openapi-generator, oapi-codegen, + …) emits a `@deprecated` marker on the generated SDK method. +- **Response headers (RFC 9745)**: every response carries `Deprecation: true`. +- **Response headers (RFC 8288)**: every response carries a `Link` header + pointing at the canonical successor: + `Link: ; rel="successor-version"` for `/read`, and + `Link: ; rel="successor-version"` for `/change`. SDKs and HTTP + proxies can pick the successor up automatically. + +Migration is purely cosmetic on the client side — swap the URL path, leave +the request body and response handling alone. ## Streaming @@ -61,8 +82,8 @@ Uniform `ErrorOutput { error, code?, merge_conflicts[], manifest_conflict? }` wi caller's pre-write view of one table's manifest version was stale. `ManifestConflictOutput { table_key, expected, actual }` tells the client which table to refresh and retry. This is the conflict shape produced by -concurrent `/change` or `/ingest` calls landing the same `(table, branch)` -race. +concurrent `/mutate` (or its `/change` alias) or `/ingest` calls landing +the same `(table, branch)` race. HTTP status codes used: 200, 400, 401, 403, 404, 409, 429, 500. @@ -88,10 +109,11 @@ actors are unaffected. Cedar policy authorization runs **before** admission accounting so denied requests don't consume admission slots. -Today admission gates every mutating handler: `/change`, `/ingest`, -`/branches/{create,delete,merge}`, and `/schema/apply`. Read-only -endpoints (`/snapshot`, `/read`, `/export`, `/branches` GET, `/commits`, -`/schema` GET) are not admission-gated. +Today admission gates every mutating handler: `/mutate` (and its +deprecated alias `/change`), `/ingest`, `/branches/{create,delete,merge}`, +and `/schema/apply`. Read-only endpoints (`/snapshot`, `/query`, `/read`, +`/export`, `/branches` GET, `/commits`, `/schema` GET) are not +admission-gated. ## Body limits @@ -120,8 +142,9 @@ See [deployment.md](deployment.md) for token-source operational details. ## Not implemented (by design or "TBD") - CORS — not configured; add `tower_http::cors` if needed. -- Rate limiting — per-actor admission control gates `/change`, `/ingest`, - `/branches/{create,delete,merge}`, `/schema/apply` (see "Per-actor +- Rate limiting — per-actor admission control gates `/mutate` (alias + `/change`), `/ingest`, `/branches/{create,delete,merge}`, + `/schema/apply` (see "Per-actor admission control" above). No global rate limiter is configured; add `tower_http::limit` if a graph-wide cap is needed. - Pagination — none (commits/branches return everything; export streams). diff --git a/og-cheet-sheet.md b/og-cheet-sheet.md index 8ae6f5c..2cb4d76 100644 --- a/og-cheet-sheet.md +++ b/og-cheet-sheet.md @@ -5,23 +5,27 @@ Use an explicit schema file: ```bash -omnigraph query lint --query ./queries.gq --schema ./schema.pg --json -omnigraph query check --query ./queries.gq --schema ./schema.pg +omnigraph lint --query ./queries.gq --schema ./schema.pg --json +omnigraph check --query ./queries.gq --schema ./schema.pg ``` Use a local or `s3://` repo target: ```bash -omnigraph query lint --query ./queries.gq ./repo.omni --json -omnigraph query check --query ./queries.gq s3://bucket/repo +omnigraph lint --query ./queries.gq ./repo.omni --json +omnigraph check --query ./queries.gq s3://bucket/repo ``` Use `omnigraph.yaml` target resolution: ```bash -omnigraph query lint --query ./queries.gq --target local --config ./omnigraph.yaml +omnigraph lint --query ./queries.gq --target local --config ./omnigraph.yaml ``` +> The previous `omnigraph query lint` / `omnigraph query check` spellings +> are kept as deprecated argv shims that print a one-line warning to +> stderr and rewrite to the canonical `omnigraph lint` / `omnigraph check`. + ## What It Checks - parses every query in the file diff --git a/openapi.json b/openapi.json index c9fab6f..0f335bb 100644 --- a/openapi.json +++ b/openapi.json @@ -312,8 +312,8 @@ "tags": [ "mutations" ], - "summary": "Apply a GQ mutation to a branch.", - "description": "Writes to the named `branch` (defaults to `main`). Mutations are atomic\nper call and produce a new commit. Returns counts of nodes and edges\naffected. **Destructive**: on success the branch is updated; rejected\nmutations may still acquire locks briefly. Returns 409 on merge conflict.", + "summary": "**Deprecated** — use [`POST /mutate`](#tag/mutations/operation/mutate) instead.", + "description": "Apply a GQ mutation to a branch. Behavior is unchanged; the route is\nkept indefinitely for back-compat. New integrations should target\n`POST /mutate`, which has identical semantics and a name that pairs\ncleanly with `POST /query`. Responses from this route include\n`Deprecation: true` and `Link: ; rel=\"successor-version\"`\nheaders per RFC 9745 / RFC 8288 so SDKs and proxies can surface the\nsignal.", "operationId": "change", "requestBody": { "content": { @@ -327,7 +327,7 @@ }, "responses": { "200": { - "description": "Mutation results", + "description": "Mutation results (response includes `Deprecation: true` + `Link: ; rel=\"successor-version\"`)", "content": { "application/json": { "schema": { @@ -387,6 +387,7 @@ } } }, + "deprecated": true, "security": [ { "bearer_token": [] @@ -684,6 +685,93 @@ ] } }, + "/mutate": { + "post": { + "tags": [ + "mutations" + ], + "summary": "Apply a GQ mutation to a branch (canonical mutation endpoint).", + "description": "Writes to the named `branch` (defaults to `main`). Mutations are atomic\nper call and produce a new commit. Returns counts of nodes and edges\naffected. **Destructive**: on success the branch is updated; rejected\nmutations may still acquire locks briefly. Returns 409 on merge conflict.\n\nPairs with `POST /query` (read-only). The legacy `POST /change` route\nhas identical semantics and is kept as a deprecated alias.", + "operationId": "mutate", + "requestBody": { + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/ChangeRequest" + } + } + }, + "required": true + }, + "responses": { + "200": { + "description": "Mutation results", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/ChangeOutput" + } + } + } + }, + "400": { + "description": "Bad request", + "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" + } + } + } + }, + "409": { + "description": "Merge conflict", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/ErrorOutput" + } + } + } + }, + "429": { + "description": "Per-actor admission cap exceeded; honor `Retry-After` header", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/ErrorOutput" + } + } + } + } + }, + "security": [ + { + "bearer_token": [] + } + ] + } + }, "/query": { "post": { "tags": [ @@ -756,8 +844,8 @@ "tags": [ "queries" ], - "summary": "Execute a GQ read query.", - "description": "Runs the query in `query_source` against either a branch or a frozen\nsnapshot (mutually exclusive). When `query_source` defines multiple named\nqueries, pick one with `query_name`. `params` is a JSON object whose keys\nmatch the parameters declared by the query. Returns rows as a JSON array\nplus a `columns` list. Read-only.", + "summary": "**Deprecated** — use [`POST /query`](#tag/queries/operation/query) instead.", + "description": "Execute a GQ read query. Behavior is unchanged from prior releases; the\nroute is kept indefinitely for byte-stable back-compat. New integrations\nshould target `POST /query`, which has clean field names (`query` /\n`name`) and a 400-on-mutation guard. Responses from this route include\n`Deprecation: true` and `Link: ; rel=\"successor-version\"`\nheaders per RFC 9745 / RFC 8288 so SDKs and proxies can surface the\nsignal.", "operationId": "read", "requestBody": { "content": { @@ -771,7 +859,7 @@ }, "responses": { "200": { - "description": "Query results", + "description": "Query results (response includes `Deprecation: true` + `Link: ; rel=\"successor-version\"`)", "content": { "application/json": { "schema": { @@ -811,6 +899,7 @@ } } }, + "deprecated": true, "security": [ { "bearer_token": []