mirror of
https://github.com/ModernRelay/omnigraph.git
synced 2026-06-18 02:24:27 +02:00
feat(cli): --quiet/--yes globals; echo resolved write target; gate non-local destructive writes (#243)
RFC-011 Decision 9. Writes echo their resolved target + access path to stderr (suppress with --quiet). Destructive writes (cleanup, overwrite load, branch delete) against a non-local scope require consent: --yes, a TTY prompt, or a hard refusal for non-TTY/--json runs. Local file:// writes unaffected.
This commit is contained in:
parent
a09045028f
commit
2ed05d2cb1
9 changed files with 295 additions and 2 deletions
|
|
@ -66,6 +66,18 @@ pub(crate) struct Cli {
|
|||
#[arg(long, global = true, value_name = "DIR|URI")]
|
||||
pub(crate) cluster: Option<String>,
|
||||
|
||||
/// Skip the confirmation prompt for a destructive write (`cleanup`,
|
||||
/// overwrite `load`, `branch delete`) against a non-local scope (RFC-011
|
||||
/// Decision 9). Without it, a non-local destructive write prompts on a TTY
|
||||
/// and refuses (errors) when there is no TTY or `--json` is set.
|
||||
#[arg(long, global = true)]
|
||||
pub(crate) yes: bool,
|
||||
|
||||
/// Suppress the one-line resolved-write-target diagnostic that write
|
||||
/// commands echo to stderr (RFC-011 Decision 9).
|
||||
#[arg(long, global = true)]
|
||||
pub(crate) quiet: bool,
|
||||
|
||||
#[command(subcommand)]
|
||||
pub(crate) command: Command,
|
||||
}
|
||||
|
|
|
|||
|
|
@ -2,6 +2,8 @@
|
|||
//! remote HTTP, env/token handling, scaffolding (moved verbatim from
|
||||
//! main.rs in the modularization).
|
||||
|
||||
use std::io::IsTerminal;
|
||||
|
||||
use super::*;
|
||||
use crate::operator;
|
||||
|
||||
|
|
@ -16,6 +18,59 @@ pub(crate) fn is_remote_uri(uri: &str) -> bool {
|
|||
uri.starts_with("http://") || uri.starts_with("https://")
|
||||
}
|
||||
|
||||
/// Whether a resolved write target is *local* for the purposes of the RFC-011
|
||||
/// Decision 9 destructive-confirm gate: a bare path or a `file://` URI. Anything
|
||||
/// else carrying a scheme — `http(s)://` (served), `s3://` / `gs://` / … (object
|
||||
/// store) — is non-local and a destructive write against it requires explicit
|
||||
/// consent. Generalizes `is_remote_uri` (which only catches http(s)).
|
||||
pub(crate) fn uri_is_local(uri: &str) -> bool {
|
||||
!uri.contains("://") || uri.starts_with("file://")
|
||||
}
|
||||
|
||||
/// Echo the resolved write target + access path to stderr (RFC-011 Decision 9),
|
||||
/// unless `--quiet`. One line, e.g. `omnigraph load → file://g.omni (direct,
|
||||
/// local)`. stderr so `--json` consumers reading stdout are unaffected; the line
|
||||
/// legitimately differs embedded-vs-served (that visibility is the point).
|
||||
pub(crate) fn echo_write_target(quiet: bool, label: &str, uri: &str, served: bool) {
|
||||
if quiet {
|
||||
return;
|
||||
}
|
||||
let access = if served {
|
||||
"served"
|
||||
} else if uri_is_local(uri) {
|
||||
"direct, local"
|
||||
} else {
|
||||
"direct, remote"
|
||||
};
|
||||
eprintln!("omnigraph {label} → {uri} ({access})");
|
||||
}
|
||||
|
||||
/// Gate a destructive write (`cleanup`, overwrite `load`, `branch delete`)
|
||||
/// against a non-local scope (RFC-011 Decision 9). A local target needs no
|
||||
/// confirmation; otherwise `--yes` consents, an interactive TTY is prompted, and
|
||||
/// a non-TTY / `--json` run refuses rather than silently proceeding.
|
||||
pub(crate) fn confirm_destructive(label: &str, uri: &str, yes: bool, json: bool) -> Result<()> {
|
||||
if uri_is_local(uri) || yes {
|
||||
return Ok(());
|
||||
}
|
||||
if json || !std::io::stdin().is_terminal() {
|
||||
bail!(
|
||||
"refusing destructive `{label}` against non-local target {uri} without confirmation; \
|
||||
pass --yes to confirm (an interactive TTY would be prompted instead)"
|
||||
);
|
||||
}
|
||||
eprint!(
|
||||
"About to run a destructive `{label}` against {uri} (not local). Type 'yes' to continue: "
|
||||
);
|
||||
io::stderr().flush()?;
|
||||
let mut answer = String::new();
|
||||
io::stdin().read_line(&mut answer)?;
|
||||
match answer.trim().to_ascii_lowercase().as_str() {
|
||||
"yes" | "y" => Ok(()),
|
||||
_ => bail!("aborted: destructive `{label}` not confirmed"),
|
||||
}
|
||||
}
|
||||
|
||||
/// THE one way the CLI composes a remote request URL. Every remote call
|
||||
/// routes through here so URL assembly has a single mechanism instead of
|
||||
/// per-callsite string interpolation.
|
||||
|
|
@ -1112,6 +1167,40 @@ pub(crate) fn rewrite_deprecated_argv(args: Vec<OsString>) -> Vec<OsString> {
|
|||
mod tests {
|
||||
use super::*;
|
||||
|
||||
// RFC-011 Decision 9: locality classifier for the destructive-confirm gate.
|
||||
#[test]
|
||||
fn uri_is_local_truth_table() {
|
||||
// Local: bare path or file://.
|
||||
assert!(uri_is_local("graph.omni"));
|
||||
assert!(uri_is_local("/abs/path/graph.omni"));
|
||||
assert!(uri_is_local("file:///tmp/graph.omni"));
|
||||
// Non-local: served or object-store schemes.
|
||||
assert!(!uri_is_local("http://host/graphs/g"));
|
||||
assert!(!uri_is_local("https://host/graphs/g"));
|
||||
assert!(!uri_is_local("s3://bucket/graph.omni"));
|
||||
assert!(!uri_is_local("gs://bucket/graph.omni"));
|
||||
}
|
||||
|
||||
// RFC-011 Decision 9: a non-local destructive write with `--json` (the CI
|
||||
// shape — also covers the no-TTY case, since tests run without a terminal)
|
||||
// refuses rather than proceeding; a local one and an explicit `--yes` pass.
|
||||
#[test]
|
||||
fn confirm_destructive_refuses_non_local_without_consent() {
|
||||
let err = confirm_destructive("cleanup", "s3://b/g.omni", false, true)
|
||||
.unwrap_err()
|
||||
.to_string();
|
||||
assert!(err.contains("--yes"), "{err}");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn confirm_destructive_allows_local_and_explicit_yes() {
|
||||
// Local needs no confirmation, even with --json.
|
||||
assert!(confirm_destructive("cleanup", "file:///tmp/g.omni", false, true).is_ok());
|
||||
assert!(confirm_destructive("branch delete", "graph.omni", false, true).is_ok());
|
||||
// --yes consents to a non-local target.
|
||||
assert!(confirm_destructive("cleanup", "s3://b/g.omni", true, true).is_ok());
|
||||
}
|
||||
|
||||
// RFC-011 Decision 2: `--server` accepts a literal URL (value with `://`),
|
||||
// bypassing the operator-config registry — so no config / OMNIGRAPH_HOME is
|
||||
// read on this path (hermetic).
|
||||
|
|
|
|||
|
|
@ -188,6 +188,10 @@ async fn main() -> Result<()> {
|
|||
cli.store.as_deref(),
|
||||
)?;
|
||||
let branch = resolve_branch(&config, branch, None, "main");
|
||||
if matches!(mode, CliLoadMode::Overwrite) {
|
||||
confirm_destructive("load --mode overwrite", client.uri(), cli.yes, json)?;
|
||||
}
|
||||
echo_write_target(cli.quiet, "load", client.uri(), client.is_remote());
|
||||
let payload = client
|
||||
.load(&branch, from.as_deref(), &data.to_string_lossy(), mode)
|
||||
.await?;
|
||||
|
|
@ -223,6 +227,7 @@ async fn main() -> Result<()> {
|
|||
)?;
|
||||
let branch = resolve_branch(&config, branch, None, "main");
|
||||
let from = resolve_branch(&config, from, None, "main");
|
||||
echo_write_target(cli.quiet, "ingest", client.uri(), client.is_remote());
|
||||
let payload = client
|
||||
.ingest(&branch, &from, &data.to_string_lossy(), mode)
|
||||
.await?;
|
||||
|
|
@ -251,6 +256,7 @@ async fn main() -> Result<()> {
|
|||
cli.store.as_deref(),
|
||||
)?;
|
||||
let from = resolve_branch(&config, from, None, "main");
|
||||
echo_write_target(cli.quiet, "branch create", client.uri(), client.is_remote());
|
||||
let payload = client.branch_create_from(&from, &name).await?;
|
||||
if json {
|
||||
print_json(&payload)?;
|
||||
|
|
@ -297,6 +303,8 @@ async fn main() -> Result<()> {
|
|||
cli.profile.as_deref(),
|
||||
cli.store.as_deref(),
|
||||
)?;
|
||||
confirm_destructive("branch delete", client.uri(), cli.yes, json)?;
|
||||
echo_write_target(cli.quiet, "branch delete", client.uri(), client.is_remote());
|
||||
let payload = client.branch_delete(&name).await?;
|
||||
if json {
|
||||
print_json(&payload)?;
|
||||
|
|
@ -322,6 +330,7 @@ async fn main() -> Result<()> {
|
|||
cli.store.as_deref(),
|
||||
)?;
|
||||
let into = resolve_branch(&config, into, None, "main");
|
||||
echo_write_target(cli.quiet, "branch merge", client.uri(), client.is_remote());
|
||||
let payload = client.branch_merge(&source, &into).await?;
|
||||
if json {
|
||||
print_json(&payload)?;
|
||||
|
|
@ -440,6 +449,7 @@ async fn main() -> Result<()> {
|
|||
(!registry.is_empty()).then_some(registry)
|
||||
};
|
||||
let label = client.selected().unwrap_or(client.uri()).to_string();
|
||||
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() {
|
||||
|
|
@ -802,6 +812,7 @@ async fn main() -> Result<()> {
|
|||
"optimize",
|
||||
)
|
||||
.await?;
|
||||
echo_write_target(cli.quiet, "optimize", &uri, false);
|
||||
let db = Omnigraph::open(&uri).await?;
|
||||
let stats = db.optimize().await?;
|
||||
if json {
|
||||
|
|
@ -852,6 +863,7 @@ async fn main() -> Result<()> {
|
|||
"repair",
|
||||
)
|
||||
.await?;
|
||||
echo_write_target(cli.quiet, "repair", &uri, false);
|
||||
let db = Omnigraph::open(&uri).await?;
|
||||
let stats = db
|
||||
.repair(omnigraph::db::RepairOptions { confirm, force })
|
||||
|
|
@ -967,6 +979,11 @@ async fn main() -> Result<()> {
|
|||
);
|
||||
return Ok(());
|
||||
}
|
||||
// Past the preview gate: a real destructive run. Against a non-local
|
||||
// scope this additionally requires --yes (or an interactive yes), so
|
||||
// `cleanup --confirm s3://…` in CI refuses rather than destroying.
|
||||
confirm_destructive("cleanup", &uri, cli.yes, json)?;
|
||||
echo_write_target(cli.quiet, "cleanup", &uri, false);
|
||||
|
||||
let options = omnigraph::db::CleanupPolicyOptions {
|
||||
keep_versions: keep,
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue