omnigraph/crates/omnigraph-cli/tests/cli_queries.rs
Andrew Altshuler bc2a989a7b
feat(cli)!: remove legacy data-plane addressing (--target, positional http→remote, --as-on-served) (#238)
* feat(cli): --server accepts a literal URL (RFC-011 Decision 2)

`resolve_server_flag` now treats a `--server` value containing `://` as a literal
base URL (trailing slash trimmed; `--graph` appends `/graphs/<id>`), bypassing the
operator-config `servers:` registry; a bare name still resolves through the
registry. This is the replacement the upcoming `--uri http(s)://` deprecation
points at, and a small ergonomic win on its own (`--server https://host` with no
config entry). Token resolution for a literal-URL server falls to the legacy
OMNIGRAPH_BEARER_TOKEN chain, same as a positional URL today.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* test(cli): address the parity-matrix arms with global --store/--server flags

Prep for removing the positional-http→remote dispatch. The parity harness
addressed both arms with a positional graph right after the verb
(`omnigraph <verb> <addr> <args…>`), which only parses for top-level verbs —
for nested subcommands (`schema show`, `branch list`, …) the address landed in
the subcommand slot and BOTH arms failed identically, so the test passed
vacuously (matching exit codes, never comparing output).

Address both arms with the global flags instead — local `--store <graph>`
(embedded), remote `--server <url>` (served) — appended after the verb + args,
valid regardless of nesting. The previously-vacuous nested-verb parity checks
now actually compare embedded vs remote (and pass — parity holds), and the
remote arm no longer relies on the positional-URL dispatch that's about to be
removed.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* feat(cli)!: --as on a served write is a hard error (was a silent no-op)

A served write resolves the actor server-side from the bearer token, so `--as`
could never set identity there — it was silently ignored. It now errors (in the
remote write factory, before any HTTP call), pointing the user at removing `--as`
or writing directly with `--store`. Reads don't carry `--as`, so this is
write-path only. BREAKING for any script that passed `--as` to a remote write
(it was a no-op, so behavior is unchanged except the now-explicit error).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* feat(cli)!: a positional/--uri http(s):// URL no longer dispatches to a server

Remote graphs must be addressed with `--server <url>` (or a named server / a
profile binding one). A positional or `--uri` `http(s)://` URL on a data verb now
errors instead of silently routing to the remote HTTP client — the scheme no
longer carries transport semantics. The discriminator is `via_server`: a remote
URL produced by a server scope is fine; a remote URL from a positional/`--uri`
source is rejected (`reject_positional_remote` in both GraphClient factories).

Storage verbs are unaffected — they already reject remote URIs through
`resolve_local_graph` with the existing "direct (storage-native)" error.

Migrated the gh-host keyed-credential system test to `--server <url>` (the literal
URL still prefix-matches the operator server for token resolution). BREAKING:
scripts addressing a server by a bare URL must switch to `--server <url>`.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* feat(cli)!: remove the --target flag (use --store / --profile / --server)

Removes the legacy named-graph flag and threads its parameter out of the whole
resolver chain. `--target` resolved a graph name through `omnigraph.yaml`'s
`graphs:` map; its replacements (`--store <uri>`, `--profile <name>`,
`--server <name>`) all ship.

- Drops the 22 `target` clap fields + the `--cluster` exclusion that named it.
- Threads `target`/`cli_target` out of `resolve_uri`/`resolve_cli_graph`/
  `resolve_local_graph`/`resolve_local_uri`/`resolve_storage_uri`/
  `resolve_remote_bearer_token`/`apply_server_flag`/`execute_query_lint`/
  `resolve_selected_graph`/`resolve_registry_selection_for_list`/
  `execute_queries_{validate,list}`, the two `GraphClient` factories, and
  `ScopeFlags`/`ResolvedScope`.
- Keeps the shared `OmnigraphConfig::resolve_target_uri` 3-arg (server boot uses
  it); the CLI passes None for the explicit-target arm. The `cli.graph` default
  (omnigraph.yaml bare-command fallback) is unchanged — its removal belongs to
  the omnigraph.yaml excision.
- Operator/file aliases that bind a `graph` name still work: the name is now
  resolved to a URI inline (a positional URI wins).
- Error messages and `--graph`/`--server`/`--store` help text no longer name
  `--target`; the queries-list selection hint points at `cli.graph`.

BREAKING. Tests updated (named-target resolution rewritten onto `cli.graph`;
positional-URI tests unchanged). Full omnigraph-cli suite green (228).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* docs(cli): drop --target and positional-http addressing; --as-on-served is an error

Update the user docs for the legacy data-plane addressing removals:
- the CLI `--target` flag is gone — address graphs with a positional URI,
  `--store`, `--profile`, or `--server <name|url>`;
- a positional `http(s)://` URI no longer dispatches to a server (use `--server`);
- `--as` on a served write is now rejected (was a silent no-op).

Touches cli/reference.md (addressing intro, capability table, error examples,
scopes), cli/index.md (the remote-read example → --server), operations/maintenance
+ policy, and the cluster docs' data-plane load guidance. The server's own
`--target` boot flag is unchanged (server.md untouched). Also fixes a pre-existing
broken maintenance link in search/indexes.md.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* fix(cli): --store is loudly exclusive with a positional URI / --server; test graphs→Served

Address two Greptile findings on the RFC-011 slices:
- Slice A (P1): `--store` combined with a positional URI silently dropped the URI
  (`scope.rs` did `store.or(uri)`); `--store` + `--server` errored with a
  misleading "positional URI" message. Now both combinations fail loudly with a
  declared `--store is exclusive with a positional URI and --server` error.
- Slice B (P2): the `command_capability` unit test never exercised the one
  Data→Served refinement (`graphs`); added the assertion so deleting that guard
  can't pass silently.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
2026-06-15 04:29:16 +03:00

526 lines
16 KiB
Rust

//! Stored-query commands and alias resolution.
//! Moved verbatim from tests/cli.rs in the modularization.
use serde_json::Value;
use tempfile::tempdir;
mod support;
use support::*;
#[test]
fn query_check_alias_matches_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 lint_output = output_success(
cli()
.arg("query")
.arg("lint")
.arg("--query")
.arg(&query_path)
.arg("--schema")
.arg(&schema_path)
.arg("--json"),
);
let check_output = output_success(
cli()
.arg("query")
.arg("check")
.arg("--query")
.arg(&query_path)
.arg("--schema")
.arg(&schema_path)
.arg("--json"),
);
assert_eq!(stdout_string(&lint_output), stdout_string(&check_output));
}
#[test]
fn read_alias_from_yaml_config_runs_with_kv_output() {
let temp = tempdir().unwrap();
let graph = graph_path(temp.path());
let config = temp.path().join("omnigraph.yaml");
let query = temp.path().join("aliases.gq");
init_graph(&graph);
load_fixture(&graph);
write_query_file(
&query,
&std::fs::read_to_string(fixture("test.gq")).unwrap(),
);
write_config(
&config,
&format!(
"{}aliases:\n owner:\n command: read\n query: aliases.gq\n name: get_person\n args: [name]\n format: kv\n",
local_yaml_config(&graph)
),
);
let output = output_success(
cli()
.arg("read")
.arg("--config")
.arg(&config)
.arg("--alias")
.arg("owner")
.arg("Alice"),
);
let stdout = stdout_string(&output);
assert!(stdout.contains("row 1"));
assert!(stdout.contains("p.name: Alice"));
}
#[test]
fn read_alias_uses_alias_target_without_cli_default_and_accepts_url_like_arg() {
let temp = tempdir().unwrap();
let graph = graph_path(temp.path());
let config = temp.path().join("omnigraph.yaml");
let query = temp.path().join("aliases.gq");
let data = temp.path().join("url-like.jsonl");
init_graph(&graph);
write_jsonl(
&data,
r#"{"type":"Person","data":{"name":"https://example.com","age":30}}"#,
);
output_success(
cli()
.arg("load")
.arg("--mode")
.arg("overwrite")
.arg("--data")
.arg(&data)
.arg(&graph),
);
write_query_file(
&query,
&std::fs::read_to_string(fixture("test.gq")).unwrap(),
);
write_config(
&config,
&format!(
"graphs:\n local:\n uri: '{}'\nquery:\n roots:\n - .\npolicy: {{}}\naliases:\n owner:\n command: read\n query: aliases.gq\n name: get_person\n args: [name]\n graph: local\n format: kv\n",
graph.to_string_lossy()
),
);
let output = output_success(
cli()
.arg("read")
.arg("--config")
.arg(&config)
.arg("--alias")
.arg("owner")
.arg("https://example.com"),
);
let stdout = stdout_string(&output);
assert!(stdout.contains("row 1"));
assert!(stdout.contains("p.name: https://example.com"));
}
#[test]
fn change_alias_from_yaml_config_persists_changes() {
let temp = tempdir().unwrap();
let graph = graph_path(temp.path());
let config = temp.path().join("omnigraph.yaml");
let query = temp.path().join("mutations.gq");
init_graph(&graph);
load_fixture(&graph);
write_query_file(
&query,
r#"
query insert_person($name: String, $age: I32) {
insert Person { name: $name, age: $age }
}
"#,
);
write_config(
&config,
&format!(
"{}aliases:\n add_person:\n command: change\n query: mutations.gq\n name: insert_person\n args: [name, age]\n",
local_yaml_config(&graph)
),
);
let output = output_success(
cli()
.arg("change")
.arg("--config")
.arg(&config)
.arg("--alias")
.arg("add_person")
.arg("Eve")
.arg("29")
.arg("--json"),
);
let payload: Value = serde_json::from_slice(&output.stdout).unwrap();
assert_eq!(payload["affected_nodes"], 1);
let verify = output_success(
cli()
.arg("read")
.arg(&graph)
.arg("--query")
.arg(fixture("test.gq"))
.arg("--name")
.arg("get_person")
.arg("--params")
.arg(r#"{"name":"Eve"}"#)
.arg("--json"),
);
let verify_payload: Value = serde_json::from_slice(&verify.stdout).unwrap();
assert_eq!(verify_payload["row_count"], 1);
}
#[test]
fn queries_validate_exits_zero_on_clean_registry() {
let graph = SystemGraph::loaded();
graph.write_query(
"find_person.gq",
"query find_person($name: String) { match { $p: Person { name: $name } } return { $p.age } }",
);
let config = graph.write_config(
"omnigraph.yaml",
&queries_test_config(
&graph.path().to_string_lossy(),
"find_person",
"find_person.gq",
),
);
let output = output_success(
cli()
.arg("queries")
.arg("validate")
.arg("--config")
.arg(&config),
);
let stdout = stdout_string(&output);
assert!(stdout.contains("OK"), "stdout:\n{stdout}");
}
#[test]
fn queries_validate_exits_nonzero_on_type_broken_query() {
let graph = SystemGraph::loaded();
// `Widget` is not in the fixture schema.
graph.write_query(
"ghost.gq",
"query ghost() { match { $w: Widget } return { $w.name } }",
);
let config = graph.write_config(
"omnigraph.yaml",
&queries_test_config(&graph.path().to_string_lossy(), "ghost", "ghost.gq"),
);
let output = output_failure(
cli()
.arg("queries")
.arg("validate")
.arg("--config")
.arg(&config),
);
let stdout = stdout_string(&output);
assert!(
stdout.contains("ghost"),
"validation should name the broken query; stdout:\n{stdout}"
);
}
#[test]
fn queries_list_prints_registered_query() {
let graph = SystemGraph::loaded();
graph.write_query(
"find_person.gq",
"query find_person($name: String) { match { $p: Person { name: $name } } return { $p.age } }",
);
// Exposed with an explicit tool name so the list shows the MCP suffix.
let config = graph.write_config(
"omnigraph.yaml",
&format!(
concat!(
"graphs:\n",
" local:\n",
" uri: '{}'\n",
" queries:\n",
" find_person:\n",
" file: ./find_person.gq\n",
" mcp: {{ expose: true, tool_name: lookup_person }}\n",
"cli:\n",
" graph: local\n",
"policy: {{}}\n",
),
graph.path().to_string_lossy().replace('\'', "''")
),
);
let output = output_success(
cli()
.arg("queries")
.arg("list")
.arg("--config")
.arg(&config),
);
let stdout = stdout_string(&output);
assert!(stdout.contains("find_person"), "stdout:\n{stdout}");
assert!(
stdout.contains("$name: String"),
"list should show typed params; stdout:\n{stdout}"
);
assert!(
stdout.contains("[mcp: lookup_person]"),
"list should show the MCP tool name for exposed queries; stdout:\n{stdout}"
);
}
#[test]
fn queries_list_requires_graph_selection_for_per_graph_only_registries() {
let graph = SystemGraph::loaded();
graph.write_query(
"find_person.gq",
"query find_person($name: String) { match { $p: Person { name: $name } } return { $p.age } }",
);
let config = graph.write_config(
"omnigraph.yaml",
&format!(
concat!(
"graphs:\n",
" local:\n",
" uri: '{}'\n",
" queries:\n",
" find_person:\n",
" file: ./find_person.gq\n",
"policy: {{}}\n",
),
graph.path().to_string_lossy().replace('\'', "''")
),
);
let output = output_failure(
cli()
.arg("queries")
.arg("list")
.arg("--config")
.arg(&config),
);
let stderr = String::from_utf8_lossy(&output.stderr);
assert!(
stderr.contains("local") && stderr.contains("set `cli.graph`"),
"error must name the graph and give a concrete selection hint; stderr:\n{stderr}"
);
}
#[test]
fn queries_list_without_graph_selection_lists_top_level_registry() {
let graph = SystemGraph::loaded();
graph.write_query(
"top_find.gq",
"query top_find($name: String) { match { $p: Person { name: $name } } return { $p.age } }",
);
let config = graph.write_config(
"omnigraph.yaml",
concat!(
"queries:\n",
" top_find:\n",
" file: ./top_find.gq\n",
"policy: {}\n",
),
);
let output = output_success(
cli()
.arg("queries")
.arg("list")
.arg("--config")
.arg(&config),
);
let stdout = stdout_string(&output);
assert!(stdout.contains("top_find"), "stdout:\n{stdout}");
}
#[test]
fn queries_list_unknown_cli_graph_errors() {
// `queries list` opens no graph URI, so unknown-graph validation can't ride
// along on URI resolution the way it does for every other command. An
// unknown `cli.graph` selection must still error (naming the graph) instead
// of silently falling back to the top-level registry and showing the wrong
// (or empty) catalog. (`--target` was removed; `cli.graph` drives selection.)
let graph = SystemGraph::loaded();
graph.write_query(
"find_person.gq",
"query find_person($name: String) { match { $p: Person { name: $name } } return { $p.age } }",
);
let config = graph.write_config(
"omnigraph.yaml",
&format!(
"graphs:\n local:\n uri: '{}'\n queries:\n find_person:\n file: ./find_person.gq\ncli:\n graph: nonexistent\npolicy: {{}}\n",
graph.path().to_string_lossy().replace('\'', "''"),
),
);
let output = output_failure(cli().arg("queries").arg("list").arg("--config").arg(&config));
let stderr = String::from_utf8_lossy(&output.stderr);
assert!(
stderr.contains("nonexistent"),
"error must name the unknown graph; stderr:\n{stderr}"
);
}
#[test]
fn queries_commands_reject_named_graph_with_populated_top_level_block() {
// A named graph (here via `cli.graph`) uses its own `graphs.<name>` block,
// so a populated top-level `queries:` block would be silently ignored — a
// config the server REFUSES to boot. `queries validate`/`list` must reject
// it too (matching boot) instead of validating/listing the per-graph block
// and giving a false green.
let graph = SystemGraph::loaded();
graph.write_query(
"find_person.gq",
"query find_person($name: String) { match { $p: Person { name: $name } } return { $p.age } }",
);
let config = graph.write_config(
"omnigraph.yaml",
&format!(
concat!(
"graphs:\n",
" local:\n",
" uri: '{}'\n",
" queries:\n",
" find_person:\n",
" file: ./find_person.gq\n",
"cli:\n",
" graph: local\n",
"queries:\n", // populated top-level block: the coherence violation
" legacy:\n",
" file: ./legacy.gq\n",
"policy: {{}}\n",
),
graph.path().to_string_lossy().replace('\'', "''")
),
);
// Both resolve `local` from cli.graph (no positional URI), so both must
// error and name the graph + the ignored block — like server boot does.
for sub in ["validate", "list"] {
let output = output_failure(cli().arg("queries").arg(sub).arg("--config").arg(&config));
let stderr = String::from_utf8_lossy(&output.stderr);
assert!(
stderr.contains("local") && stderr.contains("queries"),
"`queries {sub}` must reject a named graph with a populated top-level block; stderr:\n{stderr}"
);
}
}
#[test]
fn queries_validate_exits_nonzero_on_duplicate_tool_name() {
// Two exposed queries claiming one MCP tool name is a load-time
// collision — `queries validate` must fail (offline, before the engine
// opens) and name both queries plus the contested tool.
let graph = SystemGraph::loaded();
graph.write_query(
"a.gq",
"query a() { match { $p: Person } return { $p.name } }",
);
graph.write_query(
"b.gq",
"query b() { match { $p: Person } return { $p.name } }",
);
let config = graph.write_config(
"omnigraph.yaml",
&format!(
concat!(
"graphs:\n",
" local:\n",
" uri: '{}'\n",
" queries:\n",
" a:\n",
" file: ./a.gq\n",
" mcp: {{ expose: true, tool_name: dup }}\n",
" b:\n",
" file: ./b.gq\n",
" mcp: {{ expose: true, tool_name: dup }}\n",
"cli:\n",
" graph: local\n",
"policy: {{}}\n",
),
graph.path().to_string_lossy().replace('\'', "''")
),
);
let output = output_failure(
cli()
.arg("queries")
.arg("validate")
.arg("--config")
.arg(&config),
);
let stderr = String::from_utf8_lossy(&output.stderr);
assert!(
stderr.contains("dup") && stderr.contains("'a'") && stderr.contains("'b'"),
"duplicate tool name should be reported naming both queries; stderr:\n{stderr}"
);
}
#[test]
fn queries_validate_positional_uri_ignores_default_graph() {
// A positional URI is anonymous → the schema AND the registry both come
// from top-level, even when `cli.graph` names a graph whose per-graph
// queries would fail. Pins that the URI and registry can't diverge.
let graph = SystemGraph::loaded();
graph.write_query(
"clean.gq",
"query clean($name: String) { match { $p: Person { name: $name } } return { $p.age } }",
);
// `Widget` is not in the fixture schema — the default graph's per-graph
// query would break validate if it were (wrongly) selected.
graph.write_query(
"broken.gq",
"query broken() { match { $w: Widget } return { $w.name } }",
);
let config = graph.write_config(
"omnigraph.yaml",
concat!(
"cli:\n graph: prod\n",
"graphs:\n",
" prod:\n",
" uri: /nonexistent-prod.omni\n",
" queries:\n",
" broken:\n",
" file: ./broken.gq\n",
"queries:\n",
" clean:\n",
" file: ./clean.gq\n",
"policy: {}\n",
),
);
// Positional URI = the real loaded graph; selection is anonymous, so the
// CLEAN top-level registry validates (not prod's broken one).
let output = output_success(
cli()
.arg("queries")
.arg("validate")
.arg(graph.path())
.arg("--config")
.arg(&config),
);
let stdout = stdout_string(&output);
assert!(
stdout.contains("OK"),
"positional URI must validate the top-level registry, not the cli.graph default; stdout:\n{stdout}"
);
}