feat: inline query strings in CLI and HTTP server (#110)

* feat(MR-656): inline query strings in CLI and HTTP server

CLI:
- Add -e / --query-string <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 <ragnor.comerford@gmail.com>

* feat(MR-656): rename read/change to query/mutate with deprecation signals

HTTP server:
- Add POST /mutate as canonical write endpoint (pairs with POST /query).
- Mark POST /read and POST /change as deprecated. Three-channel signal:
  * OpenAPI: `deprecated: true` on the operation (every codegen flags
    the generated SDK method).
  * RFC 9745: response `Deprecation: true` header on every response.
  * RFC 8288: response `Link: </successor>; rel="successor-version"`
    pointing at /query and /mutate respectively.
- Share business logic across /mutate and /change via run_mutate(); the
  /change wrapper is the only place that adds the deprecation headers.
- ChangeRequest field aliases (query_source/query_name) preserved.
- AliasCommand serde now accepts `query`/`mutate` alongside `read`/`change`.

CLI:
- Promote `omnigraph query` / `omnigraph mutate` to top-level canonical
  subcommands (clap visible_alias keeps `omnigraph read` / `omnigraph
  change` working forever).
- Promote `omnigraph lint` / `omnigraph check` to top-level (was nested
  under `omnigraph query lint`, which is now a deprecated argv shim that
  rewrites to the canonical form).
- Argv-level preprocessing prints a one-line deprecation warning to
  stderr when any legacy spelling is used. Canonical names are silent.

Tests:
- Server: /mutate works, /change emits Deprecation+Link headers, /read
  emits Deprecation+Link headers, /query carries no deprecation signal.
- OpenAPI: /read and /change flagged deprecated; /query and /mutate not.
- CLI: canonical `lint` matches deprecated `query lint` / `query check`
  output; `read` / `change` print deprecation warnings.

Docs:
- cli.md: new canonical examples; "Deprecated names" migration table.
- cli-reference.md: top-level table updated; aliases.<name>.command
  accepts both legacy and canonical spellings.
- server.md: endpoint inventory shows /query and /mutate as canonical
  and /read and /change as deprecated; dedicated section explains the
  three-channel deprecation signal.
- og-cheet-sheet.md: use new `omnigraph lint` / `omnigraph check`.
- openapi.json regenerated.

Migration is purely cosmetic — every deprecated form continues to work
indefinitely; only the spelling changes.

Co-Authored-By: Ragnor Comerford <ragnor.comerford@gmail.com>

* fix(MR-656): address Devin Review findings on /query and /change

Two issues raised by Devin Review on PR #110:

1. `POST /query` mutation-rejection error pointed at the deprecated
   `/change` endpoint instead of the canonical `/mutate`. Fixed in
   three places: the runtime error message in `server_query`, the
   utoipa 400-response description, and the handler doc comment. The
   `QueryRequest` schema docstrings in `api.rs` got the same update so
   the openapi.json bodies match. Server and openapi tests updated.

2. `execute_change_remote` serialized `ChangeRequest` directly, which
   emits the new canonical field names `query` / `name` on the wire.
   `#[serde(alias = "query_source")]` only affects deserialization, so
   a newer CLI talking to an older server would have its `/change`
   POST body fail with "missing field: query_source". Fixed by
   extracting a `legacy_change_request_body` helper that hand-rolls
   the JSON with the legacy keys (`query_source` / `query_name`), the
   same byte-stable contract `execute_read_remote` already uses
   against `/read`. Added two unit tests on the helper to lock the
   wire shape in.

Co-Authored-By: Ragnor Comerford <ragnor.comerford@gmail.com>

* docs(dev): RFC 001 — inline + stored queries, envelope, MCP

Tracked artifact consolidating the design across MR-656 (this branch),
MR-976 (Phase 1 envelope hardening parent, with MR-977/978/979/980
sub-issues), and MR-969 (stored queries + MCP).

Sections:

* Two paths, one engine — inline `/query` + `/mutate` (this PR) coexist
  with stored `/queries/{name}` (MR-969). Same `run_query` / `run_mutate`
  backend (the fold-in landed in the previous commit).
* Request envelope ("before") — Idempotency-Key, If-Match, X-Deadline,
  X-Trace-Id, expect, dry_run, fields. Phase 1 ships the load-bearing
  subset on `/mutate`.
* Response envelope ("after") — audit_id, snapshot_id, commit_id, stats,
  warnings. Closes the provenance loop today's `ChangeOutput` leaves
  open.
* `.gq` pragmas — `@description`, `@returns`, `@mcp`. Source-of-truth
  for the stored-query agent contract; no separate YAML registry.
* Multi-graph MCP — per-graph `/graphs/{id}/mcp/tools` + `/mcp/invoke`.
  Token binds to one graph by default; cross-graph agents loop.
* Cedar split — `read`/`change` for inline, `invoke_query` for stored.
  Operators deny ad-hoc for agent groups while keeping curated tool
  list open.
* Rejected alternatives — per-env override files, compiled bundles,
  tool-name prefixing across graphs, body-field graph dispatch.

Index entry added under "Active Implementation Plans" so future agents
land on the RFC before touching queries / mutations / envelope code.
`scripts/check-agents-md.sh` clean (35 links, 34 docs).

* docs(server): clarify why run_query lacks AppState parameter

run_mutate takes state for workload admission; run_query doesn't because
reads aren't admission-gated today. Mark the asymmetry as intentional and
flag the two future events that would grow the signature: Phase 1's
`expect: { max_rows_scanned: N }` budget (MR-976) or per-actor admission
extending to stored-read invocations (MR-969). Prevents the natural
"make these symmetrical" follow-up.

* refactor(server): run_query / run_mutate take &ResolvedActor

Replace `Option<Extension<ResolvedActor>>` in the helpers with
`Option<&ResolvedActor>`. Saves MR-969's stored-query handler from
wrapping a bare actor in axum's `Extension(...)` before calling.
Handler signatures (`server_query`, `server_read`, `server_mutate`,
`server_change`) keep `Option<Extension<ResolvedActor>>` because that
is what axum injects, and unwrap at the call site with
`actor.as_ref().map(|Extension(actor)| actor)`.

Net: -13/+10 LOC, 89/0 server tests pass.

* docs(releases): v0.6.0 — describe inline + canonical-named queries (MR-656)

Extend the v0.6.0 release notes to cover the third piece of work landing
alongside the graph terminology rename and multi-graph server mode:
canonical-named `POST /query` and `POST /mutate` endpoints, the CLI's
new `-e/--query-string` flag, the top-level promotion of `lint` /
`check`, and the three-channel deprecation signal on `/read` and
`/change` (OpenAPI `deprecated: true` + RFC 9745 + RFC 8288).

Additions:

* Top blurb: "Two pieces" -> "Three pieces" with a bullet describing
  the rename + inline flow.
* Breaking Changes: new "Query / mutation rename" subsection covering
  the `ChangeRequest` field rename (with the back-compat serde aliases
  and the CLI's `legacy_change_request_body` byte-stable wire helper)
  and the `omnigraph query lint` -> `omnigraph lint` move.
* New: 5 bullets — the two endpoints, the CLI subcommands, the `-e`
  flag, the deprecation signal channels, the widened `aliases.<name>.command`
  vocabulary.
* User Impact: one bullet making explicit that the rename is cosmetic
  on the client side and migration is voluntary.
* Documentation: pointers to the updated `server.md` / `cli.md` /
  `cli-reference.md` and the new `docs/dev/rfc-001-queries-envelope-mcp.md`.

+15/-1 lines. `./scripts/check-agents-md.sh` clean.

* refactor(cli): demote `check` from visible_alias to deprecation shim

`omnigraph check` was a clap `visible_alias` on `lint`, advertised in
`--help` as an equivalent canonical name. Per MR-981 §6 (long-form
flags as canonical, short forms as visible aliases), visible aliases
on subcommand names hurt agent CX: agents emit either spelling
depending on training-data drift, and there's no length signal
pointing at the canonical name.

Changes:

* Remove `#[command(visible_alias = "check")]` from the `Lint` variant.
  `omnigraph --help` now shows only `lint`.
* Add bare `check` to `rewrite_deprecated_argv` so `omnigraph check
  <args>` still works — it rewrites to `omnigraph lint <args>` and
  emits a one-line stderr deprecation warning, matching the existing
  pattern for `read` / `change` / `query lint` / `query check`.
* Fix the nested `query check` shim to substitute `check` -> `lint` in
  the rewritten argv (previously it relied on `check` being a
  visible_alias to reach the `Lint` variant).
* New test `deprecated_check_top_level_rewrites_to_lint` covers: bare
  `check` produces identical stdout to `lint`, emits the deprecation
  warning, and `check` does NOT appear as an alias in `omnigraph
  --help`.
* Release notes updated to reflect the deprecation-shim treatment and
  cross-reference MR-981 §6 reasoning.

Cargo / Go users typing `check` still work indefinitely; one stderr
nudge per invocation teaches the canonical name. Agents see only
`lint` in `--help --json` so they emit one canonical form.

67/0 omnigraph-cli tests pass; 39 workspace test suites green.

---------

Co-authored-by: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
Co-authored-by: Ragnor Comerford <ragnor.comerford@gmail.com>
Co-authored-by: Ragnor Comerford <hello@ragnor.co>
This commit is contained in:
devin-ai-integration[bot] 2026-05-29 13:41:54 +02:00 committed by GitHub
parent e0f13b32c5
commit 1a4d2cee97
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
19 changed files with 2088 additions and 264 deletions

View file

@ -631,6 +631,202 @@ 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}"
);
}
/// Bare `omnigraph check` is NOT a clap `visible_alias` on `lint` (MR-981 §6:
/// visible aliases give agents two canonical names to emit interchangeably).
/// It's an argv-level shim: rewrites to `omnigraph lint`, prints a one-line
/// stderr deprecation warning, and produces identical stdout to the canonical
/// invocation. Cargo/Go users typing `check` keep working; help text shows
/// only `lint`.
#[test]
fn deprecated_check_top_level_rewrites_to_lint() {
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_check = output_success(
cli()
.arg("check")
.arg("--query")
.arg(&query_path)
.arg("--schema")
.arg(&schema_path)
.arg("--json"),
);
assert_eq!(stdout_string(&canonical), stdout_string(&deprecated_check));
let check_stderr = String::from_utf8(deprecated_check.stderr).unwrap();
assert!(
check_stderr.contains("`omnigraph check` is deprecated")
&& check_stderr.contains("`omnigraph lint`"),
"expected `omnigraph check` deprecation warning pointing at `omnigraph lint`; got: {check_stderr}"
);
// `check` must NOT appear in the canonical `omnigraph --help` output —
// agents copy the surface from help text and would otherwise emit both
// names interchangeably.
let help = cli().arg("--help").output().unwrap();
let stdout = String::from_utf8(help.stdout).unwrap();
let check_aliased = stdout
.lines()
.any(|line| line.trim_start().starts_with("lint") && line.contains("check"));
assert!(
!check_aliased,
"`check` must not be advertised as a visible alias of `lint`; help output: {stdout}"
);
}
/// `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_graph_via_positional_uri() {
let temp = tempdir().unwrap();
@ -1422,6 +1618,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 = graph_path(temp.path());
init_graph(&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 = graph_path(temp.path());
init_graph(&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 = graph_path(temp.path());
init_graph(&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 = graph_path(temp.path());
init_graph(&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();