mirror of
https://github.com/ModernRelay/omnigraph.git
synced 2026-06-15 01:55:13 +02:00
feat(cli): RFC-010 Slice 1 — declared plane capability surface + honest addressing (#217)
* feat(cli): declared plane capability surface + wrong-plane guard (RFC-010 Slice 1) New `planes.rs` is the single source of truth for which plane each subcommand belongs to (Data / Storage / Control / Session). `command_plane` is an exhaustive match — adding a `Command` variant is a compile error until its plane is declared, so the surface cannot silently drift from the command set. It descends into the nested enums where the plane differs per subcommand (`schema plan` is storage while `schema show/apply` are data; `queries validate` opens the graph while `queries list` reads only config). `guard_addressing` runs once in `main` before dispatch: the data-plane addressing flags `--server`/`--graph` on any non-data verb now fail with one declared, pinned error instead of being silently ignored (`optimize --server prod` previously dropped `--server`). `init`'s message drops the `--target` half since it takes only a positional URI today. Test: `cli_schema_config::schema_plan_with_server_flag_errors_wrong_plane` pins the per-subcommand label, proving the guard descends into the nested enum. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * feat(cli): storage-plane verbs fail loudly on a remote target (RFC-010 Slice 1) `optimize`/`repair`/`cleanup` switch from `resolve_uri` to `resolve_local_uri`, so a `--target` (or positional URI) that resolves to a remote server now fails with a declared storage-plane message instead of whatever `Omnigraph::open` said about an `http(s)://` URI. The `resolve_local_graph` bail is reworded to that storage-plane message, so every storage verb already on the local resolver (`schema plan`, `queries validate`, `lint`) speaks with one voice. Net: `optimize --target knowledge` resolves to the graph's storage URI and runs embedded; `optimize --target prod` (remote) fails loudly; `optimize --server` is caught earlier by the guard. Positional-URI invocations are unchanged. Tests (pinned strings, per RFC-010's test plan): optimize happy path on a local graph, `optimize --server` wrong-plane error, `optimize <https>` storage-plane error; the existing `query_lint_rejects_http_targets_without_schema` assertion is updated to the new shared message. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
parent
2ddb88fad9
commit
106356ab25
5 changed files with 232 additions and 6 deletions
|
|
@ -464,8 +464,11 @@ pub(crate) fn resolve_local_graph(
|
|||
let graph = resolve_cli_graph(config, cli_uri, cli_target)?;
|
||||
if graph.is_remote {
|
||||
bail!(
|
||||
"{} is only supported against local graph URIs in this milestone",
|
||||
operation
|
||||
"`{}` is a storage-plane command and needs direct storage access; \
|
||||
the resolved target is a remote server ({}). Pass the graph's \
|
||||
file:// or s3:// URI.",
|
||||
operation,
|
||||
graph.uri
|
||||
);
|
||||
}
|
||||
Ok(graph)
|
||||
|
|
|
|||
|
|
@ -49,6 +49,7 @@ mod cli;
|
|||
mod client;
|
||||
mod helpers;
|
||||
mod output;
|
||||
mod planes;
|
||||
use cli::*;
|
||||
use helpers::*;
|
||||
use output::*;
|
||||
|
|
@ -70,6 +71,10 @@ async fn main() -> Result<()> {
|
|||
Cli::from_arg_matches(&matches)?
|
||||
};
|
||||
let http_client = build_http_client()?;
|
||||
// RFC-010 Slice 1: reject data-plane addressing flags (--server/--graph) on
|
||||
// a verb that doesn't live on the data plane, from one declared table —
|
||||
// before any per-command dispatch.
|
||||
planes::guard_addressing(&cli)?;
|
||||
match cli.command {
|
||||
Command::Config { command } => match command {
|
||||
ConfigCommand::Migrate { config, write, json } => {
|
||||
|
|
@ -781,7 +786,7 @@ async fn main() -> Result<()> {
|
|||
json,
|
||||
} => {
|
||||
let config = load_cli_config(config.as_ref())?;
|
||||
let uri = resolve_uri(&config, uri, target.as_deref())?;
|
||||
let uri = resolve_local_uri(&config, uri, target.as_deref(), "optimize")?;
|
||||
let db = Omnigraph::open(&uri).await?;
|
||||
let stats = db.optimize().await?;
|
||||
if json {
|
||||
|
|
@ -823,7 +828,7 @@ async fn main() -> Result<()> {
|
|||
json,
|
||||
} => {
|
||||
let config = load_cli_config(config.as_ref())?;
|
||||
let uri = resolve_uri(&config, uri, target.as_deref())?;
|
||||
let uri = resolve_local_uri(&config, uri, target.as_deref(), "repair")?;
|
||||
let db = Omnigraph::open(&uri).await?;
|
||||
let stats = db
|
||||
.repair(omnigraph::db::RepairOptions { confirm, force })
|
||||
|
|
@ -907,7 +912,7 @@ async fn main() -> Result<()> {
|
|||
json,
|
||||
} => {
|
||||
let config = load_cli_config(config.as_ref())?;
|
||||
let uri = resolve_uri(&config, uri, target.as_deref())?;
|
||||
let uri = resolve_local_uri(&config, uri, target.as_deref(), "cleanup")?;
|
||||
|
||||
let older_than_dur = older_than.as_deref().map(parse_duration_arg).transpose()?;
|
||||
|
||||
|
|
|
|||
151
crates/omnigraph-cli/src/planes.rs
Normal file
151
crates/omnigraph-cli/src/planes.rs
Normal file
|
|
@ -0,0 +1,151 @@
|
|||
//! Declared CLI "planes" (RFC-010 Slice 1).
|
||||
//!
|
||||
//! Every subcommand belongs to exactly one plane. This classification is the
|
||||
//! single source of truth the wrong-plane guard consumes — and that later
|
||||
//! RFC-010 slices (the capability surface, plane-grouped help) will consume
|
||||
//! too. The `command_plane` match is **exhaustive on purpose**: adding a
|
||||
//! `Command` variant is a compile error until its plane is declared, so the
|
||||
//! surface cannot silently drift from the command set.
|
||||
//!
|
||||
//! See [docs/dev/rfc-010-cli-planes-restructure.md].
|
||||
|
||||
use color_eyre::Result;
|
||||
use color_eyre::eyre::bail;
|
||||
|
||||
use crate::cli::{Cli, Command, QueriesCommand, SchemaCommand};
|
||||
|
||||
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
|
||||
pub(crate) enum Plane {
|
||||
/// Runs against a graph, embedded **or** via `--server` (the `GraphClient`
|
||||
/// axis). The only plane on which the data-plane addressing flags
|
||||
/// (`--server`/`--graph`) apply.
|
||||
Data,
|
||||
/// Direct storage access; no server. Maintenance + local-only inspection
|
||||
/// that must work with the server down.
|
||||
Storage,
|
||||
/// Operates on a cluster directory, not a graph URI.
|
||||
Control,
|
||||
/// Touches no graph at all — session / config / local tooling.
|
||||
Session,
|
||||
}
|
||||
|
||||
impl std::fmt::Display for Plane {
|
||||
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
|
||||
f.write_str(match self {
|
||||
Plane::Data => "data",
|
||||
Plane::Storage => "storage",
|
||||
Plane::Control => "control",
|
||||
Plane::Session => "session",
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
/// The plane a subcommand belongs to. Exhaustive — a new `Command` variant
|
||||
/// will not compile until classified. Descends into the nested enums where
|
||||
/// the plane differs per subcommand (`schema plan` is storage while `schema
|
||||
/// show`/`apply` are data; `queries validate` opens the graph while `queries
|
||||
/// list` only reads config).
|
||||
pub(crate) fn command_plane(cmd: &Command) -> Plane {
|
||||
match cmd {
|
||||
Command::Query { .. }
|
||||
| Command::Mutate { .. }
|
||||
| Command::Load { .. }
|
||||
| Command::Ingest { .. }
|
||||
| Command::Branch { .. }
|
||||
| Command::Snapshot { .. }
|
||||
| Command::Export { .. }
|
||||
| Command::Commit { .. }
|
||||
| Command::Graphs { .. } => Plane::Data,
|
||||
Command::Schema {
|
||||
command: SchemaCommand::Show { .. } | SchemaCommand::Apply { .. },
|
||||
} => Plane::Data,
|
||||
Command::Schema {
|
||||
command: SchemaCommand::Plan { .. },
|
||||
} => Plane::Storage,
|
||||
Command::Queries {
|
||||
command: QueriesCommand::Validate { .. },
|
||||
} => Plane::Storage,
|
||||
Command::Queries {
|
||||
command: QueriesCommand::List { .. },
|
||||
} => Plane::Session,
|
||||
Command::Init { .. }
|
||||
| Command::Optimize { .. }
|
||||
| Command::Repair { .. }
|
||||
| Command::Cleanup { .. }
|
||||
| Command::Lint { .. } => Plane::Storage,
|
||||
Command::Cluster { .. } => Plane::Control,
|
||||
Command::Policy { .. }
|
||||
| Command::Embed(_)
|
||||
| Command::Login { .. }
|
||||
| Command::Logout { .. }
|
||||
| Command::Config { .. }
|
||||
| Command::Version => Plane::Session,
|
||||
}
|
||||
}
|
||||
|
||||
/// User-facing label for a subcommand (descends one level for the nested
|
||||
/// families so messages read `schema plan`, `queries validate`, etc.).
|
||||
pub(crate) fn command_label(cmd: &Command) -> &'static str {
|
||||
match cmd {
|
||||
Command::Version => "version",
|
||||
Command::Login { .. } => "login",
|
||||
Command::Logout { .. } => "logout",
|
||||
Command::Config { .. } => "config",
|
||||
Command::Embed(_) => "embed",
|
||||
Command::Init { .. } => "init",
|
||||
Command::Load { .. } => "load",
|
||||
Command::Ingest { .. } => "ingest",
|
||||
Command::Branch { .. } => "branch",
|
||||
Command::Schema { command } => match command {
|
||||
SchemaCommand::Plan { .. } => "schema plan",
|
||||
SchemaCommand::Apply { .. } => "schema apply",
|
||||
SchemaCommand::Show { .. } => "schema show",
|
||||
},
|
||||
Command::Lint { .. } => "lint",
|
||||
Command::Queries { command } => match command {
|
||||
QueriesCommand::Validate { .. } => "queries validate",
|
||||
QueriesCommand::List { .. } => "queries list",
|
||||
},
|
||||
Command::Snapshot { .. } => "snapshot",
|
||||
Command::Export { .. } => "export",
|
||||
Command::Commit { .. } => "commit",
|
||||
Command::Query { .. } => "query",
|
||||
Command::Mutate { .. } => "mutate",
|
||||
Command::Policy { .. } => "policy",
|
||||
Command::Optimize { .. } => "optimize",
|
||||
Command::Repair { .. } => "repair",
|
||||
Command::Cleanup { .. } => "cleanup",
|
||||
Command::Cluster { .. } => "cluster",
|
||||
Command::Graphs { .. } => "graphs",
|
||||
}
|
||||
}
|
||||
|
||||
/// Reject the data-plane addressing flags (`--server`/`--graph`) on any verb
|
||||
/// that does not live on the data plane. This replaces the old silent-ignore
|
||||
/// — e.g. `optimize --server prod` previously dropped `--server` and tried to
|
||||
/// resolve a default target, failing (if at all) with an unrelated message.
|
||||
/// Now it fails with one honest, declared error. RFC-010 Slice 1.
|
||||
pub(crate) fn guard_addressing(cli: &Cli) -> Result<()> {
|
||||
if cli.server.is_none() && cli.graph.is_none() {
|
||||
return Ok(());
|
||||
}
|
||||
let plane = command_plane(&cli.command);
|
||||
if plane == Plane::Data {
|
||||
return Ok(());
|
||||
}
|
||||
let label = command_label(&cli.command);
|
||||
let how = match plane {
|
||||
// `init` is the one storage verb with no `--target` today (it takes a
|
||||
// required positional URI), so its remediation drops the `--target` half.
|
||||
Plane::Storage => match cli.command {
|
||||
Command::Init { .. } => "Pass a storage URI.",
|
||||
_ => "Use --target <name> or a storage URI.",
|
||||
},
|
||||
Plane::Control => "It operates on a cluster directory (pass --config <dir>).",
|
||||
Plane::Session => "It does not address a graph.",
|
||||
Plane::Data => unreachable!("data plane returned early"),
|
||||
};
|
||||
bail!(
|
||||
"`{label}` is a {plane}-plane command; --server/--graph address the data plane and do not apply. {how}"
|
||||
);
|
||||
}
|
||||
|
|
@ -142,6 +142,47 @@ fn embed_seed_preserves_non_entity_rows() {
|
|||
assert_eq!(embedded[2]["to"], "dec-alpha");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn optimize_json_succeeds_on_local_graph() {
|
||||
// Happy path for the resolve_local_uri swap (RFC-010 Slice 1): a positional
|
||||
// local path still resolves and runs embedded.
|
||||
let temp = tempdir().unwrap();
|
||||
let graph = graph_path(temp.path());
|
||||
init_graph(&graph);
|
||||
load_fixture(&graph);
|
||||
|
||||
let output = output_success(cli().arg("optimize").arg("--json").arg(&graph));
|
||||
let payload: Value = serde_json::from_slice(&output.stdout).unwrap();
|
||||
assert!(payload["tables"].as_array().is_some());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn optimize_with_server_flag_errors_wrong_plane() {
|
||||
// RFC-010 Slice 1: --server is a data-plane addressing flag; on a
|
||||
// storage-plane verb the guard rejects it loudly (was: silently ignored).
|
||||
let output = output_failure(cli().arg("optimize").arg("--server").arg("prod"));
|
||||
let stderr = String::from_utf8_lossy(&output.stderr);
|
||||
assert!(
|
||||
stderr.contains("`optimize` is a storage-plane command")
|
||||
&& stderr.contains("--server/--graph address the data plane and do not apply")
|
||||
&& stderr.contains("Use --target <name> or a storage URI."),
|
||||
"wrong-plane guard message not found; got: {stderr}"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn optimize_with_remote_target_errors_storage_plane() {
|
||||
// RFC-010 Slice 1: a maintenance verb pointed at a remote URI fails loudly
|
||||
// and declaratively (was: whatever Omnigraph::open said about an https URI).
|
||||
let output = output_failure(cli().arg("optimize").arg("https://graph.example.invalid"));
|
||||
let stderr = String::from_utf8_lossy(&output.stderr);
|
||||
assert!(
|
||||
stderr.contains("`optimize` is a storage-plane command and needs direct storage access")
|
||||
&& stderr.contains("remote server"),
|
||||
"storage-plane remote-target message not found; got: {stderr}"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn repair_json_reports_noop_on_clean_graph() {
|
||||
let temp = tempdir().unwrap();
|
||||
|
|
@ -542,8 +583,12 @@ query list_people() {
|
|||
.arg("http://127.0.0.1:8080"),
|
||||
);
|
||||
let stderr = String::from_utf8_lossy(&output.stderr);
|
||||
// RFC-010 Slice 1: the storage-plane verbs now share one declared message
|
||||
// (was: "query lint is only supported against local graph URIs …").
|
||||
assert!(
|
||||
stderr.contains("query lint is only supported against local graph URIs in this milestone")
|
||||
stderr.contains("`query lint` is a storage-plane command and needs direct storage access")
|
||||
&& stderr.contains("remote server"),
|
||||
"storage-plane remote-target message not found; got: {stderr}"
|
||||
);
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -72,6 +72,28 @@ fn schema_plan_json_reports_supported_additive_change() {
|
|||
assert_eq!(payload["steps"][0]["property_name"], "nickname");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn schema_plan_with_server_flag_errors_wrong_plane() {
|
||||
// RFC-010 Slice 1: `schema plan` is storage-plane while `schema show/apply`
|
||||
// are data-plane — the guard rejects --server on plan with the per-subcommand
|
||||
// label (proving command_plane/command_label descend into the nested enum).
|
||||
let output = output_failure(
|
||||
cli()
|
||||
.arg("schema")
|
||||
.arg("plan")
|
||||
.arg("--schema")
|
||||
.arg(fixture("test.pg"))
|
||||
.arg("--server")
|
||||
.arg("prod"),
|
||||
);
|
||||
let stderr = String::from_utf8_lossy(&output.stderr);
|
||||
assert!(
|
||||
stderr.contains("`schema plan` is a storage-plane command")
|
||||
&& stderr.contains("Use --target <name> or a storage URI."),
|
||||
"schema plan wrong-plane message not found; got: {stderr}"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn schema_plan_json_reports_unsupported_type_change() {
|
||||
let temp = tempdir().unwrap();
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue