From ef55f24b35a8a173124974489df3b220f0ffa610 Mon Sep 17 00:00:00 2001 From: Mohamed Aziz Ktata Date: Wed, 7 Oct 2026 17:50:12 +0100 Subject: [PATCH 1/5] fix(cli): a non-UTF-8 argument is a usage error, not a crash std::env::args() panics on an argument that is not UTF-8, so gfs exited 101 -- the code of any crash -- with the undecodable bytes echoed in the panic message. Read args_os instead and turn the first bad argument into a clap usage error: exit 3 like any value that will not parse, the position named, the bytes never repeated. --json still gets its error object. argv[0] is converted lossily rather than refused, since it is the launch path and not something the user typed. --- README.md | 9 +-- crates/applications/cli/src/main.rs | 46 +++++++++++++- crates/applications/cli/tests/json_output.rs | 66 ++++++++++++++++++++ 3 files changed, 114 insertions(+), 7 deletions(-) diff --git a/README.md b/README.md index 1e07a8e6..85216f31 100644 --- a/README.md +++ b/README.md @@ -552,10 +552,11 @@ Exit status is meaningful: | `2` | corruption found: something referenced is missing, or an entry could not be identified | | `3` | **the check could not be completed**, so the report says nothing about the repository | -A usage error — a flag that does not exist, a value that will not parse, an unknown -subcommand — also exits `3`, for every `gfs` command and not just this one. It is -the same statement: the command did not run, so nothing here describes your -repository. `--help` and `--version` exit `0`; they are requests, not errors. +A usage error — a flag that does not exist, a value that will not parse, an argument +that is not valid UTF-8, an unknown subcommand — also exits `3`, for every `gfs` +command and not just this one. It is the same statement: the command did not run, so +nothing here describes your repository. `--help` and `--version` exit `0`; they are +requests, not errors. `3` is the one to handle first. It is not a statement about the repository — an unreadable object, a `refs/heads` entry that could not be read or is absent, a diff --git a/crates/applications/cli/src/main.rs b/crates/applications/cli/src/main.rs index abd52398..1785f74a 100644 --- a/crates/applications/cli/src/main.rs +++ b/crates/applications/cli/src/main.rs @@ -2,6 +2,8 @@ //! //! Thin wrapper around the library. See `gfs_cli::run()` for programmatic use. +use std::ffi::OsString; + use gfs_cli::output::red; use serde_json::json; @@ -21,6 +23,34 @@ fn wants_json(args: &[String]) -> bool { false } +/// The arguments as UTF-8, or the position of the first one that is not. +/// +/// `std::env::args()` panics on such an argument, which exited 101 -- the code +/// of any crash -- with a message that echoed the undecodable bytes back. The +/// position is counted from 1 for the first argument after `gfs`. +/// +/// argv[0] is exempt: it is the path the binary was launched by, not something +/// the user typed, and clap only uses it for display. +fn utf8_args(raw: Vec) -> Result, usize> { + raw.into_iter() + .enumerate() + .map(|(position, arg)| match arg.into_string() { + Ok(arg) => Ok(arg), + Err(arg) if position == 0 => Ok(arg.to_string_lossy().into_owned()), + Err(_) => Err(position), + }) + .collect() +} + +/// A usage error, so it takes the same exit code and rendering as a value clap +/// could not parse. It names the position only: the bytes are not echoed. +fn non_utf8_argument(position: usize) -> clap::Error { + clap::Error::raw( + clap::error::ErrorKind::InvalidUtf8, + format!("argument {position} is not valid UTF-8\n"), + ) +} + #[tokio::main] async fn main() { // rustls 0.23 requires an explicit process-level CryptoProvider when both @@ -43,10 +73,20 @@ async fn main() { .with_ansi(std::io::IsTerminal::is_terminal(&std::io::stderr())) .init(); - let args: Vec = std::env::args().collect(); - let wants_json = wants_json(&args); + let raw: Vec = std::env::args_os().collect(); + // Lossy is safe here: a replacement character can never produce "--json". + let lossy: Vec = raw + .iter() + .map(|arg| arg.to_string_lossy().into_owned()) + .collect(); + let wants_json = wants_json(&lossy); + + let result = match utf8_args(raw) { + Ok(args) => gfs_cli::run(args).await, + Err(position) => Err(non_utf8_argument(position).into()), + }; - match gfs_cli::run(args).await { + match result { Ok(exit_code) => std::process::exit(exit_code), Err(err) => { if wants_json { diff --git a/crates/applications/cli/tests/json_output.rs b/crates/applications/cli/tests/json_output.rs index c14b0abf..7d6f6c0a 100644 --- a/crates/applications/cli/tests/json_output.rs +++ b/crates/applications/cli/tests/json_output.rs @@ -794,3 +794,69 @@ fn fsck_plan_writes_exactly_one_artefact() { assert!(v.get("cutoff").is_some(), "plan must carry its cutoff"); assert!(v.get("report").is_some(), "plan must carry the marked set"); } + +/// Runs gfs with raw argument bytes, and optionally a raw argv[0]. +#[cfg(unix)] +fn run_gfs_raw(cwd: &Path, arg0: Option<&[u8]>, args: &[&[u8]]) -> (i32, String, String) { + use std::ffi::OsStr; + use std::os::unix::ffi::OsStrExt; + use std::os::unix::process::CommandExt; + + let mut command = Command::new(gfs_bin()); + if let Some(arg0) = arg0 { + command.arg0(OsStr::from_bytes(arg0)); + } + let out = command + .current_dir(cwd) + .args(args.iter().map(|arg| OsStr::from_bytes(arg))) + .env("RUST_LOG", "off") + .env("NO_COLOR", "1") + .output() + .expect("failed to run gfs"); + + let code = out.status.code().unwrap_or(1); + let stdout = String::from_utf8(out.stdout).expect("stdout must be utf-8"); + let stderr = String::from_utf8(out.stderr).expect("stderr must be utf-8"); + (code, stdout, stderr) +} + +/// An argument that is not UTF-8 panicked inside `std::env::args()`: exit 101, +/// the code of any crash, with the undecodable bytes echoed in the panic message. +/// It is a usage error like any other value that will not parse, so it exits 3 +/// and names the position, and never repeats the bytes back. +/// +/// Unix only: Windows arguments are UTF-16 and cannot carry these bytes. +#[cfg(unix)] +#[test] +fn an_argument_that_is_not_utf8_is_a_usage_error_not_a_crash() { + let tmp = TempDir::new().unwrap(); + assert_eq!(run_gfs(tmp.path(), &["init", "."]).0, 0, "init"); + + let (code, stdout, stderr) = + run_gfs_raw(tmp.path(), None, &[b"commit", b"-m", b"echomarker\xe9"]); + assert_eq!(code, 3, "a usage error, not a crash: {stderr}"); + assert!(stdout.is_empty(), "nothing on stdout: {stdout}"); + assert!( + stderr.contains("argument 3 is not valid UTF-8"), + "names the position: {stderr}" + ); + assert!(!stderr.contains("panicked"), "no panic: {stderr}"); + assert!( + !stderr.contains("echomarker"), + "the argument is not echoed: {stderr}" + ); + + let (code, stdout, stderr) = run_gfs_raw(tmp.path(), None, &[b"--json", b"log", b"\xff\xfe"]); + assert_eq!(code, 3, "the same code under --json: {stderr}"); + let v = assert_stdout_json(&stdout); + let message = v["error"]["message"].as_str().unwrap_or_default(); + assert!( + message.contains("argument 3 is not valid UTF-8"), + "the JSON error names the position: {v}" + ); + + // The control: argv[0] is the path the binary was launched by, not user + // input, so a non-UTF-8 one must not turn every command into an error. + let (code, _, stderr) = run_gfs_raw(tmp.path(), Some(b"gfs\xff"), &[b"--version"]); + assert_eq!(code, 0, "argv[0] is not an argument: {stderr}"); +} From a72a9d9faca17a0eb6d45bbd2f095ab949f9c5d0 Mon Sep 17 00:00:00 2001 From: Mohamed Aziz Ktata Date: Wed, 7 Oct 2026 18:04:15 +0100 Subject: [PATCH 2/5] fix(cli): a closed reader never panics a command Every bare println! panicked on BrokenPipe, so `gfs status | head -1` exited 101 -- the code of a crash -- and `gfs schema diff | head` turned "breaking changes" (2) into the same 101. All 335 now go through println_safe!, which records the closed reader and leaves the exit status to the command. Nine printing helpers returned () and now return Result, so a write error other than a closed pipe propagates instead of panicking; every caller was found by compiling with unused_must_use denied. The one deliberate discard is main's --json error object, written just before the process exits with that error's own code. println_safe!() gains an empty arm for the 54 bare newlines. --- .../cli/src/commands/cmd_branch.rs | 65 ++--- .../cli/src/commands/cmd_checkout.rs | 13 +- .../cli/src/commands/cmd_clone.rs | 25 +- .../cli/src/commands/cmd_commit.rs | 9 +- .../cli/src/commands/cmd_compute.rs | 62 ++--- .../cli/src/commands/cmd_config.rs | 3 +- .../cli/src/commands/cmd_destroy.rs | 9 +- .../cli/src/commands/cmd_export.rs | 9 +- .../cli/src/commands/cmd_freeze.rs | 79 +++--- .../cli/src/commands/cmd_import.rs | 9 +- .../applications/cli/src/commands/cmd_init.rs | 19 +- .../applications/cli/src/commands/cmd_mcp.rs | 44 ++-- .../cli/src/commands/cmd_providers.rs | 63 ++--- .../cli/src/commands/cmd_proxy.rs | 58 +++-- .../cli/src/commands/cmd_schema.rs | 35 +-- .../cli/src/commands/cmd_source.rs | 239 +++++++++--------- .../cli/src/commands/cmd_status.rs | 132 +++++----- .../applications/cli/src/commands/cmd_user.rs | 70 ++--- .../cli/src/commands/cmd_version.rs | 27 +- crates/applications/cli/src/lib.rs | 54 ++-- crates/applications/cli/src/main.rs | 5 +- crates/applications/cli/src/output.rs | 3 + crates/applications/cli/tests/json_output.rs | 77 ++++++ 23 files changed, 619 insertions(+), 490 deletions(-) diff --git a/crates/applications/cli/src/commands/cmd_branch.rs b/crates/applications/cli/src/commands/cmd_branch.rs index aa190fbe..d4cdb075 100644 --- a/crates/applications/cli/src/commands/cmd_branch.rs +++ b/crates/applications/cli/src/commands/cmd_branch.rs @@ -5,6 +5,7 @@ //! - `gfs branch ` — create a new branch at a specific commit/branch //! - `gfs branch -d ` — delete a branch +use crate::println_safe; use std::path::PathBuf; use std::sync::Arc; @@ -72,13 +73,13 @@ fn list_branches(repo_path: &std::path::Path, json_output: bool) -> Result<()> { let branches = list_branch_tips(repo_path, false)?; if branches.is_empty() { if json_output { - println!( + println_safe!( "{}", serde_json::to_string_pretty(&json!({ "branches": [] }))? - ); + )?; return Ok(()); } - println!(" (no branches)"); + println_safe!(" (no branches)")?; return Ok(()); } @@ -115,10 +116,10 @@ fn list_branches(repo_path: &std::path::Path, json_output: bool) -> Result<()> { }) }) .collect(); - println!( + println_safe!( "{}", serde_json::to_string_pretty(&json!({ "branches": out }))? - ); + )?; return Ok(()); } @@ -135,15 +136,15 @@ fn list_branches(repo_path: &std::path::Path, json_output: bool) -> Result<()> { }; if *name == current { - println!( + println_safe!( " {} {} {} {}", gold("*"), green(name), dimmed(short_hash), subject - ); + )?; } else { - println!(" {} {} {}", cyan(name), dimmed(short_hash), subject); + println_safe!(" {} {} {}", cyan(name), dimmed(short_hash), subject)?; } } @@ -201,7 +202,7 @@ async fn create_branch( let start_label = start_point.unwrap_or("HEAD"); if json_output { - println!( + println_safe!( "{}", serde_json::to_string_pretty(&json!({ "action": "create", @@ -209,18 +210,18 @@ async fn create_branch( "hash": commit_hash, "start_point": start_label, }))? - ); + )?; return Ok(()); } let short_hash = &commit_hash[..7.min(commit_hash.len())]; - println!( + println_safe!( "{} Created branch '{}' at {} ({})", green("✓"), cyan(name), start_label, dimmed(short_hash) - ); + )?; } Ok(()) @@ -312,7 +313,7 @@ fn delete_branch(repo_path: &std::path::Path, name: &str, json_output: bool) -> } if json_output { - println!( + println_safe!( "{}", serde_json::to_string_pretty(&json!({ "action": "delete", @@ -320,17 +321,17 @@ fn delete_branch(repo_path: &std::path::Path, name: &str, json_output: bool) -> "commit": deleted.commit_hash, "recoverable": true, }))? - ); + )?; return Ok(()); } - println!("{} Deleted branch '{}'", green("✓"), name); - println!( + println_safe!("{} Deleted branch '{}'", green("✓"), name)?; + println_safe!( " {}", dimmed(format!( "restore with: gfs branch --restore {name} (restores committed work)" )) - ); + )?; Ok(()) } @@ -403,25 +404,25 @@ fn list_deleted(repo_path: &std::path::Path, json_output: bool) -> Result<()> { }) }) .collect(); - println!( + println_safe!( "{}", serde_json::to_string_pretty(&json!({ "deleted": rows }))? - ); + )?; return Ok(()); } if entries.is_empty() { - println!("{}", dimmed("No deleted branches are recoverable.")); + println_safe!("{}", dimmed("No deleted branches are recoverable."))?; return Ok(()); } - println!( + println_safe!( "{}", dimmed(format!( "Recoverable for {} days after deletion:", deleted_retention_days(repo_path) )) - ); + )?; // Entries are newest-first, so the first occurrence of a name is the one // `--restore` would pick. Repeated deletions of the same name are shown // rather than collapsed, but only one of them is actionable. @@ -430,16 +431,16 @@ fn list_deleted(repo_path: &std::path::Path, json_output: bool) -> Result<()> { let short: String = d.commit_hash.chars().take(7).collect(); let newest = !seen.contains(&d.name.as_str()); seen.push(d.name.as_str()); - println!( + println_safe!( " {} {} {}{}", cyan(&d.name), gold(&short), dimmed(format_age(d.deleted_at_ms)), if newest { "" } else { " (older deletion)" } - ); + )?; } - println!(); - println!("{}", dimmed("restore with: gfs branch --restore ")); + println_safe!()?; + println_safe!("{}", dimmed("restore with: gfs branch --restore "))?; Ok(()) } @@ -541,27 +542,27 @@ fn restore_branch(repo_path: &std::path::Path, name: &str, json_output: bool) -> .with_context(|| format!("failed to restore branch '{}'", name))?; if json_output { - println!( + println_safe!( "{}", serde_json::to_string_pretty(&json!({ "action": "restore", "branch": restored.name, "commit": restored.commit_hash, }))? - ); + )?; return Ok(()); } let short: String = restored.commit_hash.chars().take(7).collect(); - println!( + println_safe!( "{} Restored branch '{}' at {}", green("\u{2713}"), restored.name, gold(&short) - ); - println!( + )?; + println_safe!( " {}", dimmed(format!("check it out with: gfs checkout {}", restored.name)) - ); + )?; Ok(()) } diff --git a/crates/applications/cli/src/commands/cmd_checkout.rs b/crates/applications/cli/src/commands/cmd_checkout.rs index 8bbc75b4..3761af75 100644 --- a/crates/applications/cli/src/commands/cmd_checkout.rs +++ b/crates/applications/cli/src/commands/cmd_checkout.rs @@ -4,6 +4,7 @@ //! When the repo has a compute container, the use case stops it before checkout //! and starts (or recreates with the new workspace mount) after checkout. +use crate::println_safe; use std::path::PathBuf; use std::sync::Arc; @@ -175,30 +176,30 @@ pub async fn checkout( }; if json_output { - println!( + println_safe!( "{}", json!({ "hash": commit_hash, "branch": create_branch.as_deref().unwrap_or(&revision), "new_branch": create_branch.is_some(), }) - ); + )?; } else { let short_hash = &commit_hash[..7.min(commit_hash.len())]; if let Some(ref name) = create_branch { - println!( + println_safe!( "{} Switched to new branch '{}' ({})", green("✓"), green(name.trim()), dimmed(short_hash) - ); + )?; } else { - println!( + println_safe!( "{} Switched to {} ({})", green("✓"), cyan(revision.trim()), dimmed(short_hash) - ); + )?; } } Ok(()) diff --git a/crates/applications/cli/src/commands/cmd_clone.rs b/crates/applications/cli/src/commands/cmd_clone.rs index e47e8c1a..9ac178a3 100644 --- a/crates/applications/cli/src/commands/cmd_clone.rs +++ b/crates/applications/cli/src/commands/cmd_clone.rs @@ -1,3 +1,4 @@ +use crate::println_safe; use std::path::PathBuf; use std::sync::Arc; @@ -64,7 +65,7 @@ pub async fn clone( None => match use_case.detect_remote_version(&remote).await { Ok(v) => { if !json_output { - println!(" {} Detected remote version {}", green("✓"), cyan(&v)); + println_safe!(" {} Detected remote version {}", green("✓"), cyan(&v))?; } Some(v) } @@ -110,25 +111,25 @@ pub async fn clone( let output = use_case.run(&target_path, remote).await?; if json_output && !snapshot { - println!( + println_safe!( "{}", serde_json::to_string_pretty(&json!({ "path": target_path.display().to_string(), "remote": output.remote, "mode": "lazy-clone", }))? - ); + )?; } else if !json_output { - println!(); - println!( + println_safe!()?; + println_safe!( " {} Lazy clone ready from {}", green("✓"), cyan(output.remote) - ); - println!( + )?; + println_safe!( " {:<16} copy-on-read (data fetched on first read)", dimmed("Mode") - ); + )?; } if !output.stderr.is_empty() { eprintln!("{}", output.stderr.trim_end()); @@ -140,8 +141,8 @@ pub async fn clone( // clone, and the error says so instead of pretending the clone broke. if snapshot { if !json_output { - println!(); - println!(" {} taking the snapshot (gfs freeze)...", dimmed("\u{b7}")); + println_safe!()?; + println_safe!(" {} taking the snapshot (gfs freeze)...", dimmed("\u{b7}"))?; } if let Err(e) = crate::commands::cmd_freeze::freeze( Some(target_path.clone()), @@ -158,11 +159,11 @@ pub async fn clone( .into()); } if !json_output { - println!( + println_safe!( " {} Snapshot clone ready {}", green("\u{2713}"), dimmed("(a point in time, detached from its source)") - ); + )?; } } diff --git a/crates/applications/cli/src/commands/cmd_commit.rs b/crates/applications/cli/src/commands/cmd_commit.rs index 7db41de5..2a588b41 100644 --- a/crates/applications/cli/src/commands/cmd_commit.rs +++ b/crates/applications/cli/src/commands/cmd_commit.rs @@ -1,3 +1,4 @@ +use crate::println_safe; use std::path::PathBuf; use std::sync::Arc; @@ -115,23 +116,23 @@ async fn run( .map_err(|e| anyhow::anyhow!("{e}"))?; if json_output { - println!( + println_safe!( "{}", json!({ "hash": commit_hash, "branch": branch, "message": message, }) - ); + )?; } else { let short = &commit_hash[..7.min(commit_hash.len())]; - println!( + println_safe!( "{} [{}] {} {}", green("✓"), cyan(&branch), dimmed(short), message - ); + )?; } Ok(()) } diff --git a/crates/applications/cli/src/commands/cmd_compute.rs b/crates/applications/cli/src/commands/cmd_compute.rs index 65ae458f..d954c519 100644 --- a/crates/applications/cli/src/commands/cmd_compute.rs +++ b/crates/applications/cli/src/commands/cmd_compute.rs @@ -1,3 +1,4 @@ +use crate::println_safe; use std::path::PathBuf; use std::sync::Arc; @@ -121,21 +122,21 @@ fn handle_config(path: Option, key: &str, value: &str, json_output: boo } config.save(&repo_path).context("failed to save config")?; if json_output { - println!( + println_safe!( "{}", serde_json::to_string_pretty(&json!({ "action": "config_set", "key": "db.port", "value": port, }))? - ); + )?; return Ok(()); } - println!( + println_safe!( "{} database_port updated to {}. Run 'gfs compute restart' to apply.", green("✓"), port - ); + )?; Ok(()) } _ => anyhow::bail!("unknown config key '{}'; supported keys: db.port", key), @@ -166,7 +167,7 @@ async fn dispatch_dyn( if json_output { print_status_json(&status, data_dir, path.as_ref(), None)?; } else { - print_status(&status, data_dir, path.as_ref()); + print_status(&status, data_dir, path.as_ref())?; } } @@ -177,8 +178,8 @@ async fn dispatch_dyn( if json_output { print_status_json(&status, data_dir, path.as_ref(), Some("start"))?; } else { - println!("{} Compute started", green("✓")); - print_status(&status, data_dir, path.as_ref()); + println_safe!("{} Compute started", green("✓"))?; + print_status(&status, data_dir, path.as_ref())?; } } @@ -190,8 +191,8 @@ async fn dispatch_dyn( if json_output { print_status_json(&status, None, path.as_ref(), Some("stop"))?; } else { - println!("{} Compute stopped", green("✓")); - print_status(&status, None, path.as_ref()); + println_safe!("{} Compute stopped", green("✓"))?; + print_status(&status, None, path.as_ref())?; } } @@ -201,8 +202,8 @@ async fn dispatch_dyn( if json_output { print_status_json(&status, data_dir, path.as_ref(), Some("restart"))?; } else { - println!("{} Compute restarted", green("✓")); - print_status(&status, data_dir, path.as_ref()); + println_safe!("{} Compute restarted", green("✓"))?; + print_status(&status, data_dir, path.as_ref())?; } } @@ -214,8 +215,8 @@ async fn dispatch_dyn( if json_output { print_status_json(&status, None, path.as_ref(), Some("pause"))?; } else { - println!("{} Compute paused", green("✓")); - print_status(&status, None, path.as_ref()); + println_safe!("{} Compute paused", green("✓"))?; + print_status(&status, None, path.as_ref())?; } } @@ -227,8 +228,8 @@ async fn dispatch_dyn( if json_output { print_status_json(&status, None, path.as_ref(), Some("unpause"))?; } else { - println!("{} Compute unpaused", green("✓")); - print_status(&status, None, path.as_ref()); + println_safe!("{} Compute unpaused", green("✓"))?; + print_status(&status, None, path.as_ref())?; } } @@ -276,19 +277,19 @@ async fn dispatch_dyn( }) }) .collect(); - println!( + println_safe!( "{}", serde_json::to_string_pretty(&json!({ "action": "logs", "id": instance_id.0, "entries": out, }))? - ); + )?; return Ok(()); } for entry in &entries { - println!( + println_safe!( "[{}] [{}] {}", entry.timestamp.format("%Y-%m-%dT%H:%M:%SZ"), match entry.stream { @@ -296,7 +297,7 @@ async fn dispatch_dyn( gfs_domain::ports::compute::LogStream::Stderr => "stderr", }, entry.message.trim_end() - ); + )?; } } } @@ -320,7 +321,7 @@ fn print_status_json( let repo_path = path.cloned().unwrap_or_else(get_repo_dir); let rel_data_dir = data_dir.map(|d| relativize_to_repo(&repo_path, d)); - println!( + println_safe!( "{}", serde_json::to_string_pretty(&json!({ "action": action, @@ -333,17 +334,17 @@ fn print_status_json( "data_dir": rel_data_dir, } }))? - ); + )?; Ok(()) } -fn print_status(s: &InstanceStatus, data_dir: Option<&str>, path: Option<&PathBuf>) { - println!("{}", box_top(&bold("Compute"), BOX_W)); +fn print_status(s: &InstanceStatus, data_dir: Option<&str>, path: Option<&PathBuf>) -> Result<()> { + println_safe!("{}", box_top(&bold("Compute"), BOX_W))?; // ID let truncated_id = truncate_id(&s.id.0); let row = fmt_box_row_colored("id", &dimmed(&truncated_id), &truncated_id, LABEL_W, BOX_W); - println!("{}", box_row(&row, BOX_W)); + println_safe!("{}", box_row(&row, BOX_W))?; // State with dot indicator let state_str = format_state(&s.state); @@ -351,27 +352,27 @@ fn print_status(s: &InstanceStatus, data_dir: Option<&str>, path: Option<&PathBu let colored_state = format!("{} {}", dot, format_state_colored_text(&s.state)); let raw_state = format!("{} {}", status_indicator_raw(&s.state), state_str); let row = fmt_box_row_colored("state", &colored_state, &raw_state, LABEL_W, BOX_W); - println!("{}", box_row(&row, BOX_W)); + println_safe!("{}", box_row(&row, BOX_W))?; // PID if let Some(pid) = s.pid { let pid_str = pid.to_string(); let row = fmt_box_row("pid", &pid_str, LABEL_W, BOX_W); - println!("{}", box_row(&row, BOX_W)); + println_safe!("{}", box_row(&row, BOX_W))?; } // Started at if let Some(started_at) = s.started_at { let ts = started_at.format("%Y-%m-%dT%H:%M:%SZ").to_string(); let row = fmt_box_row("started_at", &ts, LABEL_W, BOX_W); - println!("{}", box_row(&row, BOX_W)); + println_safe!("{}", box_row(&row, BOX_W))?; } // Exit code if let Some(code) = s.exit_code { let code_str = code.to_string(); let row = fmt_box_row("exit_code", &code_str, LABEL_W, BOX_W); - println!("{}", box_row(&row, BOX_W)); + println_safe!("{}", box_row(&row, BOX_W))?; } // Container data dir @@ -379,10 +380,11 @@ fn print_status(s: &InstanceStatus, data_dir: Option<&str>, path: Option<&PathBu let repo_path = path.cloned().unwrap_or_else(get_repo_dir); let rel = relativize_to_repo(&repo_path, dir); let row = fmt_box_row("data dir", &rel, LABEL_W, BOX_W); - println!("{}", box_row(&row, BOX_W)); + println_safe!("{}", box_row(&row, BOX_W))?; } - println!("{}", box_bottom(BOX_W)); + println_safe!("{}", box_bottom(BOX_W))?; + Ok(()) } fn format_state(state: &InstanceState) -> &'static str { diff --git a/crates/applications/cli/src/commands/cmd_config.rs b/crates/applications/cli/src/commands/cmd_config.rs index 6ba42834..5dabab81 100644 --- a/crates/applications/cli/src/commands/cmd_config.rs +++ b/crates/applications/cli/src/commands/cmd_config.rs @@ -6,6 +6,7 @@ //! //! Resolution order for commits: CLI flag → local → global → git config. +use crate::println_safe; use std::path::PathBuf; use anyhow::{Context, Result}; @@ -284,7 +285,7 @@ fn get_global(key: &str) -> Result<()> { } } KEY_TELEMETRY_ENABLED => { - println!("{}", settings.telemetry); + println_safe!("{}", settings.telemetry)?; } _ => { anyhow::bail!( diff --git a/crates/applications/cli/src/commands/cmd_destroy.rs b/crates/applications/cli/src/commands/cmd_destroy.rs index 69d9cf4e..0a7c3373 100644 --- a/crates/applications/cli/src/commands/cmd_destroy.rs +++ b/crates/applications/cli/src/commands/cmd_destroy.rs @@ -10,6 +10,7 @@ //! reclaims the PVCs and per-commit VolumeSnapshots — so only the on-disk `.gfs` //! metadata is removed. +use crate::println_safe; use std::io::Write; use std::path::{Path, PathBuf}; use std::sync::Arc; @@ -66,7 +67,9 @@ pub async fn destroy(path: Option, yes: bool) -> Result<()> { let id = InstanceId(name.to_string()); let _ = compute.stop(&id).await; match compute.remove_instance(&id).await { - Ok(()) => println!(" {} removed compute instance {}", green("✓"), cyan(name)), + Ok(()) => { + println_safe!(" {} removed compute instance {}", green("✓"), cyan(name))? + } Err(e) => { eprintln!( " {} could not remove instance {name}: {e} (continuing)", @@ -104,11 +107,11 @@ pub async fn destroy(path: Option, yes: bool) -> Result<()> { gfs_dir.display() ); } - println!( + println_safe!( " {} destroyed GFS repository at {}", green("✓"), cyan(repo_path.display().to_string()) - ); + )?; Ok(()) } diff --git a/crates/applications/cli/src/commands/cmd_export.rs b/crates/applications/cli/src/commands/cmd_export.rs index 8ceca5b3..1a47ab1a 100644 --- a/crates/applications/cli/src/commands/cmd_export.rs +++ b/crates/applications/cli/src/commands/cmd_export.rs @@ -1,5 +1,6 @@ //! `gfs export` — export data from the running database instance. +use crate::println_safe; use std::path::PathBuf; use std::sync::Arc; @@ -49,19 +50,19 @@ pub async fn run( let output = use_case.run(&repo_path, output_dir, &format).await?; if json_output { - println!( + println_safe!( "{}", json!({ "file_path": output.file_path.display().to_string(), "format": format, }) - ); + )?; } else { - println!( + println_safe!( "{} Exported to {}", green("✓"), cyan(output.file_path.display().to_string()) - ); + )?; } if !output.stderr.is_empty() { eprintln!("{}", output.stderr.trim_end()); diff --git a/crates/applications/cli/src/commands/cmd_freeze.rs b/crates/applications/cli/src/commands/cmd_freeze.rs index 0563b39f..1a58d8f5 100644 --- a/crates/applications/cli/src/commands/cmd_freeze.rs +++ b/crates/applications/cli/src/commands/cmd_freeze.rs @@ -13,6 +13,7 @@ //! raises it or `--force` bypasses it. For big sources the better answer is //! cloning from an already-frozen endpoint (snapshot, backup, paused replica). +use crate::println_safe; use std::path::PathBuf; use anyhow::{Context, Result, bail}; @@ -40,20 +41,20 @@ pub async fn freeze( // satisfied, not an error. if let Some((true, at, _lsn)) = frozen_info(&repo).await { if json_output { - println!( + println_safe!( "{}", serde_json::to_string_pretty( &json!({ "frozen": true, "already": true, "frozen_at": at }) )? - ); + )?; } else { - println!(); - println!( + println_safe!()?; + println_safe!( " {} already frozen {}", green("✓"), dimmed(format!("(at {at})")) - ); - println!(); + )?; + println_safe!()?; } return Ok(()); } @@ -95,34 +96,34 @@ pub async fn freeze( let cap = max_bytes.unwrap_or(DEFAULT_MAX_BYTES); if est_bytes > cap && !force { if !json_output { - println!(); - println!( + println_safe!()?; + println_safe!( " {} freezing copies everything: ~{} from the source ({} table(s))", yellow("!"), bold(fmt_bytes(est_bytes)), n_copy - ); - println!( + )?; + println_safe!( " {}", dimmed(format!( "that exceeds the {} budget, so nothing was copied", fmt_bytes(cap) )) - ); - println!( + )?; + println_safe!( " {}", dimmed(format!( "raise it with `gfs freeze --max-bytes {est_bytes}` or bypass with --force" )) - ); - println!( + )?; + println_safe!( " {}", dimmed( "for a large source, prefer cloning from an already-frozen endpoint \ - (storage snapshot, backup, paused replica): lazy AND a point in time" + (storage snapshot, backup, paused replica): lazy AND a point in time" ) - ); - println!(); + )?; + println_safe!()?; } bail!( "estimated copy of {} exceeds the {} budget (nothing was copied)", @@ -132,13 +133,13 @@ pub async fn freeze( } if !json_output { - println!(); - println!( + println_safe!()?; + println_safe!( " {} freezing: copying ~{} from one source instant ({} table(s))...", dimmed("\u{b7}"), fmt_bytes(est_bytes), n_copy - ); + )?; } // Phase B: ONE statement = ONE transaction = atomic. Everything -- the @@ -170,7 +171,7 @@ pub async fn freeze( let frozen_line = actions.iter().find(|a| a[0] == "frozen"); if json_output { - println!( + println_safe!( "{}", serde_json::to_string_pretty(&json!({ "frozen": true, @@ -182,21 +183,21 @@ pub async fn freeze( "action": a[0], "table": a[1], "detail": a[2] })).collect::>(), }))? - ); + )?; return Ok(()); } - println!(); + println_safe!()?; if already { - println!(" {} already frozen", green("\u{2713}")); - println!(); + println_safe!(" {} already frozen", green("\u{2713}"))?; + println_safe!()?; return Ok(()); } let lsn = frozen_line .and_then(|a| a[2].split("source LSN ").nth(1).map(str::to_string)) .unwrap_or_default(); - println!( + println_safe!( " {} clone is now a snapshot: {} table(s) copied from one instant{}", green("\u{2713}"), bold(copied.to_string()), @@ -205,36 +206,36 @@ pub async fn freeze( } else { format!(" {}", dimmed(format!("(source LSN {lsn})"))) } - ); + )?; if matviews > 0 || seqs > 0 { - println!( + println_safe!( " {} {} matview(s) recomputed, {} sequence(s) advanced", green("\u{2713}"), matviews, seqs - ); + )?; } if !kept.is_empty() { - println!(); - println!( + println_safe!()?; + println_safe!( " {} {} table(s) kept your local writes", yellow("!"), bold(kept.len().to_string()) - ); + )?; for k in &kept { - println!(" {} {}", red("kept"), cyan(&k[1])); + println_safe!(" {} {}", red("kept"), cyan(&k[1]))?; } - println!( + println_safe!( " {}", dimmed("kept tables are your branch; their source-derived rows may predate the freeze") - ); + )?; } - println!(); - println!( + println_safe!()?; + println_safe!( " {} detached from source {}", green("\u{2713}"), dimmed("(fetch/pull are disabled; the source can go away and this clone still answers)") - ); - println!(); + )?; + println_safe!()?; Ok(()) } diff --git a/crates/applications/cli/src/commands/cmd_import.rs b/crates/applications/cli/src/commands/cmd_import.rs index a2956833..1b85a6f5 100644 --- a/crates/applications/cli/src/commands/cmd_import.rs +++ b/crates/applications/cli/src/commands/cmd_import.rs @@ -1,5 +1,6 @@ //! `gfs import` — import data into the running database instance. +use crate::println_safe; use std::path::PathBuf; use std::sync::Arc; @@ -46,18 +47,18 @@ pub async fn run( let output = use_case.run(&repo_path, file, format_str).await?; if json_output { - println!( + println_safe!( "{}", json!({ "imported_from": output.imported_from.display().to_string(), }) - ); + )?; } else { - println!( + println_safe!( "{} Imported from {}", green("✓"), cyan(output.imported_from.display().to_string()) - ); + )?; } if !output.stderr.is_empty() { eprintln!("{}", output.stderr.trim_end()); diff --git a/crates/applications/cli/src/commands/cmd_init.rs b/crates/applications/cli/src/commands/cmd_init.rs index 932941bb..54aa6bc8 100644 --- a/crates/applications/cli/src/commands/cmd_init.rs +++ b/crates/applications/cli/src/commands/cmd_init.rs @@ -1,3 +1,4 @@ +use crate::println_safe; use std::path::PathBuf; use std::sync::Arc; @@ -167,7 +168,7 @@ pub async fn init( } if json_output { - println!( + println_safe!( "{}", serde_json::to_string_pretty(&json!({ "path": target_path.display().to_string(), @@ -176,21 +177,21 @@ pub async fn init( "provider": provider_display, "connection_string": connection_string, }))? - ); + )?; } else { - println!( + println_safe!( " {} Initialized GFS repository at {}", green("✓"), cyan(target_path.display().to_string()) - ); - println!(); - println!(" {:<16} {}", dimmed("Branch"), cyan("main")); - println!(" {:<16} .gfs/config.toml", dimmed("Config")); + )?; + println_safe!()?; + println_safe!(" {:<16} {}", dimmed("Branch"), cyan("main"))?; + println_safe!(" {:<16} .gfs/config.toml", dimmed("Config"))?; if let Some(ref provider) = provider_display { - println!(" {:<16} {}", dimmed("Provider"), cyan(provider)); + println_safe!(" {:<16} {}", dimmed("Provider"), cyan(provider))?; } if let Some(ref c) = connection_string { - println!(" {:<16} {}", dimmed("Connection"), cyan(c)); + println_safe!(" {:<16} {}", dimmed("Connection"), cyan(c))?; } } diff --git a/crates/applications/cli/src/commands/cmd_mcp.rs b/crates/applications/cli/src/commands/cmd_mcp.rs index 9fc02af0..77a3bbd2 100644 --- a/crates/applications/cli/src/commands/cmd_mcp.rs +++ b/crates/applications/cli/src/commands/cmd_mcp.rs @@ -7,6 +7,7 @@ //! //! PID file: .gfs/mcp.pid, log file: .gfs/mcp.log. +use crate::println_safe; use std::fs; use std::net::SocketAddr; use std::path::PathBuf; @@ -117,7 +118,7 @@ async fn run_web_embedded(port: u16) -> Result<()> { bind ); - println!("Starting MCP server on http://127.0.0.1:{}/mcp", port); + println_safe!("Starting MCP server on http://127.0.0.1:{}/mcp", port)?; if generated { let path = token_file_path().ok_or_else(|| { anyhow::anyhow!("no writable location for a generated token; set GFS_MCP_TOKEN") @@ -128,9 +129,9 @@ async fn run_web_embedded(port: u16) -> Result<()> { path.display() ) })?; - println!("Bearer token written to {}", path.display()); + println_safe!("Bearer token written to {}", path.display())?; } else { - println!("Bearer token taken from GFS_MCP_TOKEN"); + println_safe!("Bearer token taken from GFS_MCP_TOKEN")?; } let listener = tokio::net::TcpListener::bind(bind).await?; @@ -225,13 +226,13 @@ async fn spawn_daemon_process( fs::write(pid_file, pid.to_string()).context("write PID file")?; drop(child); - println!( + println_safe!( "MCP daemon started (PID {}, http://127.0.0.1:{}/mcp, repo {}). {}.", pid, port, repo_path.display(), token_note - ); + )?; Ok(()) } else { anyhow::bail!( @@ -250,20 +251,20 @@ async fn stop(pid_file: &std::path::Path) -> Result<()> { let pid = match read_pid(pid_file)? { Some(p) => p, None => { - println!("MCP daemon is not running (no PID file)"); + println_safe!("MCP daemon is not running (no PID file)")?; return Ok(()); } }; if !process_exists(pid) { fs::remove_file(pid_file).ok(); - println!("MCP daemon is not running (stale PID {})", pid); + println_safe!("MCP daemon is not running (stale PID {})", pid)?; return Ok(()); } kill_process(pid)?; fs::remove_file(pid_file).context("remove PID file")?; - println!("MCP daemon stopped (PID {})", pid); + println_safe!("MCP daemon stopped (PID {})", pid)?; Ok(()) } @@ -272,8 +273,8 @@ async fn stop(pid_file: &std::path::Path) -> Result<()> { // --------------------------------------------------------------------------- async fn status(pid_file: &std::path::Path, default_port: u16) -> Result<()> { - println!("MCP Embedded Handler Status"); - println!(); + println_safe!("MCP Embedded Handler Status")?; + println_safe!()?; let running = pid_file .exists() @@ -282,21 +283,22 @@ async fn status(pid_file: &std::path::Path, default_port: u16) -> Result<()> { let running = running.and_then(|pid| process_exists(pid).then_some(pid)); if let Some(pid) = running { - println!( + println_safe!( "Daemon: running (PID {}, http://127.0.0.1:{}/mcp, bearer token required)", - pid, default_port - ); + pid, + default_port + )?; } else if pid_file.exists() { - println!("Daemon: stopped (use 'gfs mcp stop' to remove stale PID file)"); + println_safe!("Daemon: stopped (use 'gfs mcp stop' to remove stale PID file)")?; } else { - println!("Daemon: stopped"); + println_safe!("Daemon: stopped")?; } - println!(); - println!("Embedded MCP modes:"); - println!(" - Stdio (default): gfs mcp"); - println!(" - HTTP (foreground): gfs mcp web --port 3000"); - println!(" - HTTP (daemon): gfs mcp start"); + println_safe!()?; + println_safe!("Embedded MCP modes:")?; + println_safe!(" - Stdio (default): gfs mcp")?; + println_safe!(" - HTTP (foreground): gfs mcp web --port 3000")?; + println_safe!(" - HTTP (daemon): gfs mcp start")?; Ok(()) } @@ -306,7 +308,7 @@ async fn status(pid_file: &std::path::Path, default_port: u16) -> Result<()> { fn show_version() -> Result<()> { const VERSION: &str = env!("CARGO_PKG_VERSION"); - println!("gfs-mcp version: {}", VERSION); + println_safe!("gfs-mcp version: {}", VERSION)?; Ok(()) } diff --git a/crates/applications/cli/src/commands/cmd_providers.rs b/crates/applications/cli/src/commands/cmd_providers.rs index d2756217..69b7e592 100644 --- a/crates/applications/cli/src/commands/cmd_providers.rs +++ b/crates/applications/cli/src/commands/cmd_providers.rs @@ -1,5 +1,6 @@ //! `gfs providers` — list database providers and their supported versions. +use crate::println_safe; use std::sync::Arc; use anyhow::{Context, Result}; @@ -44,7 +45,7 @@ pub fn run(provider_name: Option, json_output: bool) -> Result<()> { fn print_all_providers(registry: &impl DatabaseProviderRegistry) -> Result<()> { let names = registry.list(); if names.is_empty() { - println!(" (no providers registered)"); + println_safe!(" (no providers registered)")?; return Ok(()); } @@ -63,7 +64,7 @@ fn print_all_providers(registry: &impl DatabaseProviderRegistry) -> Result<()> { }) .collect(); - print_providers_table(&rows); + print_providers_table(&rows)?; Ok(()) } @@ -81,10 +82,10 @@ fn print_all_providers_json(registry: &impl DatabaseProviderRegistry) -> Result< }) .collect(); - println!( + println_safe!( "{}", serde_json::to_string_pretty(&json!({ "providers": providers }))? - ); + )?; Ok(()) } @@ -96,13 +97,13 @@ fn print_provider_detail(registry: &impl DatabaseProviderRegistry, name: &str) - let versions = provider.supported_versions(); let features = provider.supported_features(); - println!(" {} {}", bold("Provider:"), cyan(name)); - println!(); - println!(" Supported versions: {}", versions.join(", ")); - println!(); - print_features_table(&features); - println!(); - println!(" Images are pulled from Docker Hub by default."); + println_safe!(" {} {}", bold("Provider:"), cyan(name))?; + println_safe!()?; + println_safe!(" Supported versions: {}", versions.join(", "))?; + println_safe!()?; + print_features_table(&features)?; + println_safe!()?; + println_safe!(" Images are pulled from Docker Hub by default.")?; Ok(()) } @@ -123,7 +124,7 @@ fn print_provider_detail_json(registry: &impl DatabaseProviderRegistry, name: &s "images_source": "docker_hub", } }); - println!("{}", serde_json::to_string_pretty(&out)?); + println_safe!("{}", serde_json::to_string_pretty(&out)?)?; Ok(()) } @@ -135,17 +136,17 @@ const COL_PROVIDER: usize = 20; const COL_VERSION: usize = 30; const COL_FEATURES: usize = 30; -fn print_providers_table(rows: &[(String, String, String)]) { +fn print_providers_table(rows: &[(String, String, String)]) -> Result<()> { let cols = [COL_PROVIDER, COL_VERSION, COL_FEATURES]; // Top border: ┌──────┬──────┬──────┐ - println!("{}", tbl_rule(&cols, TBL_TL, TBL_T_DOWN, TBL_TR)); + println_safe!("{}", tbl_rule(&cols, TBL_TL, TBL_T_DOWN, TBL_TR))?; // Header row let h_provider = format!("{: Result<()> { let cols = [COL_FEATURE, COL_DESC]; - println!(" {}", bold("Features")); + println_safe!(" {}", bold("Features"))?; // Top border - println!("{}", tbl_rule(&cols, TBL_TL, TBL_T_DOWN, TBL_TR)); + println_safe!("{}", tbl_rule(&cols, TBL_TL, TBL_T_DOWN, TBL_TR))?; // Header let h_feat = format!("{:, max_len: usize) -> String { diff --git a/crates/applications/cli/src/commands/cmd_proxy.rs b/crates/applications/cli/src/commands/cmd_proxy.rs index 7bc45b46..a489e242 100644 --- a/crates/applications/cli/src/commands/cmd_proxy.rs +++ b/crates/applications/cli/src/commands/cmd_proxy.rs @@ -17,6 +17,7 @@ //! The binary is located via `GFS_PROXY_BIN`, then next to the `gfs` exe, then //! `PATH`. A helpful error points at `cargo build -p guepard-proxy-v2` otherwise. +use crate::println_safe; use std::ffi::OsString; use std::fs; use std::path::{Path, PathBuf}; @@ -171,11 +172,11 @@ fn spawn_daemon(state: &State, args: &[String]) -> Result<()> { } else { "single-backend" }; - println!( + println_safe!( "proxy daemon started (PID {pid}, {mode}). Logs: {}", state.log_file.display() - ); - println!(" argv: {}", args.join(" ")); + )?; + println_safe!(" argv: {}", args.join(" "))?; Ok(()) } @@ -183,18 +184,18 @@ fn stop(state: &State) -> Result<()> { let pid = match read_pid(&state.pid_file)? { Some(p) => p, None => { - println!("proxy daemon is not running (no PID file)"); + println_safe!("proxy daemon is not running (no PID file)")?; return Ok(()); } }; if !process_exists(pid) { fs::remove_file(&state.pid_file).ok(); - println!("proxy daemon is not running (stale PID {pid})"); + println_safe!("proxy daemon is not running (stale PID {pid})")?; return Ok(()); } kill_process(pid)?; fs::remove_file(&state.pid_file).context("remove ~/.gfs/proxy.pid")?; - println!("proxy daemon stopped (PID {pid})"); + println_safe!("proxy daemon stopped (PID {pid})")?; Ok(()) } @@ -215,23 +216,23 @@ fn status(state: &State) -> Result<()> { match running_pid { Some(pid) => { - println!("Daemon: running (PID {pid})"); - println!("Metrics + /clones: http://{}", metrics_addr); + println_safe!("Daemon: running (PID {pid})")?; + println_safe!("Metrics + /clones: http://{}", metrics_addr)?; if let Some(args) = &saved_args { - println!("argv: {}", args.join(" ")); + println_safe!("argv: {}", args.join(" "))?; } - print_clones(&metrics_addr); + print_clones(&metrics_addr)?; } None if state.pid_file.exists() => { - println!( + println_safe!( "Daemon: stopped (stale PID file at {})", state.pid_file.display() - ); - println!(" → `gfs proxy stop` to clean it up, or `gfs proxy start` to restart"); + )?; + println_safe!(" → `gfs proxy stop` to clean it up, or `gfs proxy start` to restart")?; } None => { - println!("Daemon: stopped"); - println!(" → `gfs proxy start` to launch (auto-discovery by default)"); + println_safe!("Daemon: stopped")?; + println_safe!(" → `gfs proxy start` to launch (auto-discovery by default)")?; } } Ok(()) @@ -245,7 +246,7 @@ fn find_arg_value(args: &[String], name: &str) -> Option { /// Pretty-print the live clone→listener map scraped from `GET /clones`. Best- /// effort: a fetch/parse failure prints a diagnostic and returns — `status` is /// still useful even if the HTTP endpoint hiccups. -fn print_clones(metrics_addr: &str) { +fn print_clones(metrics_addr: &str) -> Result<()> { let url = format!("http://{metrics_addr}/clones"); let out = match Command::new("curl") .args(["-fsS", "--max-time", "2", &url]) @@ -254,41 +255,42 @@ fn print_clones(metrics_addr: &str) { { Ok(o) if o.status.success() => o.stdout, Ok(o) => { - println!( + println_safe!( "\nFronted clones: ", String::from_utf8_lossy(&o.stderr).trim() - ); - return; + )?; + return Ok(()); } Err(e) => { - println!("\nFronted clones: "); - return; + println_safe!("\nFronted clones: ")?; + return Ok(()); } }; let body = match serde_json::from_slice::(&out) { Ok(v) => v, Err(e) => { - println!("\nFronted clones: "); - return; + println_safe!("\nFronted clones: ")?; + return Ok(()); } }; let arr = body.get("clones").and_then(|v| v.as_array()); match arr { Some(a) if a.is_empty() => { - println!("\nFronted clones: (none yet — discovery scans Docker periodically)") + println_safe!("\nFronted clones: (none yet — discovery scans Docker periodically)")? } Some(a) => { - println!("\nFronted clones ({}):", a.len()); + println_safe!("\nFronted clones ({}):", a.len())?; for c in a { let name = c.get("container").and_then(|v| v.as_str()).unwrap_or("?"); let backend = c.get("backend").and_then(|v| v.as_str()).unwrap_or("?"); let port = c.get("listen_port").and_then(|v| v.as_u64()).unwrap_or(0); let remote = c.get("remote").and_then(|v| v.as_str()).unwrap_or("?"); - println!(" - {name}: localhost:{port} → {backend} (remote: {remote})"); + println_safe!(" - {name}: localhost:{port} → {backend} (remote: {remote})")?; } } - None => println!("\nFronted clones: "), + None => println_safe!("\nFronted clones: ")?, } + Ok(()) } // --------------------------------------------------------------------------- @@ -297,7 +299,7 @@ fn print_clones(metrics_addr: &str) { fn logs(state: &State, follow: bool, tail: usize) -> Result<()> { if !state.log_file.exists() { - println!("no log file yet at {}", state.log_file.display()); + println_safe!("no log file yet at {}", state.log_file.display())?; return Ok(()); } let mut cmd = Command::new("tail"); diff --git a/crates/applications/cli/src/commands/cmd_schema.rs b/crates/applications/cli/src/commands/cmd_schema.rs index 0b6ece34..302203ce 100644 --- a/crates/applications/cli/src/commands/cmd_schema.rs +++ b/crates/applications/cli/src/commands/cmd_schema.rs @@ -1,5 +1,6 @@ //! `gfs schema` — database schema operations (extract, show, diff). +use crate::println_safe; use std::path::PathBuf; use std::sync::Arc; @@ -69,13 +70,13 @@ pub async fn run_extract( if let Some(output_path) = output { std::fs::write(&output_path, &json) .with_context(|| format!("failed to write schema to {}", output_path.display()))?; - println!( + println_safe!( "{} Schema extracted to {}", green("✓"), cyan(output_path.display().to_string()) - ); + )?; } else { - println!("{}", json); + println_safe!("{}", json)?; } Ok(()) @@ -115,30 +116,30 @@ pub async fn run_show( // Output based on flags (do not color ddl_only or metadata_only - raw output) if ddl_only { - println!("{}", ddl); + println_safe!("{}", ddl)?; } else if metadata_only { let json = serde_json::to_string_pretty(&metadata) .context("failed to serialize schema metadata")?; - println!("{}", json); + println_safe!("{}", json)?; } else { // Show both metadata and DDL with colors - println!(" {} {}", dimmed("Schema Hash:"), cyan(schema_hash)); - println!( + println_safe!(" {} {}", dimmed("Schema Hash:"), cyan(schema_hash))?; + println_safe!( " {} {} {}", dimmed("Driver:"), metadata.driver, metadata.version - ); - println!(); - println!(" {}", header("Metadata (JSON)")); - println!(); + )?; + println_safe!()?; + println_safe!(" {}", header("Metadata (JSON)"))?; + println_safe!()?; let json = serde_json::to_string_pretty(&metadata) .context("failed to serialize schema metadata")?; - println!("{}", json); - println!(); - println!(" {}", header("DDL (SQL)")); - println!(); - println!("{}", ddl); + println_safe!("{}", json)?; + println_safe!()?; + println_safe!(" {}", header("DDL (SQL)"))?; + println_safe!()?; + println_safe!("{}", ddl)?; } Ok(()) @@ -197,7 +198,7 @@ pub async fn run_diff( AgenticFormatter::format(&diff) }; - println!("{}", output); + println_safe!("{}", output)?; Ok(diff.exit_code()) } diff --git a/crates/applications/cli/src/commands/cmd_source.rs b/crates/applications/cli/src/commands/cmd_source.rs index d02447ca..ab0b79d6 100644 --- a/crates/applications/cli/src/commands/cmd_source.rs +++ b/crates/applications/cli/src/commands/cmd_source.rs @@ -10,6 +10,7 @@ //! //! Note there is deliberately no `push`: the source is never written to. +use crate::println_safe; use std::path::{Path, PathBuf}; use anyhow::{Context, Result, bail}; @@ -180,38 +181,38 @@ pub async fn fetch(path: Option, check: bool, json_output: bool) -> Res .map(|m| m.diverged_stale) .unwrap_or(0); if json_output { - println!( + println_safe!( "{}", serde_json::to_string_pretty(&json!({ "detached": true, "state": "frozen", "torn": false, "frozen_at": at, "frozen_lsn": lsn, "kept": kept, "diverged_stale": stale }))? - ); + )?; return Ok(()); } - println!(); - println!( + println_safe!()?; + println_safe!( " {} detached snapshot {} — no source to sync", green("\u{2713}"), dimmed(format!("(frozen {at}, source LSN {lsn})")) - ); + )?; if stale > 0 { - println!( + println_safe!( " {}", dimmed(format!( "{stale} kept table(s) hold source rows that predate the freeze (they are your branch)" )) - ); + )?; } else if kept > 0 { - println!( + println_safe!( " {}", dimmed(format!( "{kept} table(s) kept your local writes; their source-derived rows date from earlier moments" )) - ); + )?; } - println!(); + println_safe!()?; return Ok(()); } @@ -268,53 +269,53 @@ pub async fn fetch(path: Option, check: bool, json_output: bool) -> Res out["wal_span"] = json!([m.span_min, m.span_max]); out["torn"] = json!(m.torn); } - println!("{}", serde_json::to_string_pretty(&out)?); + println_safe!("{}", serde_json::to_string_pretty(&out)?)?; return Ok(()); } - println!(); + println_safe!()?; if all.is_empty() { - println!(" {} no tables tracked yet", dimmed("·")); + println_safe!(" {} no tables tracked yet", dimmed("·"))?; return Ok(()); } if changed.is_empty() && notes.is_empty() { - println!( + println_safe!( " {} source unchanged {}", green("✓"), dimmed(format!("({} tables tracked)", all.len())) - ); + )?; } else { - println!( + println_safe!( " {} {} of {} tables changed on the source", yellow("!"), bold(changed.len().to_string()), all.len() - ); - println!(); + )?; + println_safe!()?; for r in &changed { if r[3] == "true" { - println!(" {} {}", red("conflict"), cyan(&r[0])); - println!( + println_safe!(" {} {}", red("conflict"), cyan(&r[0]))?; + println_safe!( " {}", dimmed("you have local writes AND the source changed") - ); + )?; } else { - println!(" {} {}", yellow("changed "), cyan(&r[0])); + println_safe!(" {} {}", yellow("changed "), cyan(&r[0]))?; if !r[2].is_empty() { - println!(" {}", dimmed(&r[2])); + println_safe!(" {}", dimmed(&r[2]))?; } } } - println!(); - println!( + println_safe!()?; + println_safe!( " {}", dimmed("reads of these tables go to the source, so they are") - ); - println!( + )?; + println_safe!( " {}", dimmed("correct but slower. run `gfs pull` to make them local again.") - ); + )?; } for n in ¬es { @@ -323,9 +324,9 @@ pub async fn fetch(path: Option, check: bool, json_output: bool) -> Res "unattributed" => "unattributed", _ => "unaccounted", }; - println!(" {} {}", yellow(label), cyan(&n[1])); + println_safe!(" {} {}", yellow(label), cyan(&n[1]))?; if !n[2].is_empty() { - println!(" {}", dimmed(&n[2])); + println_safe!(" {}", dimmed(&n[2]))?; } } @@ -334,64 +335,64 @@ pub async fn fetch(path: Option, check: bool, json_output: bool) -> Res // baseline), and stale-but-coherent while it is loud. Say which. if let Some(m) = &moments { if m.torn { - println!(); - println!( + println_safe!()?; + println_safe!( " {} this clone spans {} source moments {}", yellow("!"), bold(m.moment_count.to_string()), dimmed(format!("(WAL {} \u{2192} {})", m.span_min, m.span_max)) - ); - println!( + )?; + println_safe!( " {}", dimmed("tables were copied while the source kept moving, so a JOIN across") - ); - println!( + )?; + println_safe!( " {}", dimmed("them can return combinations that never existed on the source at") - ); - println!( + )?; + println_safe!( " {}", dimmed("any instant. `gfs freeze` re-copies everything from one instant") - ); - println!( + )?; + println_safe!( " {}", dimmed("(tables you have written to keep your changes); `gfs pull` re-syncs") - ); - println!( + )?; + println_safe!( " {}", dimmed("changed tables but stays lazy, so it narrows the span only if the") - ); - println!( + )?; + println_safe!( " {}", dimmed("next reads happen while the source is quiet.") - ); + )?; } else if m.copied >= 2 && m.unmarked == 0 { - println!(); - println!( + println_safe!()?; + println_safe!( " {} all copied data is from one source moment", green("\u{2713}") - ); + )?; } if m.unmarked > 0 { - println!( + println_safe!( " {}", dimmed(format!( "({} copied table(s) carry no copy watermark; their moment is unknown)", m.unmarked )) - ); + )?; } } if !check && !last_checked.is_empty() { - println!(); - println!( + println_safe!()?; + println_safe!( " {} {}", dimmed(format!("as of {last_checked}")), dimmed("(use --check to probe the source now)") - ); + )?; } - println!(); + println_safe!()?; Ok(()) } @@ -415,25 +416,25 @@ pub async fn pull( // machinery the frozen guards keep off anyway). if let Some((true, at, _lsn)) = frozen_info(&repo).await { if json_output { - println!( + println_safe!( "{}", serde_json::to_string_pretty(&json!({ "detached": true, "frozen_at": at }))? - ); + )?; return Ok(()); } - println!(); - println!( + println_safe!()?; + println_safe!( " {} this clone is detached {} — pull does nothing", green("\u{2713}"), dimmed(format!("(frozen {at})")) - ); + )?; if auto.is_some() || auto_schema.is_some() { - println!( + println_safe!( " {}", dimmed("auto-pull settings have no effect on a frozen clone") - ); + )?; } - println!(); + println_safe!()?; return Ok(()); } @@ -456,32 +457,32 @@ pub async fn pull( ) .await?; if json_output { - println!( + println_safe!( "{}", serde_json::to_string_pretty(&json!({ "autoschema": on }))? - ); + )?; } else { - println!(); - println!( + println_safe!()?; + println_safe!( " {} auto schema repair {}", green("✓"), bold(if on { "on" } else { "off" }) - ); - println!( + )?; + println_safe!( " {}", dimmed(if on { "a source shape change is re-imported automatically (additive changes only)" } else { "a source shape change fails with a clear message until you run `gfs pull`" }) - ); - println!( + )?; + println_safe!( " {}", dimmed( "a column dropped on the source is never applied automatically: that could destroy local data" ) - ); - println!(); + )?; + println_safe!()?; } return Ok(()); } @@ -495,26 +496,26 @@ pub async fn pull( ) .await?; if json_output { - println!( + println_safe!( "{}", serde_json::to_string_pretty(&json!({ "autopull": on }))? - ); + )?; } else { - println!(); - println!( + println_safe!()?; + println_safe!( " {} auto-pull {}", green("✓"), bold(if on { "on" } else { "off" }) - ); - println!( + )?; + println_safe!( " {}", dimmed(if on { "a changed table costs one query from the source, then it is re-copied" } else { "changed tables are read from the source until you run `gfs pull`" }) - ); - println!(); + )?; + println_safe!()?; } return Ok(()); } @@ -531,7 +532,7 @@ pub async fn pull( let conflicts: Vec<&Vec> = actions.iter().filter(|r| r[0] == "conflict").collect(); if json_output { - println!( + println_safe!( "{}", serde_json::to_string_pretty(&json!({ "reset": reset, @@ -540,66 +541,66 @@ pub async fn pull( "action": r[0], "table": r[1], "detail": r[2] })).collect::>(), }))? - ); + )?; return Ok(()); } - println!(); + println_safe!()?; for r in actions .iter() .filter(|r| r[0] == "schema" || r[0] == "sequence" || r[0] == "enum") { - println!(" {} {} {}", green("✓"), cyan(&r[1]), dimmed(&r[2])); + println_safe!(" {} {} {}", green("✓"), cyan(&r[1]), dimmed(&r[2]))?; } if actions.is_empty() { - println!(" {} already up to date", green("✓")); - println!(); + println_safe!(" {} already up to date", green("✓"))?; + println_safe!()?; return Ok(()); } if seqs > 0 && reset == 0 { - println!( + println_safe!( " {}", dimmed("local inserts would otherwise have collided with rows fetched from the source") - ); + )?; } if reset > 0 { - println!( + println_safe!( " {} {} table(s) back on the lazy path", green("✓"), bold(reset.to_string()) - ); + )?; for r in actions.iter().filter(|r| r[0] == "reset") { - println!(" {} {}", dimmed("reset"), cyan(&r[1])); + println_safe!(" {} {}", dimmed("reset"), cyan(&r[1]))?; } - println!( + println_safe!( " {}", dimmed("the next read of each fetches from the source") - ); + )?; } if !conflicts.is_empty() { - println!(); - println!( + println_safe!()?; + println_safe!( " {} {} table(s) NOT touched", red("!"), bold(conflicts.len().to_string()) - ); + )?; // Print each conflict's OWN reason: a local-write conflict and a schema // conflict are different situations, and a single hardcoded line claimed // the user had local writes even when the clash was purely structural. for r in &conflicts { - println!(" {} {}", red("conflict"), cyan(&r[1])); - println!(" {}", dimmed(r[2].trim_start_matches("conflict: "))); + println_safe!(" {} {}", red("conflict"), cyan(&r[1]))?; + println_safe!(" {}", dimmed(r[2].trim_start_matches("conflict: ")))?; } if conflicts.iter().any(|r| r[2].contains("local writes")) { - println!( + println_safe!( " {}", dimmed("use `gfs pull --force` to discard yours and take the source's") - ); + )?; } } - println!(); + println_safe!()?; Ok(()) } @@ -636,17 +637,17 @@ pub async fn remote(path: Option, json_output: bool) -> Result<()> { // cannot trip; an empty result is how a non-clone repository answers let Some(r) = rows(&raw).into_iter().next().filter(|r| r.len() >= 4) else { if json_output { - println!( + println_safe!( "{}", serde_json::to_string_pretty(&json!({ "url": null, "push": false }))? - ); + )?; } else { - println!(); - println!( + println_safe!()?; + println_safe!( " {} this repository has no source (not a clone)", dimmed("\u{b7}") - ); - println!(); + )?; + println_safe!()?; } return Ok(()); }; @@ -660,28 +661,28 @@ pub async fn remote(path: Option, json_output: bool) -> Result<()> { let url = format!("postgres://{auth}{host}:{port}/{dbname}"); if json_output { - println!( + println_safe!( "{}", serde_json::to_string_pretty(&json!({ "url": url, "host": host, "port": port, "dbname": dbname, "user": user, "push": false, }))? - ); + )?; return Ok(()); } - println!(); - println!( + println_safe!()?; + println_safe!( " {} {} {}", bold("origin"), cyan(&url), dimmed("(fetch only)") - ); - println!( + )?; + println_safe!( " {}", dimmed("GFS never writes to the source; there is no `gfs push`.") - ); - println!(); + )?; + println_safe!()?; Ok(()) } @@ -714,11 +715,11 @@ pub(crate) async fn materialize_clone(repo_path: &Path, quiet: bool) -> Result 0 { - println!( + println_safe!( " {} frozen clone: {} table(s) kept with your local writes export as they stand", dimmed("\u{b7}"), partial - ); + )?; } } return Ok(0); @@ -740,11 +741,11 @@ pub(crate) async fn materialize_clone(repo_path: &Path, quiet: bool) -> Result, output: String) -> Result { } match output.as_str() { - "json" => print_json(&status), - _ => print_table(&status, &repo_path, moments.as_ref()), + "json" => print_json(&status)?, + _ => print_table(&status, &repo_path, moments.as_ref())?, } // Exit code: 0 if no compute or compute is running, 1 otherwise. @@ -122,70 +123,70 @@ async fn source_summary(repo_path: &Path) -> Option { Some(st) } -fn print_source(s: &SourceStatus, moments: Option<&cmd_source::CloneMoments>) { - println!(); - println!("{}", box_top(&bold("Source"), BOX_W)); +fn print_source(s: &SourceStatus, moments: Option<&cmd_source::CloneMoments>) -> Result<()> { + println_safe!()?; + println_safe!("{}", box_top(&bold("Source"), BOX_W))?; // #132: a frozen clone is a sealed snapshot; behind/diverged/checked no // longer mean anything (nothing is compared any more), so say what it IS. if s.frozen == Some(true) { let state = "frozen snapshot"; let row = fmt_box_row_colored("State", &cyan(state), state, LABEL_W, BOX_W); - println!("{}", box_row(&row, BOX_W)); + println_safe!("{}", box_row(&row, BOX_W))?; if let Some(at) = &s.frozen_at { let row = fmt_box_row("Frozen at", at, LABEL_W, BOX_W); - println!("{}", box_row(&row, BOX_W)); + println_safe!("{}", box_row(&row, BOX_W))?; } let row = fmt_box_row("Tracked tables", &s.tracked.to_string(), LABEL_W, BOX_W); - println!("{}", box_row(&row, BOX_W)); + println_safe!("{}", box_row(&row, BOX_W))?; if s.diverged > 0 { let d = s.diverged.to_string(); let row = fmt_box_row_colored("Kept", &yellow(&d).to_string(), &d, LABEL_W, BOX_W); - println!("{}", box_row(&row, BOX_W)); + println_safe!("{}", box_row(&row, BOX_W))?; } - println!("{}", box_bottom(BOX_W)); - println!(); - println!( + println_safe!("{}", box_bottom(BOX_W))?; + println_safe!()?; + println_safe!( " {}", dimmed("a point-in-time snapshot; the source is never consulted") - ); + )?; if s.diverged > 0 { // #131: when the watermarks can date them, say the sharper thing -- // kept tables were not re-copied, so their source rows are older // than the freeze instant. Never reported as "torn": keeping your // writes is what makes this clone a branch. if moments.map(|m| m.diverged_stale).unwrap_or(0) > 0 { - println!( + println_safe!( " {}", dimmed( "kept tables preserve your local writes; their source rows predate the freeze" ) - ); + )?; } else { - println!( + println_safe!( " {}", dimmed("kept tables preserve your local writes (they are your branch)") - ); + )?; } } - return; + return Ok(()); } // Zeros here would read as "up to date"; nothing has been compared yet. if s.tracked == 0 { let msg = format!("{:) { behind.clone() }; let row = fmt_box_row_colored("Behind", &behind_colored, &behind, LABEL_W, BOX_W); - println!("{}", box_row(&row, BOX_W)); + println_safe!("{}", box_row(&row, BOX_W))?; if s.diverged > 0 { let d = s.diverged.to_string(); let row = fmt_box_row_colored("Diverged", &red(&d).to_string(), &d, LABEL_W, BOX_W); - println!("{}", box_row(&row, BOX_W)); + println_safe!("{}", box_row(&row, BOX_W))?; } // #131: does this clone mix source moments? A copied-at-different-moments @@ -212,36 +213,37 @@ fn print_source(s: &SourceStatus, moments: Option<&cmd_source::CloneMoments>) { if m.torn { let v = format!("spans \u{2265}{} (torn)", m.moment_count); let row = fmt_box_row_colored("Moments", &yellow(&v).to_string(), &v, LABEL_W, BOX_W); - println!("{}", box_row(&row, BOX_W)); + println_safe!("{}", box_row(&row, BOX_W))?; } else if m.copied >= 2 && m.unmarked == 0 { let row = fmt_box_row("Moments", "single", LABEL_W, BOX_W); - println!("{}", box_row(&row, BOX_W)); + println_safe!("{}", box_row(&row, BOX_W))?; } else if m.copied > 0 && m.unmarked > 0 { let row = fmt_box_row("Moments", "unknown", LABEL_W, BOX_W); - println!("{}", box_row(&row, BOX_W)); + println_safe!("{}", box_row(&row, BOX_W))?; } } if !s.last_checked.is_empty() { let row = fmt_box_row("Checked", &s.last_checked, LABEL_W, BOX_W); - println!("{}", box_row(&row, BOX_W)); + println_safe!("{}", box_row(&row, BOX_W))?; } - println!("{}", box_bottom(BOX_W)); + println_safe!("{}", box_bottom(BOX_W))?; if s.behind > 0 { - println!(); - println!( + println_safe!()?; + println_safe!( " {}", dimmed("`gfs fetch` for detail, `gfs pull` to make these tables local again") - ); + )?; } if torn { - println!(); - println!( + println_safe!()?; + println_safe!( " {}", dimmed("`gfs fetch` shows the span; `gfs freeze` makes this clone one moment again") - ); + )?; } + Ok(()) } // --------------------------------------------------------------------------- @@ -251,9 +253,13 @@ fn print_source(s: &SourceStatus, moments: Option<&cmd_source::CloneMoments>) { const LABEL_W: usize = 20; const BOX_W: usize = 40; -fn print_table(s: &StatusResponse, repo_path: &Path, moments: Option<&cmd_source::CloneMoments>) { +fn print_table( + s: &StatusResponse, + repo_path: &Path, + moments: Option<&cmd_source::CloneMoments>, +) -> Result<()> { // Repository section - println!("{}", box_top(&bold("Repository"), BOX_W)); + println_safe!("{}", box_top(&bold("Repository"), BOX_W))?; let branch_row = fmt_box_row_colored( "Branch", @@ -262,20 +268,20 @@ fn print_table(s: &StatusResponse, repo_path: &Path, moments: Option<&cmd_source LABEL_W, BOX_W, ); - println!("{}", box_row(&branch_row, BOX_W)); + println_safe!("{}", box_row(&branch_row, BOX_W))?; // The same field the MCP `status` tool reports, from the same place. When // only MCP had it the two surfaces answered the same question differently. if let Some(ref head) = s.head_commit { let short = &head[..7.min(head.len())]; let row = fmt_box_row_colored("HEAD", &dimmed(short), short, LABEL_W, BOX_W); - println!("{}", box_row(&row, BOX_W)); + println_safe!("{}", box_row(&row, BOX_W))?; } if let Some(ref active) = s.active_workspace_data_dir { let rel = relativize_to_repo(repo_path, active); let row = fmt_box_row("Active workspace", &rel, LABEL_W, BOX_W); - println!("{}", box_row(&row, BOX_W)); + println_safe!("{}", box_row(&row, BOX_W))?; } // `gfs status --help` promises a connection string. A container-backed // provider prints one in the Compute section below; an embedded one has no @@ -284,11 +290,11 @@ fn print_table(s: &StatusResponse, repo_path: &Path, moments: Option<&cmd_source && let Some(ref conn) = s.connection_string { let row = fmt_box_row("Connection", conn, LABEL_W, BOX_W); - println!("{}", box_row(&row, BOX_W)); + println_safe!("{}", box_row(&row, BOX_W))?; } - println!("{}", box_bottom(BOX_W)); + println_safe!("{}", box_bottom(BOX_W))?; - println!(); + println_safe!()?; if let Some(ref c) = s.compute { let status_dot = status_indicator_colored(&c.container_status); @@ -298,17 +304,17 @@ fn print_table(s: &StatusResponse, repo_path: &Path, moments: Option<&cmd_source c.container_status ); - println!("{}", box_top(&bold("Compute"), BOX_W)); + println_safe!("{}", box_top(&bold("Compute"), BOX_W))?; let row = fmt_box_row("Provider", &c.provider, LABEL_W, BOX_W); - println!("{}", box_row(&row, BOX_W)); + println_safe!("{}", box_row(&row, BOX_W))?; let row = fmt_box_row("Version", &c.version, LABEL_W, BOX_W); - println!("{}", box_row(&row, BOX_W)); + println_safe!("{}", box_row(&row, BOX_W))?; let status_colored = format!("{} {}", status_dot, c.container_status); let row = fmt_box_row_colored("Status", &status_colored, &status_raw, LABEL_W, BOX_W); - println!("{}", box_row(&row, BOX_W)); + println_safe!("{}", box_row(&row, BOX_W))?; let truncated = truncate_id(&c.container_id); let row = fmt_box_row_colored( @@ -318,27 +324,27 @@ fn print_table(s: &StatusResponse, repo_path: &Path, moments: Option<&cmd_source LABEL_W, BOX_W, ); - println!("{}", box_row(&row, BOX_W)); + println_safe!("{}", box_row(&row, BOX_W))?; if let Some(ref bind) = c.data_bind_host_path { let rel = relativize_to_repo(repo_path, bind); let row = fmt_box_row("Container data dir", &rel, LABEL_W, BOX_W); - println!("{}", box_row(&row, BOX_W)); + println_safe!("{}", box_row(&row, BOX_W))?; } if !c.connection_string.is_empty() { let row = fmt_box_row("Connection", &c.connection_string, LABEL_W, BOX_W); - println!("{}", box_row(&row, BOX_W)); + println_safe!("{}", box_row(&row, BOX_W))?; } - println!("{}", box_bottom(BOX_W)); + println_safe!("{}", box_bottom(BOX_W))?; } else { - println!("{}", box_top(&bold("Compute"), BOX_W)); + println_safe!("{}", box_top(&bold("Compute"), BOX_W))?; let msg = format!("{: String { } } -fn print_json(s: &StatusResponse) { +fn print_json(s: &StatusResponse) -> Result<()> { let out = serde_json::to_string_pretty(s).expect("status serialization"); - println!("{}", out); + println_safe!("{}", out)?; + Ok(()) } diff --git a/crates/applications/cli/src/commands/cmd_user.rs b/crates/applications/cli/src/commands/cmd_user.rs index 0dd8cf0a..27e04b22 100644 --- a/crates/applications/cli/src/commands/cmd_user.rs +++ b/crates/applications/cli/src/commands/cmd_user.rs @@ -1,5 +1,6 @@ //! `gfs user` — manage database users/roles (create, list, drop, set-password). +use crate::println_safe; use std::path::{Path, PathBuf}; use std::sync::Arc; @@ -44,21 +45,27 @@ fn generate_password() -> String { uuid::Uuid::new_v4().simple().to_string() } -fn print_credential(username: &str, password: &str, generated: bool, json_output: bool) { +fn print_credential( + username: &str, + password: &str, + generated: bool, + json_output: bool, +) -> Result<()> { if json_output { // Machine output keeps a stable shape; the caller opts into it and owns // redaction of a password they themselves supplied. - println!( + println_safe!( "{}", serde_json::json!({ "username": username, "password": password }) - ); + )?; } else if generated { // Only the server-generated secret is the "shown once" copy worth echoing. - println!("user '{username}' — password (shown once): {password}"); + println_safe!("user '{username}' — password (shown once): {password}")?; } else { // A caller-supplied password is not re-echoed to the terminal/logs. - println!("user '{username}' — password set"); + println_safe!("user '{username}' — password set")?; } + Ok(()) } pub async fn run_create( @@ -94,7 +101,7 @@ pub async fn run_create( ) .await .map_err(|e| anyhow::anyhow!("{e}"))?; - print_credential(&username, &password, generated, json_output); + print_credential(&username, &password, generated, json_output)?; Ok(()) } @@ -106,15 +113,17 @@ pub async fn run_list(path: Option, json_output: bool) -> Result<()> { .await .map_err(|e| anyhow::anyhow!("{e}"))?; if json_output { - println!("{}", serde_json::to_string(&roles)?); + println_safe!("{}", serde_json::to_string(&roles)?)?; } else if roles.is_empty() { - println!("no database users"); + println_safe!("no database users")?; } else { for role in &roles { - println!( + println_safe!( "{:<32} login={} superuser={}", - role.username, role.can_login, role.is_superuser - ); + role.username, + role.can_login, + role.is_superuser + )?; } } Ok(()) @@ -128,12 +137,12 @@ pub async fn run_drop(path: Option, username: String, json_output: bool .await .map_err(|e| anyhow::anyhow!("{e}"))?; if json_output { - println!( + println_safe!( "{}", serde_json::json!({ "username": username, "dropped": true }) - ); + )?; } else { - println!("dropped user '{username}'"); + println_safe!("dropped user '{username}'")?; } Ok(()) } @@ -152,7 +161,7 @@ pub async fn run_set_password( .set_password(&repo_path, &username, &password) .await .map_err(|e| anyhow::anyhow!("{e}"))?; - print_credential(&username, &password, generated, json_output); + print_credential(&username, &password, generated, json_output)?; Ok(()) } @@ -179,12 +188,12 @@ pub async fn run_apply_preset( .await .map_err(|e| anyhow::anyhow!("{e}"))?; if json_output { - println!( + println_safe!( "{}", serde_json::json!({ "username": username, "preset_applied": true }) - ); + )?; } else { - println!("user '{username}' — preset applied"); + println_safe!("user '{username}' — preset applied")?; } Ok(()) } @@ -291,12 +300,12 @@ pub async fn run_grant( .await .map_err(|e| anyhow::anyhow!("{e}"))?; if json_output { - println!( + println_safe!( "{}", serde_json::json!({ "username": username, "granted": true }) - ); + )?; } else { - println!("granted privileges to '{username}'"); + println_safe!("granted privileges to '{username}'")?; } Ok(()) } @@ -326,12 +335,12 @@ pub async fn run_revoke( .await .map_err(|e| anyhow::anyhow!("{e}"))?; if json_output { - println!( + println_safe!( "{}", serde_json::json!({ "username": username, "revoked": true }) - ); + )?; } else { - println!("revoked privileges from '{username}'"); + println_safe!("revoked privileges from '{username}'")?; } Ok(()) } @@ -348,15 +357,18 @@ pub async fn run_list_privs( .await .map_err(|e| anyhow::anyhow!("{e}"))?; if json_output { - println!("{}", serde_json::to_string(&privileges)?); + println_safe!("{}", serde_json::to_string(&privileges)?)?; } else if privileges.is_empty() { - println!("no privileges for '{username}'"); + println_safe!("no privileges for '{username}'")?; } else { for p in &privileges { - println!( + println_safe!( "{:<10} {:<40} {:<12} grantable={}", - p.object_type, p.object_name, p.privilege, p.grantable - ); + p.object_type, + p.object_name, + p.privilege, + p.grantable + )?; } } Ok(()) diff --git a/crates/applications/cli/src/commands/cmd_version.rs b/crates/applications/cli/src/commands/cmd_version.rs index 5abfa700..e80071a6 100644 --- a/crates/applications/cli/src/commands/cmd_version.rs +++ b/crates/applications/cli/src/commands/cmd_version.rs @@ -1,9 +1,11 @@ //! `gfs version` — print the CLI version with retro arcade-style ASCII art. use crate::output::{bold, dimmed, gold}; +use crate::println_safe; +use anyhow::Result; /// Print the current gfs CLI version inside a retro branded box. -pub fn run() { +pub fn run() -> Result<()> { let version = env!("CARGO_PKG_VERSION"); let target = format!("{}-{}", std::env::consts::OS, std::env::consts::ARCH); @@ -39,59 +41,60 @@ pub fn run() { .max(art_w); // Top border - println!(" {}{}{}", tl, h.repeat(w + 2), tr); + println_safe!(" {}{}{}", tl, h.repeat(w + 2), tr)?; // Empty line - println!(" {} {} {}", v, " ".repeat(w), v); + println_safe!(" {} {} {}", v, " ".repeat(w), v)?; // ASCII art lines (gold-colored block letters) for line in &art { let plain_len = indent.len() + line.chars().count(); let remaining = w.saturating_sub(plain_len); - println!( + println_safe!( " {} {}{}{} {}", v, indent, gold(line), " ".repeat(remaining), v - ); + )?; } // Empty line - println!(" {} {} {}", v, " ".repeat(w), v); + println_safe!(" {} {} {}", v, " ".repeat(w), v)?; // Tagline (bold) { let plain_len = indent.len() + tagline.chars().count(); let remaining = w.saturating_sub(plain_len); - println!( + println_safe!( " {} {}{}{} {}", v, indent, bold(tagline), " ".repeat(remaining), v - ); + )?; } // Version + platform (dimmed) { let plain_len = indent.len() + meta.chars().count(); let remaining = w.saturating_sub(plain_len); - println!( + println_safe!( " {} {}{}{} {}", v, indent, dimmed(&meta), " ".repeat(remaining), v - ); + )?; } // Empty line - println!(" {} {} {}", v, " ".repeat(w), v); + println_safe!(" {} {} {}", v, " ".repeat(w), v)?; // Bottom border - println!(" {}{}{}", bl, h.repeat(w + 2), br); + println_safe!(" {}{}{}", bl, h.repeat(w + 2), br)?; + Ok(()) } diff --git a/crates/applications/cli/src/lib.rs b/crates/applications/cli/src/lib.rs index b32305ea..b4963bda 100644 --- a/crates/applications/cli/src/lib.rs +++ b/crates/applications/cli/src/lib.rs @@ -1223,7 +1223,7 @@ where Ok(0) } TopLevel::Version => { - commands::cmd_version::run(); + commands::cmd_version::run()?; Ok(0) } } @@ -1314,12 +1314,12 @@ async fn dispatch_storage( .await .map_err(anyhow::Error::from)?; if json_output { - println!( + println_safe!( "{}", serde_json::to_string_pretty(&json!({"status":"mounted"}))? - ); + )?; } else { - println!("mounted"); + println_safe!("mounted")?; } } StorageAction::Unmount { id } => { @@ -1328,12 +1328,12 @@ async fn dispatch_storage( .await .map_err(anyhow::Error::from)?; if json_output { - println!( + println_safe!( "{}", serde_json::to_string_pretty(&json!({"status":"unmounted"}))? - ); + )?; } else { - println!("unmounted"); + println_safe!("unmounted")?; } } StorageAction::Snapshot { id, label } => { @@ -1342,7 +1342,7 @@ async fn dispatch_storage( .await .map_err(anyhow::Error::from)?; if json_output { - println!( + println_safe!( "{}", serde_json::to_string_pretty(&json!({ "snapshot": { @@ -1352,13 +1352,13 @@ async fn dispatch_storage( "label": snap.label, } }))? - ); + )?; } else { - println!("snapshot id : {}", snap.id); - println!("volume : {}", snap.volume_id); - println!("created_at : {}", snap.created_at); + println_safe!("snapshot id : {}", snap.id)?; + println_safe!("volume : {}", snap.volume_id)?; + println_safe!("created_at : {}", snap.created_at)?; if let Some(lbl) = &snap.label { - println!("label : {lbl}"); + println_safe!("label : {lbl}")?; } } } @@ -1389,7 +1389,7 @@ async fn dispatch_storage( .await .map_err(anyhow::Error::from)?; if json_output { - println!( + println_safe!( "{}", serde_json::to_string_pretty(&json!({ "quota": { @@ -1399,12 +1399,12 @@ async fn dispatch_storage( "free_bytes": quota.free_bytes, } }))? - ); + )?; } else { - println!("volume : {}", quota.volume_id); - println!("limit_bytes : {}", quota.limit_bytes); - println!("used_bytes : {}", quota.used_bytes); - println!("free_bytes : {}", quota.free_bytes); + println_safe!("volume : {}", quota.volume_id)?; + println_safe!("limit_bytes : {}", quota.limit_bytes)?; + println_safe!("used_bytes : {}", quota.used_bytes)?; + println_safe!("free_bytes : {}", quota.free_bytes)?; } } } @@ -1424,7 +1424,7 @@ fn print_volume_status( json_output: bool, ) -> Result<()> { if json_output { - println!( + println_safe!( "{}", serde_json::to_string_pretty(&json!({ "volume": { @@ -1435,20 +1435,20 @@ fn print_volume_status( "used_bytes": s.used_bytes, } }))? - ); + )?; Ok(()) } else { - println!("id : {}", s.id); - println!( + println_safe!("id : {}", s.id)?; + println_safe!( "mount_point : {}", s.mount_point .as_deref() .map(|p| p.display().to_string()) .unwrap_or_else(|| "-".to_owned()) - ); - println!("status : {:?}", s.status); - println!("size_bytes : {}", s.size_bytes); - println!("used_bytes : {}", s.used_bytes); + )?; + println_safe!("status : {:?}", s.status)?; + println_safe!("size_bytes : {}", s.size_bytes)?; + println_safe!("used_bytes : {}", s.used_bytes)?; Ok(()) } } diff --git a/crates/applications/cli/src/main.rs b/crates/applications/cli/src/main.rs index 1785f74a..efe670e0 100644 --- a/crates/applications/cli/src/main.rs +++ b/crates/applications/cli/src/main.rs @@ -90,7 +90,10 @@ async fn main() { Ok(exit_code) => std::process::exit(exit_code), Err(err) => { if wants_json { - println!( + // Discarded on purpose: the process exits with this error's own + // code just below, and a failed write has nowhere else to go. A + // bare println! here panicked into 101 on a closed reader. + let _ = gfs_cli::println_safe!( "{}", serde_json::to_string_pretty(&json!({ "error": { diff --git a/crates/applications/cli/src/output.rs b/crates/applications/cli/src/output.rs index c90a0421..60c1ecdb 100644 --- a/crates/applications/cli/src/output.rs +++ b/crates/applications/cli/src/output.rs @@ -103,6 +103,9 @@ pub fn stdout_is_closed() -> bool { /// the command. #[macro_export] macro_rules! println_safe { + () => { + $crate::println_safe!("") + }; ($($arg:tt)*) => {{ use std::io::Write; if $crate::output::stdout_is_closed() { diff --git a/crates/applications/cli/tests/json_output.rs b/crates/applications/cli/tests/json_output.rs index 7d6f6c0a..5a893ea9 100644 --- a/crates/applications/cli/tests/json_output.rs +++ b/crates/applications/cli/tests/json_output.rs @@ -860,3 +860,80 @@ fn an_argument_that_is_not_utf8_is_a_usage_error_not_a_crash() { let (code, _, stderr) = run_gfs_raw(tmp.path(), Some(b"gfs\xff"), &[b"--version"]); assert_eq!(code, 0, "argv[0] is not an argument: {stderr}"); } + +/// Exit code of `args` when stdout's reader is gone before gfs writes. +fn closed_reader_exit_code(cwd: &Path, args: &[&str]) -> Option { + use std::process::Stdio; + + let mut child = Command::new(gfs_bin()) + .current_dir(cwd) + .args(args) + .env("RUST_LOG", "off") + .env("NO_COLOR", "1") + .stdout(Stdio::piped()) + .stderr(Stdio::null()) + .spawn() + .expect("spawn gfs"); + drop(child.stdout.take()); + child.wait().expect("wait").code() +} + +/// Every command, not just fsck, keeps its verdict when the reader closes early. +/// +/// A bare `println!` panics on BrokenPipe, so `gfs status | head -1` exited 101 -- +/// the code of a crash -- and `gfs schema diff | head` turned "breaking changes" +/// (2) into that same 101. The sqlite provider needs no container runtime, so +/// this runs anywhere. +#[test] +fn a_closed_reader_keeps_the_verdict_of_every_command() { + let tmp = TempDir::new().unwrap(); + let repo = tmp.path(); + for args in [ + vec![ + "init", + ".", + "--database-provider", + "sqlite", + "--database-version", + "3", + ], + vec!["config", "user.name", "closed-reader"], + vec!["config", "user.email", "closed-reader@example.com"], + vec!["commit", "-m", "first"], + ] { + assert_eq!(run_gfs(repo, &args).0, 0, "setup: {args:?}"); + } + let workspace = std::fs::read_to_string(repo.join(".gfs/WORKSPACE")).unwrap(); + rusqlite::Connection::open(repo.join(workspace.trim()).join("db.sqlite")) + .unwrap() + .execute("CREATE TABLE added (id INTEGER PRIMARY KEY)", []) + .unwrap(); + assert_eq!( + run_gfs(repo, &["commit", "-m", "second"]).0, + 0, + "setup: commit" + ); + + for (args, verdict) in [ + (vec!["status"], 0), + (vec!["status", "--json"], 0), + (vec!["branch"], 0), + (vec!["branch", "--json"], 0), + (vec!["providers"], 0), + (vec!["version"], 0), + // A non-zero verdict, so a closed reader cannot pass by exiting 0. + (vec!["schema", "diff", "HEAD~1", "HEAD"], 2), + ] { + let (open, stdout, _) = run_gfs(repo, &args); + assert_eq!(open, verdict, "precondition: {args:?} exits {verdict}"); + assert!( + !stdout.is_empty(), + "precondition: {args:?} has output to lose" + ); + assert_eq!( + closed_reader_exit_code(repo, &args), + Some(verdict), + "a closed reader must not change the verdict of {args:?}" + ); + } +} From 391c85aa47d571b135fc423070631297cfc302e0 Mon Sep 17 00:00:00 2001 From: Mohamed Aziz Ktata Date: Fri, 9 Oct 2026 14:58:32 +0100 Subject: [PATCH 3/5] fix(cli): route status's copy-on-write notice through println_safe! Main added three bare println! calls for the full-copy snapshot warning; on a closed reader they panicked into 101 like the ones this branch already converted. --- crates/applications/cli/src/commands/cmd_status.rs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/crates/applications/cli/src/commands/cmd_status.rs b/crates/applications/cli/src/commands/cmd_status.rs index ace4b280..3af24aca 100644 --- a/crates/applications/cli/src/commands/cmd_status.rs +++ b/crates/applications/cli/src/commands/cmd_status.rs @@ -354,19 +354,19 @@ fn print_table( if let Some(ref storage) = s.storage && !storage.copy_on_write { - println!(); + println_safe!()?; if let Some(ref detail) = storage.detail { - println!( + println_safe!( " {} snapshots are full copies: {}", yellow("!"), dimmed(detail) - ); + )?; } else if let Some(ref reason) = storage.reason { - println!( + println_safe!( " {} snapshots are full copies: {}", yellow("!"), dimmed(reason) - ); + )?; } } From f20ba9f676f18cf9a8cd24fd5be2f6cf9779c202 Mon Sep 17 00:00:00 2001 From: Mohamed Aziz Ktata Date: Fri, 16 Oct 2026 18:31:55 +0100 Subject: [PATCH 4/5] fix(cli): --help and --version never panic on a closed reader clap's help and version output was printed with `e.print().expect(..)`, so a reader that had gone away turned them into a panic and exit 101: gfs --help | head -1 -> PIPESTATUS 101 0 (sleep 1; gfs --version) | true -> PIPESTATUS 101 0 with "panicked at crates/applications/cli/src/lib.rs:847 ... BrokenPipe" on stderr. They are printed before any command runs, so println_safe! never saw them. BrokenPipe is now treated the way println_safe! treats it: output stops, the exit status (0) stands. Any other write error is returned rather than panicking. --help and --version join the closed-reader verdict table. --- crates/applications/cli/src/lib.rs | 10 ++++++++-- crates/applications/cli/tests/json_output.rs | 3 +++ 2 files changed, 11 insertions(+), 2 deletions(-) diff --git a/crates/applications/cli/src/lib.rs b/crates/applications/cli/src/lib.rs index b4963bda..691fdf07 100644 --- a/crates/applications/cli/src/lib.rs +++ b/crates/applications/cli/src/lib.rs @@ -844,8 +844,14 @@ where let cli = match Cli::try_parse_from(args_os) { Ok(c) => c, Err(e) if e.kind() == ErrorKind::DisplayVersion || e.kind() == ErrorKind::DisplayHelp => { - e.print().expect("writing version/help to stdout/stderr"); - return Ok(0); + // A closed reader (`gfs --help | head -1`) is not a failure, as in + // `println_safe!`; any other write error still is. + match e.print() { + Err(err) if err.kind() != std::io::ErrorKind::BrokenPipe => { + return Err(err.into()); + } + _ => return Ok(0), + } } Err(e) => return Err(e.into()), }; diff --git a/crates/applications/cli/tests/json_output.rs b/crates/applications/cli/tests/json_output.rs index 5a893ea9..20c742d6 100644 --- a/crates/applications/cli/tests/json_output.rs +++ b/crates/applications/cli/tests/json_output.rs @@ -921,6 +921,9 @@ fn a_closed_reader_keeps_the_verdict_of_every_command() { (vec!["branch", "--json"], 0), (vec!["providers"], 0), (vec!["version"], 0), + // Printed by clap before any command runs, so not via println_safe!. + (vec!["--help"], 0), + (vec!["--version"], 0), // A non-zero verdict, so a closed reader cannot pass by exiting 0. (vec!["schema", "diff", "HEAD~1", "HEAD"], 2), ] { From 9de72986b06d6dd658b1b92c1ecd421da676189e Mon Sep 17 00:00:00 2001 From: Mohamed Aziz Ktata Date: Mon, 26 Oct 2026 15:18:36 +0100 Subject: [PATCH 5/5] fix(cli): route branch delete's detached-workspace notice through println_safe! A bare println! landed on main after this branch's sweep (1b26050, the reserved 'detached' branch name guard), so 'gfs branch -d detached' on a repository that already carries such a branch still panicked into exit 101 on a closed reader. delete_branch already returns Result, so the call takes a ?. --- crates/applications/cli/src/commands/cmd_branch.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/crates/applications/cli/src/commands/cmd_branch.rs b/crates/applications/cli/src/commands/cmd_branch.rs index d4cdb075..1407babe 100644 --- a/crates/applications/cli/src/commands/cmd_branch.rs +++ b/crates/applications/cli/src/commands/cmd_branch.rs @@ -278,12 +278,12 @@ fn delete_branch(repo_path: &std::path::Path, name: &str, json_output: bool) -> // a repository that already carries one still can, which is why this guard // exists as well as the refusal. if workspace.exists() && name == DETACHED_WORKSPACE_SEGMENT { - println!( + println_safe!( " its workspace path is {}, which is the directory holding every detached \ working copy — left in place rather than removed.\n Move anything you need \ out of it; `gfs destroy` reclaims it with the repository", workspace.display() - ); + )?; } else if workspace.exists() { #[cfg(unix)] let _ = std::process::Command::new("chmod")