feat(cli): exit with code 4 on authentication-required failures

Returns exit code 4 whenever the CLI fails because the user needs to run
fabro auth login, so scripts and the install wizard can distinguish
re-auth from generic failures.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
Bryan Helmkamp 2026-04-21 23:34:21 -04:00
parent 720fbb210a
commit 105559bc8a
No known key found for this signature in database
12 changed files with 489 additions and 37 deletions

1
Cargo.lock generated
View file

@ -1699,6 +1699,7 @@ dependencies = [
"fabro-util",
"fs2",
"futures",
"httpmock",
"libc",
"progenitor-client",
"rand 0.9.4",

View file

@ -16,6 +16,7 @@ use fabro_mcp::config::{McpServerSettings, McpTransport};
use fabro_types::settings::cli::OutputFormat as SettingsOutputFormat;
use fabro_types::settings::run::McpEntryLayer;
use fabro_types::settings::{CliSettings, InterpString};
use fabro_util::exit::{ErrorExt, ExitClass};
use fabro_util::printer::Printer;
use futures::stream;
use serde::Deserialize;
@ -212,6 +213,27 @@ fn transport_error(provider: &str, err: &anyhow::Error) -> LlmError {
}
}
fn classify_server_agent_auth(err: anyhow::Error) -> anyhow::Error {
let is_auth = err.chain().any(|cause| {
cause
.downcast_ref::<fabro_agent::Error>()
.is_some_and(|error| {
matches!(
error,
fabro_agent::Error::Llm(fabro_llm::Error::Provider {
kind: fabro_llm::ProviderErrorKind::Authentication,
..
})
)
})
});
if is_auth {
err.classify(ExitClass::AuthRequired)
} else {
err
}
}
fn map_response_failure(provider: &str, failure: &fabro_client::ApiError) -> LlmError {
let retry_after = parse_retry_after(&failure.headers);
let (message, code, raw) = parse_server_error_body(&failure.body);
@ -430,7 +452,9 @@ pub(crate) async fn execute(
.register_provider(adapter)
.await
.map_err(|e| anyhow::anyhow!("Failed to register fabro server adapter: {e}"))?;
run_with_args_and_client(args.agent, Some(client), mcp_servers).await?;
run_with_args_and_client(args.agent, Some(client), mcp_servers)
.await
.map_err(classify_server_agent_auth)?;
} else {
tracing::info!(transport = "direct", "Agent session starting");
run_with_args(args.agent, mcp_servers).await?;

View file

@ -30,6 +30,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;
use fabro_util::printer::Printer;
use fabro_util::terminal::Styles;
use rustls::crypto::ring::default_provider;
@ -77,6 +78,7 @@ async fn main() {
let (command_name, result) = Box::pin(main_inner()).await;
let duration_ms = u64::try_from(start.elapsed().as_millis()).unwrap_or(u64::MAX);
let exit_code = result.as_ref().err().map_or(0, exit::exit_code_for);
let is_error = result.is_err();
// An empty command_name means no subcommand was invoked (landing was shown);
@ -93,7 +95,7 @@ async fn main() {
"repository": repository,
"ci": ci,
"success": false,
"exitCode": 1,
"exitCode": exit_code,
}, error);
} else {
fabro_telemetry::track!("CLI Executed", {
@ -127,7 +129,7 @@ async fn main() {
}
}
}
std::process::exit(1);
std::process::exit(exit_code);
}
}

View file

