From 92a09adb20aeecf14663315d5812e2ff9d6a6362 Mon Sep 17 00:00:00 2001 From: Bryan Helmkamp Date: Wed, 22 Apr 2026 10:49:22 -0400 Subject: [PATCH] feat(cli): suggest `fabro auth login` on auth-required errors Unauthenticated commands surfaced only `error: Authentication required.` with no remediation. Add a cyan-bold `hint:` line pointing at `fabro auth login` in the top-level error printer, keyed off `ExitClass::AuthRequired` so it covers every command that hits the server (run, exec, ps, system info, etc.). Suppressed when `--json` is set. Co-Authored-By: Claude Opus 4.7 (1M context) --- lib/crates/fabro-cli/src/main.rs | 12 +++ .../fabro-cli/tests/it/scenario/auth.rs | 78 +++++++++++++++++++ lib/crates/fabro-util/src/exit.rs | 27 ++++++- 3 files changed, 116 insertions(+), 1 deletion(-) diff --git a/lib/crates/fabro-cli/src/main.rs b/lib/crates/fabro-cli/src/main.rs index 8299f1208..51538e197 100644 --- a/lib/crates/fabro-cli/src/main.rs +++ b/lib/crates/fabro-cli/src/main.rs @@ -31,6 +31,7 @@ use fabro_config::merge::combine_files; use fabro_telemetry::{git, panic as tel_panic, sanitize, sender}; use fabro_types::settings::SettingsLayer; use fabro_types::settings::cli::OutputVerbosity; +use fabro_util::exit::ExitClass; use fabro_util::printer::Printer; use fabro_util::terminal::Styles; use fabro_util::{browser, exit}; @@ -130,6 +131,17 @@ async fn main() { } } } + let json_mode = raw_args.iter().any(|a| a == "--json"); + if !json_mode && exit::exit_class_for(&err) == Some(ExitClass::AuthRequired) { + let hint_style = console::Style::new().cyan().bold(); + let cmd_style = console::Style::new().bold(); + eprintln!(); + eprintln!( + "{} Run {} to authenticate.", + hint_style.apply_to("hint:"), + cmd_style.apply_to("`fabro auth login`"), + ); + } std::process::exit(exit_code); } } diff --git a/lib/crates/fabro-cli/tests/it/scenario/auth.rs b/lib/crates/fabro-cli/tests/it/scenario/auth.rs index eb7c7d71d..1f763659f 100644 --- a/lib/crates/fabro-cli/tests/it/scenario/auth.rs +++ b/lib/crates/fabro-cli/tests/it/scenario/auth.rs @@ -268,6 +268,84 @@ fn auth_refresh_failure_clears_local_session() { auth_required_mock.assert(); } +#[test] +fn auth_required_error_suggests_auth_login() { + let context = test_context!(); + let server = MockServer::start(); + let target = server_target(&server); + + let auth_required_mock = server.mock(|when, then| { + when.method(GET).path("/api/v1/system/info"); + then.status(401) + .header("Content-Type", "application/json") + .json_body(json!({ + "errors": [{ + "status": "401", + "title": "Unauthorized", + "detail": "Authentication required.", + "code": "authentication_required" + }] + })); + }); + + let output = context + .command() + .args(["system", "info", "--server", &target]) + .output() + .expect("system info should run"); + + assert_eq!(output.status.code(), Some(4)); + auth_required_mock.assert(); + + let stderr = console::strip_ansi_codes(&String::from_utf8_lossy(&output.stderr)).into_owned(); + assert!( + stderr.contains("Authentication required."), + "stderr should surface the auth error, got:\n{stderr}" + ); + assert!( + stderr.contains("Run `fabro auth login` to authenticate."), + "stderr should suggest `fabro auth login`, got:\n{stderr}" + ); +} + +#[test] +fn auth_required_hint_is_suppressed_in_json_mode() { + let context = test_context!(); + let server = MockServer::start(); + let target = server_target(&server); + + server.mock(|when, then| { + when.method(GET).path("/api/v1/system/info"); + then.status(401) + .header("Content-Type", "application/json") + .json_body(json!({ + "errors": [{ + "status": "401", + "title": "Unauthorized", + "detail": "Authentication required.", + "code": "authentication_required" + }] + })); + }); + + let output = context + .command() + .args(["--json", "system", "info", "--server", &target]) + .output() + .expect("system info should run"); + + assert_eq!(output.status.code(), Some(4)); + let stderr = console::strip_ansi_codes(&String::from_utf8_lossy(&output.stderr)).into_owned(); + assert!( + stderr.contains("Authentication required."), + "stderr should still surface the auth error in --json mode, got:\n{stderr}" + ); + assert!( + !stderr.contains("fabro auth login"), + "stderr should not include the hint in --json mode, got:\n{stderr}" + ); +} + #[test] fn auth_login_rejects_unix_socket_target() { let context = test_context!(); diff --git a/lib/crates/fabro-util/src/exit.rs b/lib/crates/fabro-util/src/exit.rs index ecfeae9a3..31b09d4c8 100644 --- a/lib/crates/fabro-util/src/exit.rs +++ b/lib/crates/fabro-util/src/exit.rs @@ -54,11 +54,17 @@ pub fn exit_code_for(err: &Error) -> i32 { }) } +pub fn exit_class_for(err: &Error) -> Option { + err.chain() + .find_map(|cause| cause.downcast_ref::()) + .map(Classified::class) +} + #[cfg(test)] mod tests { use anyhow::anyhow; - use super::{ErrorExt, ExitClass, exit_code_for}; + use super::{ErrorExt, ExitClass, exit_class_for, exit_code_for}; #[test] fn unclassified_errors_default_to_exit_1() { @@ -100,4 +106,23 @@ mod tests { .context("while y"); assert_eq!(exit_code_for(&err), 4); } + + #[test] + fn exit_class_for_returns_none_for_unclassified() { + assert_eq!(exit_class_for(&anyhow!("boom")), None); + } + + #[test] + fn exit_class_for_returns_auth_required() { + let err = anyhow!("boom").classify(ExitClass::AuthRequired); + assert_eq!(exit_class_for(&err), Some(ExitClass::AuthRequired)); + } + + #[test] + fn exit_class_for_resolves_through_context() { + let err = anyhow!("boom") + .classify(ExitClass::AuthRequired) + .context("while y"); + assert_eq!(exit_class_for(&err), Some(ExitClass::AuthRequired)); + } }