feat(cli)!: excise omnigraph.yaml from the CLI; policy/queries read --cluster - #251
Conversation
…g reads --cluster 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>
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| URI): query, mutate, load, branch, snapshot, export, commit, schema show/apply.\n \ | ||
| served — require a server: graphs.\n \ | ||
| direct — direct storage access; reject --server (init, optimize, repair, cleanup, \ | ||
| schema plan, lint, queries validate).\n \ | ||
| control — manage a cluster via --config: cluster.\n \ | ||
| local — no graph; local config & tooling: policy, embed, login, logout, config, \ | ||
| version, queries list.\n\ | ||
| schema plan, lint).\n \ | ||
| control — manage or inspect a cluster (cluster via --config; policy & queries via \ | ||
| --cluster).\n \ | ||
| local — no graph; local config & tooling: embed, login, logout, config, version.\n\ | ||
| See the 'Command capabilities' section of the CLI reference for which flags apply where.")] | ||
| pub(crate) struct Cli { |
There was a problem hiding this comment.
AGENTS.md Rule #1 states: "Update in the same PR. New endpoint, query function, CLI flag, env var, constant, schema construct, or invariant: update both the source code and the doc in the same change. Never split documentation drift into a follow-up."
This PR removes --config from every data/query command, adds a mandatory --cluster requirement for policy and queries, changes policy test to require --tests <file>, and retires the positional URI from queries validate / queries list — all user-visible CLI surface changes. The PR description explicitly schedules docs as a future PR "D (docs/AGENTS.md)".
In the meantime, AGENTS.md (always-loaded into agent context) still says the legacy omnigraph.yaml loads with deprecation warnings across the CLI, docs/user/cli/reference.md still documents the removed --config flags, and the policy test entry no longer reflects the implicit policy.tests.yaml behavior. The stale content in AGENTS.md actively misleads future agents working in this codebase.
Context Used: AGENTS.md (source)
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
| ) | ||
| .await?; | ||
| let schema_source = fs::read_to_string(&schema)?; | ||
| // The stored-query registry check is an embedded-only concern | ||
| // (the remote arm ignores the validator — the server runs its | ||
| // own check); build it only for the local path so the remote | ||
| // path keeps its no-registry-load behavior. | ||
| let registry = if client.is_remote() { | ||
| None | ||
| } else { | ||
| let registry = load_registry_or_report(&config, client.selected())?; | ||
| (!registry.is_empty()).then_some(registry) | ||
| }; | ||
| let label = client.selected().unwrap_or(client.uri()).to_string(); | ||
| // The embedded (direct-store) arm carries no stored-query | ||
| // registry — the registry is cluster-owned (RFC-011), so a | ||
| // direct apply has nothing to validate against. The served arm | ||
| // runs the server's own catalog check. So the validator is a | ||
| // no-op here on both arms. | ||
| echo_write_target(cli.quiet, "schema apply", client.uri(), client.is_remote()); | ||
| let output = client | ||
| .apply_schema(&schema_source, allow_data_loss, |catalog| { | ||
| if let Some(registry) = registry.as_ref() { | ||
| validate_registry_for_catalog(registry, catalog, &label)?; | ||
| } | ||
| Ok(()) | ||
| }) | ||
| .apply_schema(&schema_source, allow_data_loss, |_catalog| Ok(())) | ||
| .await?; | ||
| if json { |
There was a problem hiding this comment.
schema apply --store silently skips stored-query validation
The embedded arm now passes |_catalog| Ok(()) — a no-op validator. Previously, when omnigraph.yaml listed stored queries, the CLI would pre-flight–check them against the new schema before applying and surface breakages. That gate is now gone for any direct-store invocation.
The PR rationale is sound (RFC-011: the registry is cluster-owned), but the consequence is that omnigraph schema apply --store <uri> on a graph whose stored queries are registered in a cluster will silently apply a breaking schema change without warning. The cluster's server-side check is the only remaining guard, and that fires only on a --server / --profile path, not on --store. Operators who run maintenance applies directly against storage now need to run queries validate --cluster <dir> manually afterward to confirm no breakage.
1 participant
What
Removes the CLI's last use of the legacy
omnigraph.yaml(OmnigraphConfig). After this PR the CLI addresses graphs only via--store/--server/--cluster/--profile/ operator defaults, and reads actor / output-format / bearer credentials from~/.omnigraph/config.yaml. No CLI command readsomnigraph.yamlexceptconfig migrate.Part of the cluster-only / omnigraph.yaml-excision sequence: A (#250, cluster-only server) ✅ → B (this PR) → C (delete the
OmnigraphConfigtype +config migrate+ deprecation machinery) → D (docs/AGENTS.md). No version bump (0.7.0 unreleased).Resolvers (
helpers.rs) — drop every legacy fallbackresolve_actor→--as>operator.actor(nocli.actor)resolve_read_format→--json/--format> alias >defaults.output> tableresolve_branch/resolve_read_target→--branch> alias >mainresolve_uri/resolve_cli_graph→ scope path only; an unaddressed graph is a loud errorresolve_remote_bearer_token→ operator keyed chain (OMNIGRAPH_TOKEN_<NAME>+ credentials file) >OMNIGRAPH_BEARER_TOKENGraphClient::resolve/resolve_with_policydrop the&OmnigraphConfigparam; direct-store access carries no Cedar policy (policy lives in the cluster/server)Flags (
cli.rs)--configremoved from every data/query command; kept only oncluster *(the cluster dir) andconfig migrate(the legacy path)Re-home control-plane tooling to
--clusterpolicy validate|test|explain— source the Cedar bundle from the cluster's applied policies (serving snapshot);--graphpicks a graph's bundle;policy testnow takes--tests <file>queries list|validate— source the registry + schemas from the cluster serving snapshot;--graphscopes to one graphlint— requires--schema(offline) or a direct/cluster graph targetschema plan/lintroute their graph-target through the shared direct-scope resolver, so--store/--profile/defaults.storeaddressing worksTests
Migrated from
omnigraph.yamlfixtures to--store/ operator-config /--cluster(converged-cluster fixtures). The now-impossible command-path RFC-008 tests are deleted (legacy_config_load_warns,strict_mode_refuses);config_migrate_splits_legacy_configand thecluster_commands_ignore_legacy_omnigraph_yamlinert-file guard are kept.Verification
cargo test --workspace --lockedgreen: 61 suites, 1399 passed, 0 failed.scripts/check-agents-md.shclean except for pre-existing untracked scratch docs (not part of this PR).🤖 Generated with Claude Code
Greptile Summary
This PR completes the excision of
omnigraph.yamlfrom the CLI's data and control-plane commands. After this change,--configis gone from every data/query command;policyandqueriesre-home to--cluster(sourcing Cedar bundles and stored-query registries from the cluster's serving snapshot); andschema plan/lintresolve their graph target through the shared direct-scope resolver. Onlyconfig migrateretains any knowledge of the legacy file.helpers.rs: dropsOmnigraphConfigfrom all resolver functions; replaces the multi-leg legacy bearer-token chain with the operator keyed-chain +OMNIGRAPH_BEARER_TOKENfallback; addsrequire_cluster_scope,read_cluster_policies,select_cluster_policy,execute_queries_validate, andexecute_queries_listfor the new cluster-based control-plane tooling.main.rs/cli.rs: every data/query command sheds--config;policyandqueriesdispatch to the new cluster helpers;schema apply's embedded arm now uses a no-op catalog validator (the query registry is cluster-owned under RFC-011);policy testrequires an explicit--tests <file>.planes.rs:queriesandpolicyare reclassified to theControlplane; the exhaustivecommand_planematch is updated accordingly.Confidence Score: 4/5
The core routing and resolver changes are mechanically correct and the parity-matrix tests still pass with empty divergences. The main concern is that AGENTS.md and docs/user/cli/reference.md are left describing removed flags and deprecated behavior, and schema apply --store now silently skips stored-query pre-flight checks.
The mechanical refactor is clean and well-tested (61 suites, 1399 passing). The documentation gap is a concrete problem: AGENTS.md is always loaded into agent context and still describes the omnigraph.yaml deprecation-warning path as active CLI behavior; docs/user/cli/reference.md likely still shows --config flags that no longer exist.
AGENTS.md and docs/user/cli/reference.md are stale with respect to the removed --config flags and the new --cluster requirement for policy/queries. crates/omnigraph-cli/src/main.rs (schema apply no-op validator) is worth a second look before merge.
Important Files Changed
Flowchart
%%{init: {'theme': 'neutral'}}%% flowchart TD CLI[omnigraph CLI] --> Guard[guard_addressing] Guard --> Plane{command plane} Plane -->|Data| DataCmd["query / mutate / load / branch\nschema show/apply / commit / export"] Plane -->|Storage| StorageCmd["optimize / repair / cleanup\nschema plan / lint / init"] Plane -->|Control| CtrlCmd["cluster / policy / queries"] Plane -->|Session| SessionCmd["login / logout / config / version / embed"] DataCmd --> GraphClient["GraphClient::resolve or resolve_with_policy"] GraphClient -->|remote| Remote["HTTP server (--server / --profile)"] GraphClient -->|local| Embedded["Direct storage (--store / --cluster+graph / defaults)"] StorageCmd --> MaintURI["resolve_maintenance_uri"] MaintURI -->|cluster scope| ClusterState["omnigraph_cluster::resolve_graph_storage_uri"] MaintURI -->|direct URI| LocalStore["Local / S3 storage"] CtrlCmd --> ClusterScope["require_cluster_scope (--cluster / --profile)"] ClusterScope --> ServingSnapshot["read_serving_snapshot"] ServingSnapshot -->|policy cmds| PolicyEngine["PolicyEngine::load_graph_from_source"] ServingSnapshot -->|queries cmds| QueriesRegistry["registry_from_serving_queries"]Comments Outside Diff (1)
crates/omnigraph-cli/src/helpers.rs, line 864-925 (link)queries listis now async and requires cluster statePreviously
queries listread the query registry from the localomnigraph.yamlsynchronously — a pure offline/local operation. After this change it reads from the cluster's serving snapshot (read_serving_snapshot_or_report), which requires the cluster to be in a servable state. Any operator who usesqueries listfor quick offline inspection (e.g., in a CI job that only has the cluster directory checked out, not a running cluster) will now get acluster is not servableerror instead of a list. Worth a note in the docs / migration guide.Reviews (1): Last reviewed commit: "feat(cli)!: excise omnigraph.yaml from t..." | Re-trigger Greptile
Context used: