mirror of
https://github.com/ModernRelay/omnigraph.git
synced 2026-06-21 02:28:07 +02:00
feat(cli)!: excise omnigraph.yaml from the CLI; policy/queries tooling reads --cluster (#251)
The server already dropped omnigraph.yaml (cluster-only boot). This removes the CLI's last use of the legacy `OmnigraphConfig`: graphs are addressed only via `--store`/`--server`/`--cluster`/`--profile`/operator defaults, and actor, output format, and bearer credentials come from `~/.omnigraph/config.yaml`. After this change no CLI command reads `omnigraph.yaml` except `config migrate`. Resolvers (helpers.rs): drop every legacy fallback — - `resolve_actor` → `--as` > `operator.actor` (no `cli.actor`); - `resolve_read_format` → `--json`/`--format` > alias > `defaults.output`; - `resolve_branch`/`resolve_read_target` → `--branch` > alias > "main"; - `resolve_uri`/`resolve_cli_graph` → scope path only; an absent address is a loud error; - `resolve_remote_bearer_token` → operator keyed chain + `OMNIGRAPH_BEARER_TOKEN`. `GraphClient::resolve`/`resolve_with_policy` drop the `&OmnigraphConfig` param; direct-store access carries no Cedar policy (policy lives in the cluster/server). Flags (cli.rs): remove `--config` from every data/query command; it stays only on `cluster *` (the cluster dir) and `config migrate` (the legacy path). Re-home control-plane tooling to `--cluster` (RFC-011): - `policy validate|test|explain` source the Cedar bundle from the cluster's applied policies; `--graph` picks a graph's bundle; `policy test` takes `--tests <file>`; - `queries list|validate` source the registry + schemas from the cluster serving snapshot; `--graph` scopes to one graph; - `lint` requires `--schema` (offline) or a direct/cluster graph target; - `schema plan`/`lint` route their graph-target through the shared direct-scope resolver so `--store`/`--profile`/`defaults.store` addressing works. Tests migrate from `omnigraph.yaml` fixtures to `--store`/operator-config/ `--cluster` (converged-cluster fixtures); the now-impossible command-path RFC-008 tests are deleted (`config migrate` coverage kept). The `OmnigraphConfig` type, `load_config`/deprecation machinery, and `config migrate` are removed in a follow-up. Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
Andrew Altshuler
GitHub
parent
8b01c6e547
commit
0bee746a31
15 changed files with 1464 additions and 2262 deletions
|
|
@ -94,90 +94,91 @@ fn alias_unknown_name_errors_listing_defined() {
|
|||
);
|
||||
}
|
||||
|
||||
// RFC-011: `queries validate`/`list` source the registry + schemas from a
|
||||
// converged cluster's applied state (`--cluster <dir>`), not omnigraph.yaml.
|
||||
|
||||
/// Build a converged single-graph cluster (id `knowledge`) with one stored
|
||||
/// query. `query_block` is the YAML under the graph's `queries:` key.
|
||||
fn converged_cluster_with_query(query_file: &str, query_src: &str, query_block: &str) -> tempfile::TempDir {
|
||||
let temp = tempdir().unwrap();
|
||||
let dir = temp.path();
|
||||
std::fs::copy(fixture("test.pg"), dir.join("graph.pg")).unwrap();
|
||||
write_query_file(&dir.join(query_file), query_src);
|
||||
std::fs::write(
|
||||
dir.join("cluster.yaml"),
|
||||
format!(
|
||||
"version: 1\nmetadata:\n name: sys\nstate:\n backend: cluster\n lock: true\n\
|
||||
graphs:\n knowledge:\n schema: ./graph.pg\n queries:\n{query_block}"
|
||||
),
|
||||
)
|
||||
.unwrap();
|
||||
output_success(cli().arg("cluster").arg("import").arg("--config").arg(dir));
|
||||
output_success(cli().arg("cluster").arg("apply").arg("--config").arg(dir));
|
||||
temp
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn queries_validate_exits_zero_on_clean_registry() {
|
||||
let graph = SystemGraph::loaded();
|
||||
graph.write_query(
|
||||
let cluster = converged_cluster_with_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",
|
||||
),
|
||||
" find_person:\n file: ./find_person.gq\n",
|
||||
);
|
||||
let output = output_success(
|
||||
cli()
|
||||
.arg("queries")
|
||||
.arg("validate")
|
||||
.arg("--config")
|
||||
.arg(&config),
|
||||
.arg("--cluster")
|
||||
.arg(cluster.path()),
|
||||
);
|
||||
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",
|
||||
fn cluster_import_rejects_a_type_broken_query() {
|
||||
// In the cluster model a stored query is type-checked at the cluster
|
||||
// boundary (import/apply), so a broken query can never reach the applied
|
||||
// state `queries validate` reads — the gate is upstream. `Widget` is not in
|
||||
// the fixture schema, so import must reject it, naming the query.
|
||||
let temp = tempdir().unwrap();
|
||||
let dir = temp.path();
|
||||
std::fs::copy(fixture("test.pg"), dir.join("graph.pg")).unwrap();
|
||||
write_query_file(
|
||||
&dir.join("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"),
|
||||
std::fs::write(
|
||||
dir.join("cluster.yaml"),
|
||||
"version: 1\nmetadata:\n name: sys\nstate:\n backend: cluster\n lock: true\n\
|
||||
graphs:\n knowledge:\n schema: ./graph.pg\n queries:\n ghost:\n file: ./ghost.gq\n",
|
||||
)
|
||||
.unwrap();
|
||||
let output = output_failure(cli().arg("cluster").arg("import").arg("--config").arg(dir));
|
||||
let combined = format!(
|
||||
"{}{}",
|
||||
stdout_string(&output),
|
||||
String::from_utf8_lossy(&output.stderr)
|
||||
);
|
||||
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}"
|
||||
combined.contains("ghost"),
|
||||
"cluster import must reject the broken query, naming it; got:\n{combined}"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn queries_list_prints_registered_query() {
|
||||
let graph = SystemGraph::loaded();
|
||||
graph.write_query(
|
||||
let cluster = converged_cluster_with_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('\'', "''")
|
||||
),
|
||||
" find_person:\n file: ./find_person.gq\n",
|
||||
);
|
||||
let output = output_success(
|
||||
cli()
|
||||
.arg("queries")
|
||||
.arg("list")
|
||||
.arg("--config")
|
||||
.arg(&config),
|
||||
.arg("--cluster")
|
||||
.arg(cluster.path()),
|
||||
);
|
||||
let stdout = stdout_string(&output);
|
||||
assert!(stdout.contains("find_person"), "stdout:\n{stdout}");
|
||||
|
|
@ -185,242 +186,37 @@ fn queries_list_prints_registered_query() {
|
|||
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),
|
||||
);
|
||||
fn queries_validate_requires_a_cluster() {
|
||||
// RFC-011: with no --cluster (and no cluster profile), the command errors
|
||||
// loudly rather than reading any omnigraph.yaml.
|
||||
let output = output_failure(cli().arg("queries").arg("validate"));
|
||||
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}"
|
||||
stderr.contains("needs a cluster") || stderr.contains("--cluster"),
|
||||
"queries validate must require a cluster; 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).
|
||||
fn queries_validate_graph_filter_selects_one_graph() {
|
||||
// A multi-graph cluster: validate scoped to `knowledge` type-checks only
|
||||
// that graph's registry, ignoring `engineering`'s.
|
||||
let temp = tempdir().unwrap();
|
||||
let dir = temp.path();
|
||||
write_multi_graph_cluster_fixture(dir);
|
||||
output_success(cli().arg("cluster").arg("import").arg("--config").arg(dir));
|
||||
output_success(cli().arg("cluster").arg("apply").arg("--config").arg(dir));
|
||||
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}"
|
||||
.arg("--cluster")
|
||||
.arg(dir)
|
||||
.arg("--graph")
|
||||
.arg("knowledge"),
|
||||
);
|
||||
assert!(stdout_string(&output).contains("OK"));
|
||||
}
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue