From fa73407b357ec13548e6b3d2c72c624016927a5d Mon Sep 17 00:00:00 2001 From: Bryan Helmkamp Date: Tue, 14 Apr 2026 16:11:01 -0400 Subject: [PATCH] feat(cli): add fabro version command Add a server-targeted `fabro version` command for checking client and server build identity without reading local storage directly. This also removes version data from `/health`, moves doctor parity checks to diagnostics, and updates the API spec, docs, generated client, and coverage for the new contract. --- docs/api-reference/fabro-api.yaml | 5 - docs/reference/cli.mdx | 12 + lib/crates/fabro-cli/src/args.rs | 9 + lib/crates/fabro-cli/src/commands/doctor.rs | 81 ++++--- lib/crates/fabro-cli/src/commands/mod.rs | 1 + lib/crates/fabro-cli/src/commands/version.rs | 212 ++++++++++++++++++ lib/crates/fabro-cli/src/main.rs | 4 + lib/crates/fabro-cli/tests/it/cmd/doctor.rs | 4 + lib/crates/fabro-cli/tests/it/cmd/fabro.rs | 1 + lib/crates/fabro-cli/tests/it/cmd/mod.rs | 1 + lib/crates/fabro-cli/tests/it/cmd/version.rs | 112 +++++++++ lib/crates/fabro-server/src/server.rs | 1 - .../fabro-server/tests/it/api/routing.rs | 4 + .../src/models/health-response.ts | 4 - 14 files changed, 412 insertions(+), 39 deletions(-) create mode 100644 lib/crates/fabro-cli/src/commands/version.rs create mode 100644 lib/crates/fabro-cli/tests/it/cmd/version.rs diff --git a/docs/api-reference/fabro-api.yaml b/docs/api-reference/fabro-api.yaml index 7dcc17b18..ea2941494 100644 --- a/docs/api-reference/fabro-api.yaml +++ b/docs/api-reference/fabro-api.yaml @@ -4313,16 +4313,11 @@ components: type: object required: - status - - version properties: status: type: string description: Health status indicator. example: ok - version: - type: string - description: Server version string. - example: "0.176.2" SecretType: description: The way a secret is consumed by the sandbox. diff --git a/docs/reference/cli.mdx b/docs/reference/cli.mdx index 48e2c8855..c428c21c5 100644 --- a/docs/reference/cli.mdx +++ b/docs/reference/cli.mdx @@ -224,6 +224,18 @@ fabro rm my-workflow --force | `...` | Run IDs or workflow names to remove (required, repeatable) | | `-f, --force` | Force removal of active runs | +## `fabro version` + +Show client and server build identity side-by-side, including version, build metadata, and server uptime. + +```bash +fabro version +fabro version --server http://127.0.0.1:4110 +fabro version --json +``` + +Use this when you want to check CLI/server parity. Unlike `fabro --version`, this queries the server. Unlike `fabro system info`, this stays focused on version/build identity rather than operational runtime details. + ## `fabro system info` Show server runtime information including version, uptime, and run counts. diff --git a/lib/crates/fabro-cli/src/args.rs b/lib/crates/fabro-cli/src/args.rs index a8d98959a..10fd04146 100644 --- a/lib/crates/fabro-cli/src/args.rs +++ b/lib/crates/fabro-cli/src/args.rs @@ -636,6 +636,12 @@ pub(crate) struct SystemInfoArgs { pub(crate) connection: ServerConnectionArgs, } +#[derive(Args, Debug, Clone, Default)] +pub(crate) struct VersionArgs { + #[command(flatten)] + pub(crate) target: ServerTargetArgs, +} + #[derive(Args)] pub(crate) struct RunsPruneArgs { #[command(flatten)] @@ -980,6 +986,8 @@ pub(crate) enum Commands { Server(ServerNamespace), /// Check environment and integration health Doctor(DoctorArgs), + /// Show client and server version information + Version(VersionArgs), /// Set up the Fabro environment (LLMs, certs, GitHub) Install(InstallArgs), /// Uninstall Fabro from this machine @@ -1061,6 +1069,7 @@ impl Commands { ServerCommand::Serve(_) => "server __serve", }, Self::Doctor(_) => "doctor", + Self::Version(_) => "version", Self::Repo(ns) => match &ns.command { RepoCommand::Init(_) => "repo init", RepoCommand::Deinit => "repo deinit", diff --git a/lib/crates/fabro-cli/src/commands/doctor.rs b/lib/crates/fabro-cli/src/commands/doctor.rs index 9a3b7dcdf..99c2984c3 100644 --- a/lib/crates/fabro-cli/src/commands/doctor.rs +++ b/lib/crates/fabro-cli/src/commands/doctor.rs @@ -192,6 +192,18 @@ fn check_version_parity(server_version: &str) -> CheckResult { } } +fn skipped_version_parity(reason: &str) -> CheckResult { + CheckResult { + name: "Version parity".to_string(), + status: CheckStatus::Warning, + summary: "skipped".to_string(), + details: vec![CheckDetail::new(format!( + "Could not retrieve server version: {reason}" + ))], + remediation: None, + } +} + fn convert_diagnostics_status(status: api_types::DiagnosticsCheckStatus) -> CheckStatus { match status { api_types::DiagnosticsCheckStatus::Pass => CheckStatus::Pass, @@ -372,47 +384,46 @@ pub(crate) async fn run_doctor( } }; - let health = match server.api().get_health().send().await { - Ok(response) => response.into_inner(), - Err(err) => { - report.sections.push(CheckSection { - title: "Server".to_string(), - checks: vec![CheckResult { - name: "Fabro server".to_string(), - status: CheckStatus::Error, - summary: "health check failed".to_string(), - details: vec![CheckDetail::new(err.to_string())], - remediation: Some( - "Check that the server is reachable and responding to /health.".to_string(), - ), - }], - }); + if let Err(err) = server.api().get_health().send().await { + report.sections.push(CheckSection { + title: "Server".to_string(), + checks: vec![CheckResult { + name: "Fabro server".to_string(), + status: CheckStatus::Error, + summary: "health check failed".to_string(), + details: vec![CheckDetail::new(err.to_string())], + remediation: Some( + "Check that the server is reachable and responding to /health.".to_string(), + ), + }], + }); - if let Some(spinner) = spinner { - spinner.finish_and_clear(); - } - - if json { - print_json_pretty(&report)?; - } else { - render_report(&report, &styles, verbose, printer); - } - return Ok(1); + if let Some(spinner) = spinner { + spinner.finish_and_clear(); } - }; - report.sections[0] - .checks - .push(check_version_parity(&health.version)); + if json { + print_json_pretty(&report)?; + } else { + render_report(&report, &styles, verbose, printer); + } + return Ok(1); + } match server.api().run_diagnostics().send().await { Ok(response) => { let diagnostics = response.into_inner(); + report.sections[0] + .checks + .push(check_version_parity(&diagnostics.version)); report .sections .extend(convert_diagnostics_sections(diagnostics.sections)); } Err(err) => { + report.sections[0] + .checks + .push(skipped_version_parity(&err.to_string())); report.sections.push(CheckSection { title: "Server".to_string(), checks: vec![CheckResult { @@ -568,6 +579,18 @@ mod tests { assert_eq!(result.status, CheckStatus::Warning); } + #[test] + fn version_parity_skipped_when_diagnostics_unavailable() { + let result = skipped_version_parity("boom"); + assert_eq!(result.name, "Version parity"); + assert_eq!(result.status, CheckStatus::Warning); + assert_eq!(result.summary, "skipped"); + assert_eq!( + result.details[0].text, + "Could not retrieve server version: boom" + ); + } + #[test] fn render_report_text_without_color_has_no_ansi() { let report = CheckReport { diff --git a/lib/crates/fabro-cli/src/commands/mod.rs b/lib/crates/fabro-cli/src/commands/mod.rs index 72995e763..4493bb821 100644 --- a/lib/crates/fabro-cli/src/commands/mod.rs +++ b/lib/crates/fabro-cli/src/commands/mod.rs @@ -20,4 +20,5 @@ pub(crate) mod system; pub(crate) mod uninstall; pub(crate) mod upgrade; pub(crate) mod validate; +pub(crate) mod version; pub(crate) mod workflow; diff --git a/lib/crates/fabro-cli/src/commands/version.rs b/lib/crates/fabro-cli/src/commands/version.rs new file mode 100644 index 000000000..6b90f976d --- /dev/null +++ b/lib/crates/fabro-cli/src/commands/version.rs @@ -0,0 +1,212 @@ +use anyhow::Result; +use fabro_types::settings::CliSettings; +use fabro_types::settings::cli::{CliLayer, OutputFormat}; +use fabro_util::printer::Printer; +use serde_json::{Map, Value, json}; + +use crate::args::VersionArgs; +use crate::command_context::CommandContext; +use crate::server_client; +use crate::shared::print_json_pretty; +use crate::user_config::{self, ServerTarget}; + +pub(crate) async fn version_command( + args: &VersionArgs, + cli: &CliSettings, + cli_layer: &CliLayer, + printer: Printer, +) -> Result<()> { + let client = client_info(); + let ctx = CommandContext::for_target(&args.target, printer, cli.clone(), cli_layer)?; + let server_target = user_config::resolve_server_target(&args.target, ctx.machine_settings())?; + let server_address = format_server_target(&server_target); + let server_info = match ctx.server().await { + Ok(server) => match server + .api() + .get_system_info() + .send() + .await + .map_err(server_client::map_api_error) + { + Ok(response) => { + let response = response.into_inner(); + ServerVersionInfo::Success { + address: server_address, + version: response.version, + git_sha: response.git_sha, + build_date: response.build_date, + os: response.os, + arch: response.arch, + uptime_secs: response.uptime_secs, + } + } + Err(err) => ServerVersionInfo::Error { + address: server_address, + error: err.to_string(), + }, + }, + Err(err) => ServerVersionInfo::Error { + address: server_address, + error: err.to_string(), + }, + }; + + if cli.output.format == OutputFormat::Json { + print_json_pretty(&json_output(&client, &server_info))?; + return Ok(()); + } + + print_text_output(&client, &server_info); + Ok(()) +} + +struct ClientVersionInfo { + version: &'static str, + git_sha: &'static str, + build_date: &'static str, + os: &'static str, + arch: &'static str, +} + +enum ServerVersionInfo { + Success { + address: String, + version: Option, + git_sha: Option, + build_date: Option, + os: Option, + arch: Option, + uptime_secs: Option, + }, + Error { + address: String, + error: String, + }, +} + +fn client_info() -> ClientVersionInfo { + ClientVersionInfo { + version: env!("CARGO_PKG_VERSION"), + git_sha: env!("FABRO_GIT_SHA"), + build_date: env!("FABRO_BUILD_DATE"), + os: std::env::consts::OS, + arch: std::env::consts::ARCH, + } +} + +fn format_server_target(target: &ServerTarget) -> String { + match target { + ServerTarget::HttpUrl { api_url, .. } => api_url.clone(), + ServerTarget::UnixSocket(path) => path.display().to_string(), + } +} + +fn json_output(client: &ClientVersionInfo, server: &ServerVersionInfo) -> Value { + let client = json!({ + "version": client.version, + "git_sha": client.git_sha, + "build_date": client.build_date, + "os": client.os, + "arch": client.arch, + }); + + let mut server_map = Map::new(); + match server { + ServerVersionInfo::Success { + address, + version, + git_sha, + build_date, + os, + arch, + uptime_secs, + } => { + server_map.insert("address".to_string(), Value::String(address.clone())); + if let Some(version) = version { + server_map.insert("version".to_string(), Value::String(version.clone())); + } + if let Some(git_sha) = git_sha { + server_map.insert("git_sha".to_string(), Value::String(git_sha.clone())); + } + if let Some(build_date) = build_date { + server_map.insert("build_date".to_string(), Value::String(build_date.clone())); + } + if let Some(os) = os { + server_map.insert("os".to_string(), Value::String(os.clone())); + } + if let Some(arch) = arch { + server_map.insert("arch".to_string(), Value::String(arch.clone())); + } + if let Some(uptime_secs) = uptime_secs { + server_map.insert("uptime_secs".to_string(), Value::from(*uptime_secs)); + } + } + ServerVersionInfo::Error { address, error } => { + server_map.insert("address".to_string(), Value::String(address.clone())); + server_map.insert("error".to_string(), Value::String(error.clone())); + } + } + + json!({ + "client": client, + "server": server_map, + }) +} + +#[allow(clippy::print_stdout)] +fn print_text_output(client: &ClientVersionInfo, server: &ServerVersionInfo) { + println!("Client:"); + println!(" Version: {}", client.version); + println!(" Git SHA: {}", client.git_sha); + println!(" Build Date: {}", client.build_date); + println!(" OS/Arch: {}/{}", client.os, client.arch); + println!(); + + match server { + ServerVersionInfo::Success { + address, + version, + git_sha, + build_date, + os, + arch, + uptime_secs, + } => { + println!("Server: {address}"); + println!(" Version: {}", version.as_deref().unwrap_or("unknown")); + println!(" Git SHA: {}", git_sha.as_deref().unwrap_or("unknown")); + println!( + " Build Date: {}", + build_date.as_deref().unwrap_or("unknown") + ); + println!( + " OS/Arch: {}/{}", + os.as_deref().unwrap_or("unknown"), + arch.as_deref().unwrap_or("unknown") + ); + println!( + " Uptime: {}", + format_uptime(uptime_secs.unwrap_or_default()) + ); + } + ServerVersionInfo::Error { address, error } => { + println!("Server: {address}"); + println!(" Error: {error}"); + } + } +} + +fn format_uptime(total_secs: i64) -> String { + let total_secs = total_secs.max(0); + let hours = total_secs / 3600; + let minutes = (total_secs % 3600) / 60; + let seconds = total_secs % 60; + + if hours > 0 { + format!("{hours}h {minutes}m") + } else if minutes > 0 { + format!("{minutes}m") + } else { + format!("{seconds}s") + } +} diff --git a/lib/crates/fabro-cli/src/main.rs b/lib/crates/fabro-cli/src/main.rs index 35481ee2c..267c61a12 100644 --- a/lib/crates/fabro-cli/src/main.rs +++ b/lib/crates/fabro-cli/src/main.rs @@ -254,6 +254,10 @@ async fn main_inner() -> (String, Result<()>) { .await?; std::process::exit(exit_code); } + Commands::Version(args) => { + commands::version::version_command(&args, &cli_settings, &cli_layer, printer) + .await?; + } Commands::Discord => { if process_local_json { shared::print_json_pretty(&serde_json::json!({ diff --git a/lib/crates/fabro-cli/tests/it/cmd/doctor.rs b/lib/crates/fabro-cli/tests/it/cmd/doctor.rs index 553af0aeb..394b6713b 100644 --- a/lib/crates/fabro-cli/tests/it/cmd/doctor.rs +++ b/lib/crates/fabro-cli/tests/it/cmd/doctor.rs @@ -84,4 +84,8 @@ async fn twin_doctor() { stdout.to_lowercase().contains("openai connectivity: ok"), "expected verbose doctor output to include openai probe success, got: {stdout}" ); + assert!( + stdout.contains("Version parity"), + "expected doctor output to include version parity check, got: {stdout}" + ); } diff --git a/lib/crates/fabro-cli/tests/it/cmd/fabro.rs b/lib/crates/fabro-cli/tests/it/cmd/fabro.rs index 8d920e61b..e2e7aa12f 100644 --- a/lib/crates/fabro-cli/tests/it/cmd/fabro.rs +++ b/lib/crates/fabro-cli/tests/it/cmd/fabro.rs @@ -31,6 +31,7 @@ fn help() { model List and test LLM models server Server operations doctor Check environment and integration health + version Show client and server version information install Set up the Fabro environment (LLMs, certs, GitHub) uninstall Uninstall Fabro from this machine pr Pull request operations diff --git a/lib/crates/fabro-cli/tests/it/cmd/mod.rs b/lib/crates/fabro-cli/tests/it/cmd/mod.rs index aad21bc9a..1c335d307 100644 --- a/lib/crates/fabro-cli/tests/it/cmd/mod.rs +++ b/lib/crates/fabro-cli/tests/it/cmd/mod.rs @@ -63,6 +63,7 @@ mod top_level; mod uninstall; mod upgrade; mod validate; +mod version; mod wait; mod workflow; mod workflow_create; diff --git a/lib/crates/fabro-cli/tests/it/cmd/version.rs b/lib/crates/fabro-cli/tests/it/cmd/version.rs new file mode 100644 index 000000000..98a525cdb --- /dev/null +++ b/lib/crates/fabro-cli/tests/it/cmd/version.rs @@ -0,0 +1,112 @@ +use fabro_test::{fabro_snapshot, test_context}; +use serde_json::Value; + +#[test] +fn help() { + let context = test_context!(); + let mut cmd = context.command(); + cmd.args(["version", "--help"]); + fabro_snapshot!(context.filters(), cmd, @" + success: true + exit_code: 0 + ----- stdout ----- + Show client and server version information + + Usage: fabro version [OPTIONS] + + Options: + --json Output as JSON [env: FABRO_JSON=] + --server Fabro server target: http(s) URL or absolute Unix socket path [env: FABRO_SERVER=] + --debug Enable DEBUG-level logging (default is INFO) [env: FABRO_DEBUG=] + --no-upgrade-check Disable automatic upgrade check [env: FABRO_NO_UPGRADE_CHECK=true] + --quiet Suppress non-essential output [env: FABRO_QUIET=] + --verbose Enable verbose output [env: FABRO_VERBOSE=] + -h, --help Print help + ----- stderr ----- + "); +} + +#[test] +fn client_info_always_prints() { + let context = test_context!(); + let output = context + .command() + .args(["version"]) + .output() + .expect("command should run"); + + assert!(output.status.success(), "version failed"); + let stdout = String::from_utf8(output.stdout).expect("stdout should be utf-8"); + assert!( + stdout.contains("Client:"), + "missing client section: {stdout}" + ); + assert!( + stdout.contains("Server:"), + "missing server section: {stdout}" + ); +} + +#[test] +fn json_output() { + let context = test_context!(); + let output = context + .command() + .args(["--json", "version"]) + .output() + .expect("command should run"); + + assert!(output.status.success(), "json version failed"); + let value: Value = serde_json::from_slice(&output.stdout).expect("json should parse"); + assert!(value["client"]["version"].is_string()); + assert!(value["server"]["version"].is_string()); +} + +#[test] +fn http_unreachable_shows_error() { + let context = test_context!(); + let output = context + .command() + .args(["version", "--server", "http://127.0.0.1:1"]) + .output() + .expect("command should run"); + + assert!(output.status.success(), "version should still succeed"); + let stdout = String::from_utf8(output.stdout).expect("stdout should be utf-8"); + assert!( + stdout.contains("Client:"), + "missing client section: {stdout}" + ); + assert!(stdout.contains("Error:"), "missing error section: {stdout}"); +} + +#[test] +fn http_unreachable_json() { + let context = test_context!(); + let output = context + .command() + .args(["--json", "version", "--server", "http://127.0.0.1:1"]) + .output() + .expect("command should run"); + + assert!(output.status.success(), "version should still succeed"); + let value: Value = serde_json::from_slice(&output.stdout).expect("json should parse"); + assert!(value["client"]["version"].is_string()); + assert!(value["server"]["address"].is_string()); + assert!(value["server"]["error"].is_string()); + assert!(value["server"].get("version").is_none()); +} + +#[test] +fn invalid_server_target_fails() { + let context = test_context!(); + let output = context + .command() + .args(["version", "--server", "not-a-url"]) + .output() + .expect("command should run"); + + assert!(!output.status.success(), "version should fail"); + let stderr = String::from_utf8(output.stderr).expect("stderr should be utf-8"); + assert!(!stderr.is_empty(), "expected error output"); +} diff --git a/lib/crates/fabro-server/src/server.rs b/lib/crates/fabro-server/src/server.rs index ad7f451a8..0d395d908 100644 --- a/lib/crates/fabro-server/src/server.rs +++ b/lib/crates/fabro-server/src/server.rs @@ -1113,7 +1113,6 @@ async fn not_implemented() -> Response { async fn health() -> Response { Json(serde_json::json!({ "status": "ok", - "version": FABRO_VERSION, })) .into_response() } diff --git a/lib/crates/fabro-server/tests/it/api/routing.rs b/lib/crates/fabro-server/tests/it/api/routing.rs index b0ee0c210..1d6900139 100644 --- a/lib/crates/fabro-server/tests/it/api/routing.rs +++ b/lib/crates/fabro-server/tests/it/api/routing.rs @@ -54,6 +54,10 @@ async fn root_and_health_stay_at_root() { assert_eq!(health_response.status(), StatusCode::OK); let health_body = body_json(health_response.into_body()).await; assert_eq!(health_body["status"], "ok"); + assert!( + health_body.get("version").is_none(), + "health endpoint should not expose version" + ); } #[tokio::test] diff --git a/lib/packages/fabro-api-client/src/models/health-response.ts b/lib/packages/fabro-api-client/src/models/health-response.ts index 4f2a5ab72..5a2dc0c8f 100644 --- a/lib/packages/fabro-api-client/src/models/health-response.ts +++ b/lib/packages/fabro-api-client/src/models/health-response.ts @@ -22,9 +22,5 @@ export interface HealthResponse { * Health status indicator. */ 'status': string; - /** - * Server version string. - */ - 'version': string; }