@ -14,6 +14,8 @@ use fabro_test::{fabro_snapshot, preserve_coverage_env, test_context};
use httpmock::MockServer;
use serde_json::json;
use crate::support::fatal_error_line;
async fn run_success_output(mut cmd: assert_cmd::Command) -> Output {
tokio::task::spawn_blocking(move || cmd.assert().success().get_output().clone())
.await
@ -308,6 +310,90 @@ fn exec_server_target_uses_saved_cli_auth_without_local_api_key_resolution() {
);
}
#[test]
fn exec_server_target_auth_failure_exits_with_4() {
let context = test_context!();
let server = MockServer::start();
server.mock(|when, then| {
when.method("POST").path("/api/v1/completions");
then.status(401)
.header("Content-Type", "application/json")
.json_body(json!({
"errors": [{
"status": "401",
"title": "Unauthorized",
"detail": "Authentication required.",
"code": "authentication_required"
}]
}));
});
let mut cmd = context.exec_cmd();
cmd.env_clear();
preserve_coverage_env!(cmd);
cmd.env("HOME", &context.home_dir);
cmd.env("FABRO_NO_UPGRADE_CHECK", "true")
.env("FABRO_HTTP_PROXY_POLICY", "disabled");
cmd.args([
"--server",
&format!("{}/api/v1", server.base_url()),
"--provider",
"openai",
"--model",
"gpt-5.4-mini",
"test prompt",
]);
let output = cmd.assert().failure().get_output().clone();
assert_eq!(output.status.code(), Some(4));
assert_eq!(
fatal_error_line(&output.stderr),
"LLM error: Authentication error for openai: Authentication required."
);
}
#[test]
fn exec_direct_provider_auth_failure_stays_exit_1() {
let context = test_context!();
let server = MockServer::start();
server.mock(|when, then| {
when.method("POST")
.path("/v1/messages")
.header("x-api-key", "test-key");
then.status(401)
.header("Content-Type", "application/json")
.json_body(json!({
"error": {
"type": "authentication_error",
"message": "bad key"
}
}));
});
let mut cmd = context.exec_cmd();
cmd.env_clear();
preserve_coverage_env!(cmd);
cmd.env("HOME", &context.home_dir);
cmd.env("FABRO_NO_UPGRADE_CHECK", "true")
.env("FABRO_HTTP_PROXY_POLICY", "disabled")
.env("ANTHROPIC_API_KEY", "test-key")
.env("ANTHROPIC_BASE_URL", format!("{}/v1", server.base_url()));
cmd.args([
"--provider",
"anthropic",
"--model",
"claude-haiku-4-5",
"test prompt",
]);
let output = cmd.assert().failure().get_output().clone();
assert_eq!(output.status.code(), Some(1));
assert_eq!(
fatal_error_line(&output.stderr),
"LLM error: Authentication error for anthropic: bad key"
);
}
fn write_auth_entry(
context: &fabro_test::TestContext,
target: &str,

View file

@ -3,7 +3,7 @@ use httpmock::MockServer;
use serde_json::Value;
use super::support::{local_dev_token, setup_completed_fast_dry_run, setup_created_fast_dry_run};
use crate::support::unique_run_id;
use crate::support::{fatal_error_line, unique_run_id};
#[test]
fn help() {
@ -83,12 +83,8 @@ fn ps_explicit_local_tcp_server_target_requires_explicit_auth() {
String::from_utf8_lossy(&output.stdout),
String::from_utf8_lossy(&output.stderr)
);
assert!(
String::from_utf8_lossy(&output.stderr).contains("Authentication required."),
"explicit local TCP target should fail with an auth error:\nstdout:\n{}\nstderr:\n{}",
String::from_utf8_lossy(&output.stdout),
String::from_utf8_lossy(&output.stderr)
);
assert_eq!(output.status.code(), Some(4));
assert_eq!(fatal_error_line(&output.stderr), "Authentication required.");
}
#[test]

View file

@ -19,7 +19,7 @@ use serde_json::{Value, json};
use crate::support::{
RealAuthHarness, TEST_DEV_TOKEN, complete_login_via_browser, expire_saved_access_token,
no_redirect_browser_client, run_detached, saved_auth_entry,
fatal_error_line, no_redirect_browser_client, run_detached, saved_auth_entry,
};
const LOGIN_TIMEOUT: Duration = Duration::from_secs(10);
@ -235,10 +235,10 @@ fn auth_refresh_failure_clears_local_session() {
!system_info.status.success(),
"system info should fail when refresh is revoked"
);
assert!(
String::from_utf8_lossy(&system_info.stderr).contains("fabro auth login"),
"refresh failure should direct the user to log in again:\n{}",
String::from_utf8_lossy(&system_info.stderr)
assert_eq!(system_info.status.code(), Some(4));
assert_eq!(
fatal_error_line(&system_info.stderr),
"CLI session has expired. Run `fabro auth login` again."
);
expired_access_mock.assert();
refresh_mock.assert();

View file

@ -62,6 +62,20 @@ pub(crate) fn run_output_filters(context: &TestContext) -> Vec<(String, String)>
filters
}
pub(crate) fn fatal_error_line(stderr: &[u8]) -> String {
static ANSI_RE: std::sync::OnceLock<regex::Regex> = std::sync::OnceLock::new();
let ansi_re = ANSI_RE.get_or_init(|| {
regex::Regex::new(r"\x1b\[[0-9;]*m").expect("ANSI-stripping regex should compile")
});
let stderr = String::from_utf8_lossy(stderr);
let stripped = ansi_re.replace_all(&stderr, "");
stripped
.lines()
.rev()
.find_map(|line| line.strip_prefix("error: ").map(ToOwned::to_owned))
.expect("stderr should contain a fatal `error:` line")
}
pub(crate) fn unique_run_id() -> String {
RunId::new().to_string()
}

View file

@ -34,4 +34,5 @@ tokio-util.workspace = true
tracing.workspace = true
[dev-dependencies]
httpmock = "0.8"
tempfile = "3"

View file

@ -13,6 +13,7 @@ use fabro_model::Model;
use fabro_types::{
ArtifactUpload, EventEnvelope, RunBlobId, RunEvent, RunId, RunProjection, RunSummary, StageId,
};
use fabro_util::exit::{ErrorExt, ExitClass};
use futures::StreamExt;
use futures::future::BoxFuture;
use serde::{Deserialize, Serialize};
@ -336,8 +337,13 @@ impl Client {
}
async fn refresh_access_token(&self, failed_access_token: &str) -> Result<()> {
fn session_expired() -> anyhow::Error {
anyhow!("CLI session has expired. Run `fabro auth login` again.")
.classify(ExitClass::AuthRequired)
}
let Some(oauth_session) = &self.oauth_session else {
bail!("CLI session has expired. Run `fabro auth login` again.");
return Err(session_expired());
};
let _guard = self.refresh_lock.lock().await;
@ -348,12 +354,12 @@ impl Client {
let Some(entry) = oauth_session.auth_store.get(&oauth_session.target)? else {
self.rebuild_with_fallback(oauth_session).await?;
bail!("CLI session has expired. Run `fabro auth login` again.");
return Err(session_expired());
};
if entry.refresh_token_expires_at <= chrono::Utc::now() {
oauth_session.auth_store.remove(&oauth_session.target)?;
self.rebuild_with_fallback(oauth_session).await?;
bail!("CLI session has expired. Run `fabro auth login` again.");
return Err(session_expired());
}
ensure_refresh_target_transport(&oauth_session.target)?;
@ -394,27 +400,34 @@ impl Client {
let status = response.status();
let body = response.text().await.unwrap_or_default();
let parsed_error = serde_json::from_str::<OAuthErrorBody>(&body).ok();
if parsed_error.as_ref().is_some_and(|error| {
let auth_recoverable = parsed_error.as_ref().is_some_and(|error| {
matches!(
error.error.as_str(),
"refresh_token_expired" | "refresh_token_revoked"
)
}) {
});
if auth_recoverable {
oauth_session.auth_store.remove(&oauth_session.target)?;
self.rebuild_with_fallback(oauth_session).await?;
}
if let Some(parsed_error) = parsed_error {
let err = if let Some(parsed_error) = parsed_error {
let message = parsed_error
.error_description
.filter(|value| !value.is_empty())
.unwrap_or_else(|| format!("request failed with status {status}"));
bail!("{message}");
}
if body.is_empty() {
bail!("request failed with status {status}");
}
bail!("request failed with status {status}: {body}");
anyhow!("{message}")
} else if body.is_empty() {
anyhow!("request failed with status {status}")
} else {
anyhow!("request failed with status {status}: {body}")
};
Err(if auth_recoverable {
err.classify(ExitClass::AuthRequired)
} else {
err
})
}
async fn rebuild_with_fallback(&self, oauth_session: &OAuthSession) -> Result<()> {
@ -1333,6 +1346,10 @@ fn non_zero_u64_from_usize(value: usize) -> Option<NonZeroU64> {
#[cfg(test)]
mod tests {
use chrono::Duration as ChronoDuration;
use fabro_util::exit;
use httpmock::Method::POST;
use httpmock::MockServer;
use serde_json::json;
use super::*;
use crate::AuthStore;
@ -1389,4 +1406,106 @@ mod tests {
);
assert!(auth_store.get(&target).unwrap().is_some());
}
async fn oauth_client(
server: &MockServer,
) -> (tempfile::TempDir, Client, AuthStore, ServerTarget) {
let temp = tempfile::tempdir().unwrap();
let auth_store = AuthStore::new(temp.path().join("auth.json"));
let target = ServerTarget::http_url(server.base_url()).unwrap();
let entry = oauth_entry("octocat");
auth_store.put(&target, entry.clone()).unwrap();
let client = Client::builder()
.target(target.clone())
.credential(Credential::OAuth(entry))
.oauth_session(OAuthSession::new(target.clone(), auth_store.clone()))
.transport(
server.base_url(),
fabro_http::HttpClientBuilder::new()
.no_proxy()
.build()
.unwrap(),
)
.connect()
.await
.unwrap();
(temp, client, auth_store, target)
}
#[tokio::test]
async fn refresh_access_token_classifies_expired_refresh_tokens() {
let server = MockServer::start();
server.mock(|when, then| {
when.method(POST)
.path("/auth/cli/refresh")
.header("authorization", "Bearer refresh-octocat");
then.status(401)
.header("Content-Type", "application/json")
.json_body(json!({
"error": "refresh_token_expired",
"error_description": "CLI session has expired. Run `fabro auth login` again."
}));
});
let (_temp, client, auth_store, target) = oauth_client(&server).await;
let err = client
.refresh_access_token("access-octocat")
.await
.unwrap_err();
assert_eq!(exit::exit_code_for(&err), 4);
assert!(auth_store.get(&target).unwrap().is_none());
}
#[tokio::test]
async fn refresh_access_token_keeps_server_errors_as_exit_1() {
let server = MockServer::start();
server.mock(|when, then| {
when.method(POST)
.path("/auth/cli/refresh")
.header("authorization", "Bearer refresh-octocat");
then.status(500)
.header("Content-Type", "application/json")
.json_body(json!({
"error": "server_error",
"error_description": "OAuth server exploded."
}));
});
let (_temp, client, auth_store, target) = oauth_client(&server).await;
let err = client
.refresh_access_token("access-octocat")
.await
.unwrap_err();
assert_eq!(exit::exit_code_for(&err), 1);
assert!(auth_store.get(&target).unwrap().is_some());
}
#[tokio::test]
async fn refresh_access_token_keeps_login_not_permitted_as_exit_1() {
let server = MockServer::start();
server.mock(|when, then| {
when.method(POST)
.path("/auth/cli/refresh")
.header("authorization", "Bearer refresh-octocat");
then.status(403)
.header("Content-Type", "application/json")
.json_body(json!({
"error": "unauthorized",
"error_description": "Login not permitted for this user."
}));
});
let (_temp, client, auth_store, target) = oauth_client(&server).await;
let err = client
.refresh_access_token("access-octocat")
.await
.unwrap_err();
assert_eq!(exit::exit_code_for(&err), 1);
assert!(auth_store.get(&target).unwrap().is_some());
}
}

View file

@ -1,4 +1,5 @@
use anyhow::{Result, anyhow};
use fabro_util::exit::{ErrorExt, ExitClass};
use serde::de::DeserializeOwned;
#[derive(Debug, Clone, PartialEq, Eq)]
@ -41,6 +42,14 @@ pub fn parse_error_response_value(value: &serde_json::Value) -> (Option<String>,
(detail, code)
}
fn classify_from_status(err: anyhow::Error, status: fabro_http::StatusCode) -> anyhow::Error {
if status == fabro_http::StatusCode::UNAUTHORIZED {
err.classify(ExitClass::AuthRequired)
} else {
err
}
}
pub async fn classify_api_error<E>(err: progenitor_client::Error<E>) -> StructuredApiError
where
E: serde::Serialize + std::fmt::Debug,
@ -55,7 +64,7 @@ where
code = parsed_code;
if let Some(detail) = detail {
return StructuredApiError {
error: anyhow!("{detail}"),
error: classify_from_status(anyhow!("{detail}"), status),
failure: Some(ApiFailure { status, code }),
};
}
@ -66,7 +75,7 @@ where
anyhow!("request failed with status {status}: {body}")
};
StructuredApiError {
error,
error: classify_from_status(error, status),
failure: Some(ApiFailure { status, code }),
}
}
@ -87,18 +96,24 @@ where
code = parsed_code;
if let Some(detail) = detail {
return StructuredApiError {
error: anyhow!("{detail}"),
error: classify_from_status(anyhow!("{detail}"), status),
failure: Some(ApiFailure { status, code }),
};
}
}
StructuredApiError {
error: anyhow!("request failed with status {status}"),
error: classify_from_status(
anyhow!("request failed with status {status}"),
status,
),
failure: Some(ApiFailure { status, code }),
}
}
progenitor_client::Error::UnexpectedResponse(response) => StructuredApiError {
error: anyhow!("request failed with status {}", response.status()),
error: classify_from_status(
anyhow!("request failed with status {}", response.status()),
response.status(),
),
failure: Some(ApiFailure {
status: response.status(),
code: None,
@ -122,18 +137,24 @@ pub fn raw_response_failure_error(failure: &ApiError) -> anyhow::Error {
if let Ok(value) = serde_json::from_str::<serde_json::Value>(&failure.body) {
let (detail, _) = parse_error_response_value(&value);
if let Some(detail) = detail {
return anyhow!("{detail}");
return classify_from_status(anyhow!("{detail}"), failure.status);
}
}
if failure.body.is_empty() {
return anyhow!("request failed with status {}", failure.status);
return classify_from_status(
anyhow!("request failed with status {}", failure.status),
failure.status,
);
}
anyhow!(
"request failed with status {}: {}",
classify_from_status(
anyhow!(
"request failed with status {}: {}",
failure.status,
failure.body
),
failure.status,
failure.body
)
}
@ -182,3 +203,87 @@ where
{
serde_json::from_value(serde_json::to_value(value)?).map_err(Into::into)
}
#[cfg(test)]
mod tests {
use fabro_util::exit;
use serde_json::json;
use super::{ApiError, ApiFailure, map_api_error, raw_response_failure_error};
fn error_response(
status: fabro_http::StatusCode,
detail: &str,
code: &str,
) -> progenitor_client::Error<serde_json::Value> {
let response = progenitor_client::ResponseValue::new(
json!({
"errors": [{
"detail": detail,
"code": code,
}]
}),
status,
fabro_http::HeaderMap::new(),
);
progenitor_client::Error::ErrorResponse(response)
}
fn api_error(status: fabro_http::StatusCode, detail: &str, code: &str) -> ApiError {
ApiError {
status,
headers: fabro_http::HeaderMap::new(),
body: serde_json::to_string(&json!({
"errors": [{
"detail": detail,
"code": code,
}]
}))
.unwrap(),
failure: ApiFailure {
status,
code: Some(code.to_string()),
},
}
}
#[test]
fn map_api_error_marks_401_as_auth_required() {
let err = map_api_error(error_response(
fabro_http::StatusCode::UNAUTHORIZED,
"Authentication required.",
"authentication_required",
));
assert_eq!(exit::exit_code_for(&err), 4);
}
#[test]
fn map_api_error_keeps_500_as_exit_1() {
let err = map_api_error(error_response(
fabro_http::StatusCode::INTERNAL_SERVER_ERROR,
"Server exploded.",
"server_error",
));
assert_eq!(exit::exit_code_for(&err), 1);
}
#[test]
fn raw_response_failure_error_marks_401_as_auth_required() {
let err = raw_response_failure_error(&api_error(
fabro_http::StatusCode::UNAUTHORIZED,
"Authentication required.",
"authentication_required",
));
assert_eq!(exit::exit_code_for(&err), 4);
}
#[test]
fn raw_response_failure_error_keeps_403_as_exit_1() {
let err = raw_response_failure_error(&api_error(
fabro_http::StatusCode::FORBIDDEN,
"Forbidden.",
"forbidden",
));
assert_eq!(exit::exit_code_for(&err), 1);
}
}

View file

@ -0,0 +1,103 @@
use anyhow::Error;
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
pub enum ExitClass {
AuthRequired,
}
// Keep the wrapper transparent so existing stderr remains unchanged while the
// exit class stays discoverable via downcast.
struct Classified {
class: ExitClass,
inner: Error,
}
impl Classified {
const fn class(&self) -> ExitClass {
self.class
}
}
impl std::fmt::Debug for Classified {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
std::fmt::Debug::fmt(&self.inner, f)
}
}
impl std::fmt::Display for Classified {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
std::fmt::Display::fmt(&self.inner, f)
}
}
impl std::error::Error for Classified {
fn source(&self) -> Option<&(dyn std::error::Error + 'static)> {
self.inner.source()
}
}
pub trait ErrorExt {
fn classify(self, class: ExitClass) -> Error;
}
impl ErrorExt for Error {
fn classify(self, class: ExitClass) -> Error {
Self::new(Classified { class, inner: self })
}
}
pub fn exit_code_for(err: &Error) -> i32 {
err.chain()
.find_map(|cause| cause.downcast_ref::<Classified>())
.map_or(1, |classified| match classified.class() {
ExitClass::AuthRequired => 4,
})
}
#[cfg(test)]
mod tests {
use anyhow::anyhow;
use super::{ErrorExt, ExitClass, exit_code_for};
#[test]
fn unclassified_errors_default_to_exit_1() {
assert_eq!(exit_code_for(&anyhow!("boom")), 1);
}
#[test]
fn classified_errors_map_to_exit_4() {
let err = anyhow!("boom").classify(ExitClass::AuthRequired);
assert_eq!(exit_code_for(&err), 4);
}
#[test]
fn classification_keeps_display_transparent() {
assert_eq!(
anyhow!("boom")
.classify(ExitClass::AuthRequired)
.to_string(),
anyhow!("boom").to_string()
);
}
#[test]
fn classification_keeps_chain_length_transparent() {
assert_eq!(
anyhow!("boom")
.classify(ExitClass::AuthRequired)
.chain()
.count(),
anyhow!("boom").chain().count()
);
}
#[test]
fn buried_classification_still_resolves() {
let err = anyhow!("boom")
.context("while x")
.classify(ExitClass::AuthRequired)
.context("while y");
assert_eq!(exit_code_for(&err), 4);
}
}

View file

@ -2,6 +2,7 @@ pub mod backoff;
pub mod check_report;
pub mod dev_token;
pub mod env;
pub mod exit;
pub mod home;
pub mod json;
pub mod path;