diff --git a/lib/crates/fabro-auth/src/resolve.rs b/lib/crates/fabro-auth/src/resolve.rs index be9462fd3..604aa3262 100644 --- a/lib/crates/fabro-auth/src/resolve.rs +++ b/lib/crates/fabro-auth/src/resolve.rs @@ -262,7 +262,9 @@ impl CredentialResolver { fn codex_login_command(api_key: &str) -> String { let quoted = try_quote(api_key).map_or_else(|_| api_key.to_string(), std::borrow::Cow::into_owned); - format!("PATH=\"$HOME/.local/bin:$PATH\" echo {quoted} | codex login --with-api-key") + format!( + "export PATH=\"$HOME/.local/bin:$PATH\" && printf '%s\\n' {quoted} | codex login --with-api-key" + ) } fn credential_ids_for(provider: Provider, usage: CredentialUsage) -> &'static [&'static str] { @@ -283,6 +285,9 @@ fn credential_ids_for(provider: Provider, usage: CredentialUsage) -> &'static [& #[cfg(test)] mod tests { + #[cfg(unix)] + use std::os::unix::fs::PermissionsExt; + use chrono::{Duration, Utc}; use httpmock::Method::POST; use httpmock::MockServer; @@ -467,6 +472,64 @@ mod tests { assert!(cli.login_command.is_some()); } + #[cfg(unix)] + #[tokio::test] + async fn openai_api_key_cli_login_command_executes_codex_from_local_bin() { + let dir = tempfile::tempdir().unwrap(); + let local_bin = dir.path().join(".local/bin"); + std::fs::create_dir_all(&local_bin).unwrap(); + + let codex_path = local_bin.join("codex"); + std::fs::write( + &codex_path, + "#!/bin/sh\nprintf '%s\\n' \"$@\" > \"$HOME/codex-args.txt\"\ncat > \"$HOME/codex-stdin.txt\"\n", + ) + .unwrap(); + let mut permissions = std::fs::metadata(&codex_path).unwrap().permissions(); + permissions.set_mode(0o755); + std::fs::set_permissions(&codex_path, permissions).unwrap(); + + let mut vault = Vault::load(dir.path().join("secrets.json")).unwrap(); + vault_set_credential( + &mut vault, + "openai", + &api_key_credential(Provider::OpenAi, "openai-key"), + ) + .unwrap(); + let resolver = test_resolver(vault, Arc::new(|_| None)); + + let ResolvedCredential::Cli(cli) = resolver + .resolve( + Provider::OpenAi, + CredentialUsage::CliAgent(CliAgentKind::Codex), + ) + .await + .unwrap() + else { + panic!("expected cli credential"); + }; + + let status = std::process::Command::new("/bin/sh") + .arg("-lc") + .arg(cli.login_command.unwrap()) + .env("HOME", dir.path()) + .env("PATH", "/usr/bin:/bin") + .status() + .unwrap(); + + assert!(status.success()); + assert_eq!( + std::fs::read_to_string(dir.path().join("codex-args.txt")).unwrap(), + "login\n--with-api-key\n" + ); + assert_eq!( + std::fs::read_to_string(dir.path().join("codex-stdin.txt")) + .unwrap() + .trim_end(), + "openai-key" + ); + } + #[tokio::test] async fn with_env_lookup_overrides_vault_settings() { let dir = tempfile::tempdir().unwrap(); diff --git a/lib/crates/fabro-cli/src/args.rs b/lib/crates/fabro-cli/src/args.rs index 228e50921..47bde1374 100644 --- a/lib/crates/fabro-cli/src/args.rs +++ b/lib/crates/fabro-cli/src/args.rs @@ -599,6 +599,10 @@ pub(crate) struct ProviderLoginArgs { /// LLM provider to authenticate with #[arg(long)] pub(crate) provider: fabro_model::Provider, + + /// Read an API key from stdin instead of prompting + #[arg(long)] + pub(crate) api_key_stdin: bool, } #[derive(Args)] @@ -1268,6 +1272,39 @@ pub(crate) struct DoctorArgs { pub(crate) verbose: bool, } +#[derive(Debug, Clone, Copy, ValueEnum)] +pub(crate) enum InstallGitHubStrategyArg { + GhCli, + App, +} + +#[derive(Args, Debug, Clone, Default)] +pub(crate) struct InstallNonInteractiveArgs { + #[arg(long, hide = true)] + pub(crate) llm_provider: Option, + + #[arg(long, hide = true)] + pub(crate) llm_api_key_stdin: bool, + + #[arg(long, hide = true)] + pub(crate) llm_api_key_env: Option, + + #[arg(long, hide = true)] + pub(crate) github_strategy: Option, + + #[arg(long, hide = true)] + pub(crate) github_username: Option, + + #[arg(long, hide = true)] + pub(crate) overwrite_settings: bool, + + #[arg(long, hide = true)] + pub(crate) keep_existing_settings: bool, + + #[arg(long, hide = true)] + pub(crate) run_doctor: bool, +} + #[derive(Args)] pub(crate) struct InstallArgs { #[command(flatten)] @@ -1276,6 +1313,13 @@ pub(crate) struct InstallArgs { /// Base URL for the web UI (used for OAuth callback URLs) #[arg(long, default_value = "http://localhost:3000")] pub(crate) web_url: String, + + /// Run install without prompts; use hidden scripted flags for inputs + #[arg(long)] + pub(crate) non_interactive: bool, + + #[command(flatten)] + pub(crate) scripted: InstallNonInteractiveArgs, } #[derive(Args)] diff --git a/lib/crates/fabro-cli/src/commands/install.rs b/lib/crates/fabro-cli/src/commands/install.rs index 6f6907617..0bcc50214 100644 --- a/lib/crates/fabro-cli/src/commands/install.rs +++ b/lib/crates/fabro-cli/src/commands/install.rs @@ -1,8 +1,10 @@ use std::net::SocketAddr; use std::path::Path; use std::process::Stdio; +use std::time::Duration; use anyhow::{Context, Result, anyhow, bail}; +use async_trait::async_trait; use axum::extract::Query; use axum::response::Html; use axum::routing::get; @@ -12,16 +14,13 @@ use dialoguer::console::Term; use dialoguer::theme::ColorfulTheme; use dialoguer::{MultiSelect, Select}; use fabro_api::types::{CreateSecretRequest, SecretType as ApiSecretType}; -use fabro_auth::{ - AuthCredential, AuthMethod, codex_oauth_config, credential_id_for, parse_credential_secret, -}; +use fabro_auth::{AuthCredential, AuthMethod, codex_oauth_config, credential_id_for}; use fabro_config::user::SETTINGS_CONFIG_FILENAME; use fabro_config::{Storage, envfile, legacy_env}; use fabro_model::Provider; use fabro_util::printer::Printer; use fabro_util::terminal::Styles; -// Bootstrap-only direct vault writes for `fabro install` when no local server is running. -use fabro_vault::{SecretType, Vault}; +use futures::future::BoxFuture; use rand::Rng; use tokio::io::AsyncWriteExt; use tokio::net::TcpListener; @@ -30,11 +29,15 @@ use tokio::sync::oneshot; use tokio::task::spawn_blocking; use super::doctor; -use crate::args::{DoctorArgs, GlobalArgs, InstallArgs, ServerTargetArgs}; -use crate::commands::server::record; +use crate::args::{ + DoctorArgs, GlobalArgs, InstallArgs, InstallGitHubStrategyArg, InstallNonInteractiveArgs, + ServerTargetArgs, +}; +use crate::commands::server::{record, stop}; use crate::gh::GhCli; use crate::shared::provider_auth::{ - authenticate_provider, authenticate_provider_with_method, prompt_confirm, provider_display_name, + ApiKeySource, authenticate_provider, authenticate_provider_with_api_key_source, + authenticate_provider_with_method, prompt_confirm, provider_display_name, }; use crate::{server_client, user_config}; @@ -370,10 +373,410 @@ fn prompt_multiselect(prompt: &str, items: &[String]) -> Result> { .interact_on(&Term::stderr())?) } +impl InstallNonInteractiveArgs { + fn has_any(&self) -> bool { + self.llm_provider.is_some() + || self.llm_api_key_stdin + || self.llm_api_key_env.is_some() + || self.github_strategy.is_some() + || self.github_username.is_some() + || self.overwrite_settings + || self.keep_existing_settings + || self.run_doctor + } + + fn first_flag_name(&self) -> Option<&'static str> { + if self.llm_provider.is_some() { + Some("--llm-provider") + } else if self.llm_api_key_stdin { + Some("--llm-api-key-stdin") + } else if self.llm_api_key_env.is_some() { + Some("--llm-api-key-env") + } else if self.github_strategy.is_some() { + Some("--github-strategy") + } else if self.github_username.is_some() { + Some("--github-username") + } else if self.overwrite_settings { + Some("--overwrite-settings") + } else if self.keep_existing_settings { + Some("--keep-existing-settings") + } else if self.run_doctor { + Some("--run-doctor") + } else { + None + } + } +} + +fn non_interactive_install_usage() -> &'static str { + r#"Non-interactive install requires additional flags. + +Non-interactive usage: + fabro install --non-interactive \ + --llm-provider anthropic \ + --llm-api-key-env ANTHROPIC_API_KEY \ + --github-strategy gh_cli \ + --github-username brynary + + printf '%s\n' "$ANTHROPIC_API_KEY" | fabro install --non-interactive \ + --llm-provider anthropic \ + --llm-api-key-stdin \ + --github-strategy gh_cli \ + --github-username brynary + +Hidden non-interactive flags: + --llm-provider + --llm-api-key-stdin + --llm-api-key-env + --github-strategy + --github-username + --overwrite-settings + --keep-existing-settings + --run-doctor + +Notes: + - Only one API-key-based LLM provider is supported in non-interactive mode. + - GitHub CLI is supported in non-interactive mode; GitHub App setup is not."# +} + +#[derive(Debug, Clone)] +struct InstallFacts { + codex_detected: bool, +} + +#[derive(Debug)] +struct LlmInstallSelection { + credentials: Vec, +} + +#[derive(Debug)] +enum GitHubInstallSelection { + GhCli, + App { + owner: GitHubAppOwner, + username: Option, + }, +} + +#[derive(Debug)] +enum ServerConfigSelection { + KeepExisting, + Write { username: String }, +} + +#[async_trait] +trait InstallInputSource { + async fn choose_graphviz_install(&self, dot_missing: bool) -> Result; + + async fn collect_llm_selection( + &self, + facts: &InstallFacts, + s: &Styles, + printer: Printer, + ) -> Result; + + async fn choose_github_install( + &self, + s: &Styles, + printer: Printer, + ) -> Result; + + async fn choose_server_config(&self, config_exists: bool) -> Result; + + async fn should_run_doctor(&self) -> Result; +} + +struct InteractiveInstallInputSource; + +#[async_trait] +impl InstallInputSource for InteractiveInstallInputSource { + async fn choose_graphviz_install(&self, dot_missing: bool) -> Result { + if !dot_missing { + return Ok(false); + } + + spawn_blocking(|| prompt_confirm("Graphviz (dot) not found. Install via Homebrew?", true)) + .await? + } + + async fn collect_llm_selection( + &self, + facts: &InstallFacts, + s: &Styles, + printer: Printer, + ) -> Result { + let mut credentials = Vec::new(); + let mut configured_providers: Vec = Vec::new(); + let mut openai_configured = false; + + if facts.codex_detected { + tracing::debug!("Codex binary detected on PATH"); + let use_device_auth = spawn_blocking(|| { + prompt_confirm( + "OpenAI (Codex) detected. Set up OpenAI with device code login?", + true, + ) + }) + .await??; + + if use_device_auth { + let credential = authenticate_provider_with_method( + Provider::OpenAi, + AuthMethod::CodexDevice(codex_oauth_config()), + s, + printer, + ) + .await?; + credentials.push(credential); + configured_providers.push(Provider::OpenAi); + openai_configured = true; + } + } + + if !openai_configured { + let primary_providers = [Provider::Anthropic, Provider::OpenAi, Provider::Gemini]; + let primary_labels: Vec = primary_providers + .iter() + .map(|p| provider_display_name(*p).to_string()) + .collect(); + let primary_idx: usize = spawn_blocking({ + let labels = primary_labels.clone(); + move || prompt_select("Choose your first LLM provider", &labels) + }) + .await??; + + let first_provider = primary_providers[primary_idx]; + credentials.push(authenticate_provider(first_provider, s, printer).await?); + configured_providers.push(first_provider); + } + + let add_more = + spawn_blocking(|| prompt_confirm("Set up additional LLM providers?", false)).await??; + + if add_more { + let remaining_labels: Vec = Provider::ALL + .iter() + .filter(|p| !configured_providers.contains(p)) + .map(|p| { + let env_vars = p.api_key_env_vars().join(" / "); + format!("{} ({})", provider_display_name(*p), env_vars) + }) + .collect(); + let remaining_providers: Vec = Provider::ALL + .iter() + .filter(|p| !configured_providers.contains(p)) + .copied() + .collect(); + + let selected_indices: Vec = spawn_blocking({ + let labels = remaining_labels.clone(); + move || prompt_multiselect("Which additional LLM providers?", &labels) + }) + .await??; + + for idx in selected_indices { + let provider = remaining_providers[idx]; + credentials.push(authenticate_provider(provider, s, printer).await?); + } + } + + Ok(LlmInstallSelection { credentials }) + } + + async fn choose_github_install( + &self, + s: &Styles, + _printer: Printer, + ) -> Result { + let strategy_options = vec![ + "GitHub CLI — use your existing `gh` login".to_string(), + "GitHub App — recommended for teams".to_string(), + ]; + let strategy = spawn_blocking({ + let options = strategy_options.clone(); + move || prompt_select("How should Fabro authenticate with GitHub?", &options) + }) + .await??; + + match strategy { + 0 => Ok(GitHubInstallSelection::GhCli), + 1 => { + let (owner, username) = prompt_github_app_owner(s).await?; + Ok(GitHubInstallSelection::App { owner, username }) + } + _ => unreachable!("prompt_select returned an out-of-range index"), + } + } + + async fn choose_server_config(&self, config_exists: bool) -> Result { + let write_config = if config_exists { + spawn_blocking(|| { + prompt_confirm("~/.fabro/settings.toml already exists. Overwrite?", false) + }) + .await?? + } else { + true + }; + + if write_config { + let username: String = + spawn_blocking(|| prompt_input("GitHub username for allowed access")).await??; + Ok(ServerConfigSelection::Write { username }) + } else { + Ok(ServerConfigSelection::KeepExisting) + } + } + + async fn should_run_doctor(&self) -> Result { + spawn_blocking(|| prompt_confirm("Run fabro doctor to verify?", true)).await? + } +} + +#[derive(Debug)] +struct NonInteractiveInstallInputSource { + args: InstallNonInteractiveArgs, +} + +impl NonInteractiveInstallInputSource { + fn new(args: &InstallArgs) -> Result> { + if !args.non_interactive { + if let Some(flag) = args.scripted.first_flag_name() { + bail!("{flag} requires --non-interactive"); + } + return Ok(None); + } + + if !args.scripted.has_any() { + bail!("{}", non_interactive_install_usage()); + } + + anyhow::ensure!( + args.scripted.llm_api_key_stdin ^ args.scripted.llm_api_key_env.is_some(), + "non-interactive install requires exactly one of --llm-api-key-stdin or --llm-api-key-env" + ); + anyhow::ensure!( + !(args.scripted.overwrite_settings && args.scripted.keep_existing_settings), + "--overwrite-settings and --keep-existing-settings cannot be used together" + ); + + Ok(Some(Self { + args: args.scripted.clone(), + })) + } + + fn validate(&self, config_exists: bool) -> Result<()> { + anyhow::ensure!( + self.args.llm_provider.is_some(), + "non-interactive install requires --llm-provider" + ); + + match self.args.github_strategy { + Some(InstallGitHubStrategyArg::GhCli) => {} + Some(InstallGitHubStrategyArg::App) => { + bail!("GitHub App setup is not supported with --non-interactive") + } + None => bail!("non-interactive install requires --github-strategy"), + } + + if config_exists { + anyhow::ensure!( + self.args.keep_existing_settings || self.args.overwrite_settings, + "settings.toml already exists; pass --overwrite-settings or --keep-existing-settings" + ); + + if self.args.keep_existing_settings { + return Ok(()); + } + } + + anyhow::ensure!( + self.args.github_username.is_some(), + "non-interactive install requires --github-username" + ); + + Ok(()) + } + + fn api_key_source(&self) -> Result { + if self.args.llm_api_key_stdin { + Ok(ApiKeySource::Stdin) + } else if let Some(name) = &self.args.llm_api_key_env { + Ok(ApiKeySource::EnvVar(name.clone())) + } else { + bail!( + "non-interactive install requires exactly one of --llm-api-key-stdin or --llm-api-key-env" + ) + } + } +} + +#[async_trait] +impl InstallInputSource for NonInteractiveInstallInputSource { + async fn choose_graphviz_install(&self, _dot_missing: bool) -> Result { + Ok(false) + } + + async fn collect_llm_selection( + &self, + _facts: &InstallFacts, + s: &Styles, + printer: Printer, + ) -> Result { + let provider = self + .args + .llm_provider + .context("non-interactive install requires --llm-provider")?; + let credential = + authenticate_provider_with_api_key_source(provider, self.api_key_source()?, s, printer) + .await?; + Ok(LlmInstallSelection { + credentials: vec![credential], + }) + } + + async fn choose_github_install( + &self, + _s: &Styles, + _printer: Printer, + ) -> Result { + match self.args.github_strategy { + Some(InstallGitHubStrategyArg::GhCli) => Ok(GitHubInstallSelection::GhCli), + Some(InstallGitHubStrategyArg::App) => { + bail!("GitHub App setup is not supported with --non-interactive") + } + None => bail!("non-interactive install requires --github-strategy"), + } + } + + async fn choose_server_config(&self, config_exists: bool) -> Result { + if config_exists { + if self.args.keep_existing_settings { + return Ok(ServerConfigSelection::KeepExisting); + } + anyhow::ensure!( + self.args.overwrite_settings, + "settings.toml already exists; pass --overwrite-settings or --keep-existing-settings" + ); + } + + let username = self + .args + .github_username + .clone() + .context("non-interactive install requires --github-username")?; + Ok(ServerConfigSelection::Write { username }) + } + + async fn should_run_doctor(&self) -> Result { + Ok(self.args.run_doctor) + } +} + // --------------------------------------------------------------------------- // GitHub App owner selection // --------------------------------------------------------------------------- +#[derive(Debug)] enum GitHubAppOwner { Personal, Organization(String), @@ -694,61 +1097,66 @@ async fn setup_github_app( Ok(env_pairs) } -async fn persist_vault_secrets( +async fn persist_vault_secrets_via_server( + client: &fabro_api::Client, + secrets: &[CreateSecretRequest], +) -> Result<()> { + for secret in secrets { + client + .create_secret() + .body(CreateSecretRequest { + name: secret.name.clone(), + value: secret.value.clone(), + type_: secret.type_, + description: secret.description.clone(), + }) + .send() + .await?; + } + + Ok(()) +} + +async fn persist_vault_secrets_with( storage_dir: &Path, secrets: &[CreateSecretRequest], server_was_running: bool, + connect_api_client: impl for<'a> Fn(&'a Path) -> BoxFuture<'a, Result>, + stop_server: impl for<'a> Fn(&'a Path, Duration) -> BoxFuture<'a, bool>, ) -> Result<()> { if secrets.is_empty() { return Ok(()); } - if server_was_running { - let client = server_client::connect_api_client(storage_dir).await?; - for secret in secrets { - client - .create_secret() - .body(CreateSecretRequest { - name: secret.name.clone(), - value: secret.value.clone(), - type_: secret.type_, - description: secret.description.clone(), - }) - .send() - .await?; + let client = match connect_api_client(storage_dir).await { + Ok(client) => client, + Err(err) => { + if !server_was_running { + stop_server(storage_dir, Duration::from_secs(5)).await; + } + return Err(err); } - return Ok(()); + }; + let result = persist_vault_secrets_via_server(&client, secrets).await; + if !server_was_running { + stop_server(storage_dir, Duration::from_secs(5)).await; } - - let mut store = Vault::load(Storage::new(storage_dir).secrets_path())?; - for secret in secrets { - validate_vault_secret(secret)?; - store.set( - &secret.name, - &secret.value, - local_secret_type(secret.type_), - secret.description.as_deref(), - )?; - } - Ok(()) + result } -fn local_secret_type(secret_type: ApiSecretType) -> SecretType { - match secret_type { - ApiSecretType::Environment => SecretType::Environment, - ApiSecretType::File => SecretType::File, - ApiSecretType::Credential => SecretType::Credential, - } -} - -fn validate_vault_secret(secret: &CreateSecretRequest) -> Result<()> { - if secret.type_ != ApiSecretType::Credential { - return Ok(()); - } - - parse_credential_secret(&secret.name, &secret.value) - .map(|_| ()) - .map_err(anyhow::Error::msg) +async fn persist_vault_secrets( + storage_dir: &Path, + secrets: &[CreateSecretRequest], + server_was_running: bool, +) -> Result<()> { + persist_vault_secrets_with( + storage_dir, + secrets, + server_was_running, + |path| Box::pin(server_client::connect_api_client(path)), + |path, timeout| Box::pin(stop::stop_server(path, timeout)), + ) + .await } fn credential_secret_request(credential: &AuthCredential) -> Result { @@ -794,6 +1202,16 @@ pub(crate) async fn run_install( let cli_settings = user_config::load_settings_with_storage_dir(args.storage_dir.as_deref())?; let storage_dir = user_config::storage_dir(&cli_settings)?; let server_was_running = record::active_server_record(&storage_dir).is_some(); + let fabro_dir = fabro_util::Home::from_env().root().to_path_buf(); + let config_path = fabro_dir.join(SETTINGS_CONFIG_FILENAME); + let input_source: Box = + match NonInteractiveInstallInputSource::new(args)? { + Some(source) => { + source.validate(config_path.exists())?; + Box::new(source) + } + None => Box::new(InteractiveInstallInputSource), + }; fabro_util::printerr!(printer, ""); fabro_util::printerr!(printer, " {}{}", emoji, s.bold.apply_to("Fabro Install")); @@ -811,7 +1229,6 @@ pub(crate) async fn run_install( ); fabro_util::printerr!(printer, ""); - let fabro_dir = fabro_util::Home::from_env().root().to_path_buf(); std::fs::create_dir_all(&fabro_dir)?; { @@ -827,14 +1244,19 @@ pub(crate) async fn run_install( } // Pre-flight checks - { + let facts = { + let dep_outcomes = doctor::probe_system_deps().await; + let dep_check = doctor::check_system_deps(doctor::DEP_SPECS, &dep_outcomes); + let dot_missing = doctor::DEP_SPECS + .iter() + .position(|spec| spec.name == "dot") + .is_some_and(|idx| matches!(dep_outcomes[idx], doctor::ProbeOutcome::NotFound)); + fabro_util::printerr!( printer, " {}", s.dim.apply_to("[Pre-flight] System dependency checks") ); - let dep_outcomes = doctor::probe_system_deps().await; - let dep_check = doctor::check_system_deps(doctor::DEP_SPECS, &dep_outcomes); if dep_check.status == doctor::CheckStatus::Error { fabro_util::printerr!(printer, " Missing required system dependencies:"); @@ -844,25 +1266,14 @@ pub(crate) async fn run_install( bail!("Install missing required tools before running setup"); } - // Check if dot is missing and offer to install - let dot_idx = doctor::DEP_SPECS.iter().position(|s| s.name == "dot"); - if let Some(idx) = dot_idx { - if matches!(dep_outcomes[idx], doctor::ProbeOutcome::NotFound) { - let install = spawn_blocking(|| { - prompt_confirm("Graphviz (dot) not found. Install via Homebrew?", true) - }) - .await??; - - if install { - let status = TokioCommand::new("brew") - .args(["install", "graphviz"]) - .status() - .await - .context("failed to run brew install graphviz")?; - if !status.success() { - fabro_util::printerr!(printer, " Warning: brew install graphviz failed"); - } - } + if input_source.choose_graphviz_install(dot_missing).await? { + let status = TokioCommand::new("brew") + .args(["install", "graphviz"]) + .status() + .await + .context("failed to run brew install graphviz")?; + if !status.success() { + fabro_util::printerr!(printer, " Warning: brew install graphviz failed"); } } @@ -870,7 +1281,11 @@ pub(crate) async fn run_install( fabro_util::printerr!(printer, " {}", detail.text); } fabro_util::printerr!(printer, ""); - } + + InstallFacts { + codex_detected: detect_binary_on_path("codex").await, + } + }; // Step 1: LLM Providers fabro_util::printerr!(printer, " {}", s.bold.apply_to("Step 1 · LLM Providers")); @@ -879,88 +1294,11 @@ pub(crate) async fn run_install( let mut vault_secrets: Vec = Vec::new(); let mut server_env_pairs: Vec<(String, String)> = Vec::new(); - let mut configured_providers: Vec = Vec::new(); - - let codex_detected = detect_binary_on_path("codex").await; - let mut openai_configured = false; - - if codex_detected { - tracing::debug!("Codex binary detected on PATH"); - let use_device_auth = spawn_blocking(|| { - prompt_confirm( - "OpenAI (Codex) detected. Set up OpenAI with device code login?", - true, - ) - }) - .await??; - - if use_device_auth { - let credential = authenticate_provider_with_method( - Provider::OpenAi, - AuthMethod::CodexDevice(codex_oauth_config()), - &s, - printer, - ) - .await?; - vault_secrets.push(credential_secret_request(&credential)?); - configured_providers.push(Provider::OpenAi); - openai_configured = true; - } - } - - if !openai_configured { - // First provider — single choice from the top 3 - let primary_providers = [Provider::Anthropic, Provider::OpenAi, Provider::Gemini]; - let primary_labels: Vec = primary_providers - .iter() - .map(|p| provider_display_name(*p).to_string()) - .collect(); - - let primary_idx: usize = spawn_blocking({ - let labels = primary_labels.clone(); - move || prompt_select("Choose your first LLM provider", &labels) - }) - .await??; - - let first_provider = primary_providers[primary_idx]; - { - let credential = authenticate_provider(first_provider, &s, printer).await?; - vault_secrets.push(credential_secret_request(&credential)?); - configured_providers.push(first_provider); - } - } - - // Additional providers - fabro_util::printerr!(printer, ""); - let add_more = - spawn_blocking(|| prompt_confirm("Set up additional LLM providers?", false)).await??; - - if add_more { - let remaining_labels: Vec = Provider::ALL - .iter() - .filter(|p| !configured_providers.contains(p)) - .map(|p| { - let env_vars = p.api_key_env_vars().join(" / "); - format!("{} ({})", provider_display_name(*p), env_vars) - }) - .collect(); - let remaining_providers: Vec = Provider::ALL - .iter() - .filter(|p| !configured_providers.contains(p)) - .copied() - .collect(); - - let selected_indices: Vec = spawn_blocking({ - let labels = remaining_labels.clone(); - move || prompt_multiselect("Which additional LLM providers?", &labels) - }) - .await??; - - for idx in selected_indices { - let provider = remaining_providers[idx]; - let credential = authenticate_provider(provider, &s, printer).await?; - vault_secrets.push(credential_secret_request(&credential)?); - } + let llm_selection = input_source + .collect_llm_selection(&facts, &s, printer) + .await?; + for credential in llm_selection.credentials { + vault_secrets.push(credential_secret_request(&credential)?); } fabro_util::printerr!(printer, ""); @@ -969,72 +1307,58 @@ pub(crate) async fn run_install( fabro_util::printerr!(printer, " {}", s.dim.apply_to("───────────────")); fabro_util::printerr!(printer, ""); - { - let strategy_options = vec![ - "GitHub CLI — use your existing `gh` login".to_string(), - "GitHub App — recommended for teams".to_string(), - ]; - let strategy = spawn_blocking({ - let options = strategy_options.clone(); - move || prompt_select("How should Fabro authenticate with GitHub?", &options) - }) - .await??; - - match strategy { - 0 => { - let token = fabro_github::gh_auth_token().await.map_err(|err| { - anyhow!("{err}. Run `gh auth login` and rerun `fabro install`.") - })?; + match input_source.choose_github_install(&s, printer).await? { + GitHubInstallSelection::GhCli => { + let token = fabro_github::gh_auth_token() + .await + .map_err(|err| anyhow!("{err}. Run `gh auth login` and rerun `fabro install`."))?; + let user_toml_path = fabro_dir.join(SETTINGS_CONFIG_FILENAME); + let existing = std::fs::read_to_string(&user_toml_path).unwrap_or_default(); + let mut doc: toml::Value = if existing.is_empty() { + toml::Value::Table(toml::Table::default()) + } else { + toml::from_str(&existing).context("failed to parse existing settings.toml")? + }; + write_github_cli_settings(&mut doc)?; + std::fs::write(&user_toml_path, toml::to_string_pretty(&doc)?)?; + fabro_util::printerr!(printer, " {} GitHub CLI configured", s.green.apply_to("✔")); + vault_secrets.push(CreateSecretRequest { + name: "GITHUB_CLI_TOKEN".to_string(), + value: token, + type_: ApiSecretType::Environment, + description: None, + }); + } + GitHubInstallSelection::App { owner, username } => { + let github_env_pairs = setup_github_app( + &fabro_dir, + &s, + web_url, + &owner, + username.as_deref(), + printer, + ) + .await?; + let slug = { let user_toml_path = fabro_dir.join(SETTINGS_CONFIG_FILENAME); - let existing = std::fs::read_to_string(&user_toml_path).unwrap_or_default(); - let mut doc: toml::Value = if existing.is_empty() { - toml::Value::Table(toml::Table::default()) - } else { - toml::from_str(&existing).context("failed to parse existing settings.toml")? - }; - write_github_cli_settings(&mut doc)?; - std::fs::write(&user_toml_path, toml::to_string_pretty(&doc)?)?; - fabro_util::printerr!(printer, " {} GitHub CLI configured", s.green.apply_to("✔")); - vault_secrets.push(CreateSecretRequest { - name: "GITHUB_CLI_TOKEN".to_string(), - value: token, - type_: ApiSecretType::Environment, - description: None, - }); - } - 1 => { - let (owner, username) = prompt_github_app_owner(&s).await?; - let github_env_pairs = setup_github_app( - &fabro_dir, - &s, - web_url, - &owner, - username.as_deref(), - printer, - ) - .await?; - let slug = { - let user_toml_path = fabro_dir.join(SETTINGS_CONFIG_FILENAME); - let toml_content = std::fs::read_to_string(&user_toml_path).unwrap_or_default(); - let doc: toml::Value = toml::from_str(&toml_content) - .unwrap_or(toml::Value::Table(toml::Table::default())); - doc.get("server") - .and_then(|server| server.get("integrations")) - .and_then(|integrations| integrations.get("github")) - .and_then(|github| github.get("slug")) - .and_then(|slug| slug.as_str()) - .unwrap_or("unknown") - .to_string() - }; - fabro_util::printerr!( - printer, - " {} GitHub App registered ({})", - s.green.apply_to("✔"), - slug - ); - server_env_pairs.extend(github_env_pairs); - } - _ => unreachable!("prompt_select returned an out-of-range index"), + let toml_content = std::fs::read_to_string(&user_toml_path).unwrap_or_default(); + let doc: toml::Value = toml::from_str(&toml_content) + .unwrap_or(toml::Value::Table(toml::Table::default())); + doc.get("server") + .and_then(|server| server.get("integrations")) + .and_then(|integrations| integrations.get("github")) + .and_then(|github| github.get("slug")) + .and_then(|slug| slug.as_str()) + .unwrap_or("unknown") + .to_string() + }; + fabro_util::printerr!( + printer, + " {} GitHub App registered ({})", + s.green.apply_to("✔"), + slug + ); + server_env_pairs.extend(github_env_pairs); } } fabro_util::printerr!(printer, ""); @@ -1045,39 +1369,32 @@ pub(crate) async fn run_install( fabro_util::printerr!(printer, " {}", s.dim.apply_to("─────────────────────")); fabro_util::printerr!(printer, ""); - let config_path = fabro_dir.join(SETTINGS_CONFIG_FILENAME); - let write_config = if config_path.exists() { - spawn_blocking(|| { - prompt_confirm("~/.fabro/settings.toml already exists. Overwrite?", false) - }) - .await?? - } else { - true - }; - - if write_config { - let username: String = - spawn_blocking(|| prompt_input("GitHub username for allowed access")).await??; - - let existing = std::fs::read_to_string(&config_path).unwrap_or_default(); - let mut doc: toml::Value = if existing.is_empty() { - toml::Value::Table(toml::Table::default()) - } else { - toml::from_str(&existing).context("failed to parse existing settings.toml")? - }; - merge_server_settings(&mut doc, &username)?; - std::fs::write(&config_path, toml::to_string_pretty(&doc)?)?; - fabro_util::printerr!( - printer, - " {}", - s.dim.apply_to(format!("Wrote {}", config_path.display())) - ); - } else { - fabro_util::printerr!( - printer, - " {}", - s.dim.apply_to("Keeping existing settings.toml") - ); + match input_source + .choose_server_config(config_path.exists()) + .await? + { + ServerConfigSelection::KeepExisting => { + fabro_util::printerr!( + printer, + " {}", + s.dim.apply_to("Keeping existing settings.toml") + ); + } + ServerConfigSelection::Write { username } => { + let existing = std::fs::read_to_string(&config_path).unwrap_or_default(); + let mut doc: toml::Value = if existing.is_empty() { + toml::Value::Table(toml::Table::default()) + } else { + toml::from_str(&existing).context("failed to parse existing settings.toml")? + }; + merge_server_settings(&mut doc, &username)?; + std::fs::write(&config_path, toml::to_string_pretty(&doc)?)?; + fabro_util::printerr!( + printer, + " {}", + s.dim.apply_to(format!("Wrote {}", config_path.display())) + ); + } } fabro_util::printerr!(printer, ""); } @@ -1162,8 +1479,7 @@ pub(crate) async fn run_install( fabro_util::printerr!(printer, ""); // Verify setup - let run_doctor = - spawn_blocking(|| prompt_confirm("Run fabro doctor to verify?", true)).await??; + let run_doctor = input_source.should_run_doctor().await?; if run_doctor { fabro_util::printerr!(printer, ""); @@ -1207,8 +1523,23 @@ mod hex { mod tests { #![allow(clippy::absolute_paths)] + use std::sync::Arc; + use std::sync::atomic::{AtomicBool, Ordering}; + + use httpmock::Method::POST; + use httpmock::MockServer; + use super::*; + fn install_args(non_interactive: bool, scripted: InstallNonInteractiveArgs) -> InstallArgs { + InstallArgs { + storage_dir: crate::args::StorageDirArgs::default(), + web_url: "http://localhost:3000".to_string(), + non_interactive, + scripted, + } + } + // -- Binary detection -- #[tokio::test] @@ -1571,7 +1902,7 @@ client_id = "client-id" } #[tokio::test] - async fn persist_install_outputs_offline_splits_server_env_and_vault() { + async fn persist_install_outputs_persists_vault_secrets_via_server_when_autostarting() { let dir = tempfile::tempdir().unwrap(); let server_env_pairs = vec![ ("SESSION_SECRET".to_string(), "session".to_string()), @@ -1592,20 +1923,200 @@ client_id = "client-id" }) .unwrap(), ]; + let server = MockServer::start_async().await; + let created = server + .mock_async(|when, then| { + when.method(POST).path("/api/v1/secrets"); + then.status(200) + .header("content-type", "application/json") + .body( + serde_json::json!({ + "name": "persisted", + "type": "environment", + "created_at": "2026-01-01T00:00:00Z", + "updated_at": "2026-01-01T00:00:00Z" + }) + .to_string(), + ); + }) + .await; + let stop_called = Arc::new(AtomicBool::new(false)); - persist_install_outputs(dir.path(), &server_env_pairs, &vault_secrets, false) - .await - .unwrap(); + persist_server_env_secrets(dir.path(), &server_env_pairs).unwrap(); + persist_vault_secrets_with( + dir.path(), + &vault_secrets, + false, + |_| { + let client = fabro_api::Client::new(&server.base_url()); + Box::pin(async move { Ok(client) }) + }, + { + let stop_called = Arc::clone(&stop_called); + move |_, _| { + let stop_called = Arc::clone(&stop_called); + Box::pin(async move { + stop_called.store(true, Ordering::SeqCst); + true + }) + } + }, + ) + .await + .unwrap(); let server_env = std::fs::read_to_string(Storage::new(dir.path()).server_state().env_path()).unwrap(); assert!(server_env.contains("SESSION_SECRET=session")); assert!(server_env.contains("FABRO_JWT_PUBLIC_KEY=public-key")); + assert_eq!(created.calls_async().await, 2); + assert!(stop_called.load(Ordering::SeqCst)); + assert!(!Storage::new(dir.path()).secrets_path().exists()); + } - let vault = Vault::load(Storage::new(dir.path()).secrets_path()).unwrap(); - assert_eq!(vault.get("GITHUB_CLI_TOKEN"), Some("gh-token")); - let anthropic = vault.get_entry("anthropic").unwrap(); - assert_eq!(anthropic.secret_type, SecretType::Credential); - assert_eq!(vault.get("SESSION_SECRET"), None); + #[test] + fn non_interactive_source_rejects_missing_scripted_inputs() { + let args = install_args(true, InstallNonInteractiveArgs::default()); + let err = NonInteractiveInstallInputSource::new(&args).unwrap_err(); + assert!( + err.to_string() + .contains("Non-interactive install requires additional flags") + ); + } + + #[test] + fn non_interactive_source_rejects_hidden_args_without_switch() { + let args = install_args(false, InstallNonInteractiveArgs { + llm_provider: Some(Provider::Anthropic), + ..InstallNonInteractiveArgs::default() + }); + let err = NonInteractiveInstallInputSource::new(&args).unwrap_err(); + assert!( + err.to_string() + .contains("--llm-provider requires --non-interactive") + ); + } + + #[test] + fn non_interactive_source_rejects_conflicting_api_key_inputs() { + let args = install_args(true, InstallNonInteractiveArgs { + llm_provider: Some(Provider::Anthropic), + llm_api_key_stdin: true, + llm_api_key_env: Some("ANTHROPIC_API_KEY".to_string()), + github_strategy: Some(InstallGitHubStrategyArg::GhCli), + github_username: Some("brynary".to_string()), + ..InstallNonInteractiveArgs::default() + }); + let err = NonInteractiveInstallInputSource::new(&args).unwrap_err(); + assert!( + err.to_string() + .contains("requires exactly one of --llm-api-key-stdin or --llm-api-key-env") + ); + } + + #[test] + fn non_interactive_source_rejects_missing_llm_provider() { + let source = NonInteractiveInstallInputSource { + args: InstallNonInteractiveArgs { + llm_api_key_env: Some("ANTHROPIC_API_KEY".to_string()), + github_strategy: Some(InstallGitHubStrategyArg::GhCli), + github_username: Some("brynary".to_string()), + ..InstallNonInteractiveArgs::default() + }, + }; + + let err = source.validate(false).unwrap_err(); + assert!( + err.to_string() + .contains("non-interactive install requires --llm-provider") + ); + } + + #[test] + fn non_interactive_source_rejects_missing_github_strategy() { + let source = NonInteractiveInstallInputSource { + args: InstallNonInteractiveArgs { + llm_provider: Some(Provider::Anthropic), + llm_api_key_env: Some("ANTHROPIC_API_KEY".to_string()), + github_username: Some("brynary".to_string()), + ..InstallNonInteractiveArgs::default() + }, + }; + + let err = source.validate(false).unwrap_err(); + assert!( + err.to_string() + .contains("non-interactive install requires --github-strategy") + ); + } + + #[test] + fn non_interactive_source_rejects_missing_github_username_for_new_config() { + let source = NonInteractiveInstallInputSource { + args: InstallNonInteractiveArgs { + llm_provider: Some(Provider::Anthropic), + llm_api_key_env: Some("ANTHROPIC_API_KEY".to_string()), + github_strategy: Some(InstallGitHubStrategyArg::GhCli), + ..InstallNonInteractiveArgs::default() + }, + }; + + let err = source.validate(false).unwrap_err(); + assert!( + err.to_string() + .contains("non-interactive install requires --github-username") + ); + } + + #[test] + fn non_interactive_source_allows_keep_existing_settings_without_username() { + let source = NonInteractiveInstallInputSource { + args: InstallNonInteractiveArgs { + llm_provider: Some(Provider::Anthropic), + llm_api_key_env: Some("ANTHROPIC_API_KEY".to_string()), + github_strategy: Some(InstallGitHubStrategyArg::GhCli), + keep_existing_settings: true, + ..InstallNonInteractiveArgs::default() + }, + }; + + source.validate(true).unwrap(); + } + + #[tokio::test] + async fn non_interactive_source_rejects_github_app_setup() { + let source = NonInteractiveInstallInputSource { + args: InstallNonInteractiveArgs { + llm_provider: Some(Provider::Anthropic), + llm_api_key_env: Some("ANTHROPIC_API_KEY".to_string()), + github_strategy: Some(InstallGitHubStrategyArg::App), + github_username: Some("brynary".to_string()), + ..InstallNonInteractiveArgs::default() + }, + }; + + let err = source.validate(false).unwrap_err(); + assert!( + err.to_string() + .contains("GitHub App setup is not supported with --non-interactive") + ); + } + + #[tokio::test] + async fn non_interactive_source_requires_config_choice_when_settings_exist() { + let source = NonInteractiveInstallInputSource { + args: InstallNonInteractiveArgs { + llm_provider: Some(Provider::Anthropic), + llm_api_key_env: Some("ANTHROPIC_API_KEY".to_string()), + github_strategy: Some(InstallGitHubStrategyArg::GhCli), + github_username: Some("brynary".to_string()), + ..InstallNonInteractiveArgs::default() + }, + }; + + let err = source.choose_server_config(true).await.unwrap_err(); + assert!(err.to_string().contains( + "settings.toml already exists; pass --overwrite-settings or --keep-existing-settings" + )); } } diff --git a/lib/crates/fabro-cli/src/commands/provider/login.rs b/lib/crates/fabro-cli/src/commands/provider/login.rs index 8af8d4cf6..39c42fd29 100644 --- a/lib/crates/fabro-cli/src/commands/provider/login.rs +++ b/lib/crates/fabro-cli/src/commands/provider/login.rs @@ -18,7 +18,17 @@ pub(super) async fn login_command( let s = Styles::detect_stderr(); let ctx = CommandContext::for_target(&args.target, printer)?; let server = ctx.server().await?; - let credential = provider_auth::authenticate_provider(args.provider, &s, printer).await?; + let credential = if args.api_key_stdin { + provider_auth::authenticate_provider_with_api_key_source( + args.provider, + provider_auth::ApiKeySource::Stdin, + &s, + printer, + ) + .await? + } else { + provider_auth::authenticate_provider(args.provider, &s, printer).await? + }; let credential_id = credential_id_for(&credential).map_err(anyhow::Error::msg)?; let value = serde_json::to_string(&credential)?; diff --git a/lib/crates/fabro-cli/src/commands/server/stop.rs b/lib/crates/fabro-cli/src/commands/server/stop.rs index 2e53337ff..b0460db03 100644 --- a/lib/crates/fabro-cli/src/commands/server/stop.rs +++ b/lib/crates/fabro-cli/src/commands/server/stop.rs @@ -7,10 +7,9 @@ use tokio::time; use super::record; -pub(crate) async fn execute(storage_dir: &Path, timeout: Duration, printer: Printer) { +pub(crate) async fn stop_server(storage_dir: &Path, timeout: Duration) -> bool { let Some(active) = record::active_server_record_details(storage_dir) else { - fabro_util::printerr!(printer, "Server is not running"); - std::process::exit(1); + return false; }; let record = active.record; @@ -37,5 +36,14 @@ pub(crate) async fn execute(storage_dir: &Path, timeout: Duration, printer: Prin let _ = std::fs::remove_file(path); } + true +} + +pub(crate) async fn execute(storage_dir: &Path, timeout: Duration, printer: Printer) { + if !stop_server(storage_dir, timeout).await { + fabro_util::printerr!(printer, "Server is not running"); + std::process::exit(1); + } + fabro_util::printerr!(printer, "Server stopped"); } diff --git a/lib/crates/fabro-cli/src/main.rs b/lib/crates/fabro-cli/src/main.rs index 3021038e1..2f1489756 100644 --- a/lib/crates/fabro-cli/src/main.rs +++ b/lib/crates/fabro-cli/src/main.rs @@ -356,6 +356,28 @@ mod tests { } } + #[test] + fn parse_provider_login_api_key_stdin() { + let cli = Cli::try_parse_from([ + "fabro", + "provider", + "login", + "--provider", + "anthropic", + "--api-key-stdin", + ]) + .expect("should parse"); + match *cli.command { + Commands::Provider(ProviderNamespace { + command: ProviderCommand::Login(args), + }) => { + assert_eq!(args.provider, fabro_model::Provider::Anthropic); + assert!(args.api_key_stdin); + } + _ => panic!("unexpected command variant"), + } + } + #[test] fn parse_provider_login_missing_provider_flag() { let result = Cli::try_parse_from(["fabro", "provider", "login"]); diff --git a/lib/crates/fabro-cli/src/shared/provider_auth.rs b/lib/crates/fabro-cli/src/shared/provider_auth.rs index e9d832743..0f5843cf6 100644 --- a/lib/crates/fabro-cli/src/shared/provider_auth.rs +++ b/lib/crates/fabro-cli/src/shared/provider_auth.rs @@ -1,6 +1,7 @@ +use std::io::Read; use std::sync::Arc; -use anyhow::Result; +use anyhow::{Context, Result, anyhow}; use dialoguer::console::Term; use dialoguer::theme::ColorfulTheme; use dialoguer::{Confirm, Password}; @@ -56,6 +57,13 @@ pub(crate) fn prompt_password(prompt: &str) -> Result { .interact_on(&Term::stderr())?) } +#[derive(Debug, Clone)] +pub(crate) enum ApiKeySource { + Prompt, + Stdin, + EnvVar(String), +} + // --------------------------------------------------------------------------- // API key validation // --------------------------------------------------------------------------- @@ -99,36 +107,66 @@ pub(crate) async fn validate_api_key(provider: Provider, api_key: &str) -> Resul .map_err(|e| e.to_string()) } -pub(crate) async fn prompt_and_validate_key( +fn normalize_api_key_input(raw: &str) -> Result { + let key = raw.trim_end_matches(['\r', '\n']).to_string(); + anyhow::ensure!(!key.is_empty(), "API key input is empty"); + Ok(key) +} + +fn read_api_key_from_stdin() -> Result { + let mut raw = String::new(); + std::io::stdin() + .read_to_string(&mut raw) + .context("failed to read API key from stdin")?; + normalize_api_key_input(&raw) +} + +fn read_api_key_from_env_var(name: &str) -> Result { + let value = + std::env::var(name).with_context(|| format!("environment variable {name} is not set"))?; + normalize_api_key_input(&value) + .with_context(|| format!("environment variable {name} did not contain an API key")) +} + +async fn read_api_key_from_source(source: &ApiKeySource, prompt: &str) -> Result { + match source { + ApiKeySource::Prompt => { + let prompt = prompt.to_string(); + let key: String = spawn_blocking(move || prompt_password(&prompt)).await??; + Ok(key) + } + ApiKeySource::Stdin => spawn_blocking(read_api_key_from_stdin).await?, + ApiKeySource::EnvVar(name) => read_api_key_from_env_var(name), + } +} + +async fn read_and_validate_api_key( provider: Provider, + source: &ApiKeySource, + env_var: &str, s: &Styles, printer: Printer, -) -> Result<(String, String)> { - let env_var = provider.api_key_env_vars()[0]; - let url = provider_key_url(provider); - fabro_util::printerr!( - printer, - " {}", - s.dim.apply_to(format!("Get your API key at: {url}")) - ); - +) -> Result { loop { - let prompt = env_var.to_string(); - let key: String = spawn_blocking(move || prompt_password(&prompt)).await??; + let key = read_api_key_from_source(source, env_var).await?; fabro_util::printerr!(printer, " {}", s.dim.apply_to("Validating API key...")); match validate_api_key(provider, &key).await { Ok(()) => { fabro_util::printerr!(printer, " {} API key is valid", s.green.apply_to("✔")); - return Ok((env_var.to_string(), key)); + return Ok(key); } Err(e) => { fabro_util::printerr!(printer, " [error] API key validation failed: {e}"); - let retry = - spawn_blocking(|| prompt_confirm("Try again with a different key?", true)) - .await??; - if !retry { - return Ok((env_var.to_string(), key)); + if matches!(source, ApiKeySource::Prompt) { + let retry = + spawn_blocking(|| prompt_confirm("Try again with a different key?", true)) + .await??; + if !retry { + return Ok(key); + } + } else { + return Err(anyhow!("API key validation failed: {e}")); } } } @@ -159,6 +197,19 @@ pub(crate) async fn authenticate_provider( authenticate_provider_with_method(provider, method, s, printer).await } +pub(crate) async fn authenticate_provider_with_api_key_source( + provider: Provider, + source: ApiKeySource, + s: &Styles, + printer: Printer, +) -> Result { + let mut strategy = strategy_for(provider, AuthMethod::ApiKey); + let request = strategy.init().await?; + present_to_user(&request, s, printer); + let response = await_user_response_from_source(&request, &source, s, printer).await?; + strategy.complete(response).await +} + pub(crate) async fn authenticate_provider_with_method( provider: Provider, method: AuthMethod, @@ -168,7 +219,8 @@ pub(crate) async fn authenticate_provider_with_method( let mut strategy = strategy_for(provider, method); let request = strategy.init().await?; present_to_user(&request, s, printer); - let response = await_user_response(&request, s, printer).await?; + let response = + await_user_response_from_source(&request, &ApiKeySource::Prompt, s, printer).await?; strategy.complete(response).await } @@ -210,17 +262,26 @@ pub(crate) fn present_to_user(request: &AuthContextRequest, s: &Styles, printer: } } -pub(crate) async fn await_user_response( +async fn await_user_response_from_source( request: &AuthContextRequest, + source: &ApiKeySource, s: &Styles, printer: Printer, ) -> Result { match request { - AuthContextRequest::ApiKey { provider, .. } => { - let (_, key) = prompt_and_validate_key(*provider, s, printer).await?; + AuthContextRequest::ApiKey { + provider, + env_var_names, + } => { + let env_var = env_var_names.first().map_or("API_KEY", String::as_str); + let key = read_and_validate_api_key(*provider, source, env_var, s, printer).await?; Ok(AuthContextResponse::ApiKey { key }) } AuthContextRequest::DeviceCode { .. } => { + anyhow::ensure!( + matches!(source, ApiKeySource::Prompt), + "device code login is not supported for scripted API key input" + ); let ready = spawn_blocking(|| { prompt_confirm("Continue after completing sign-in in the browser?", true) }) @@ -259,4 +320,16 @@ mod tests { let result = validate_api_key(Provider::Anthropic, "sk-invalid-key-12345").await; assert!(result.is_err(), "expected invalid key to be rejected"); } + + #[test] + fn normalize_api_key_input_trims_trailing_newlines() { + let key = normalize_api_key_input("secret-key\r\n").unwrap(); + assert_eq!(key, "secret-key"); + } + + #[test] + fn normalize_api_key_input_rejects_empty_input() { + let err = normalize_api_key_input("\n").unwrap_err(); + assert!(err.to_string().contains("API key input is empty")); + } } diff --git a/lib/crates/fabro-cli/tests/it/cmd/install.rs b/lib/crates/fabro-cli/tests/it/cmd/install.rs index 78ef9db0a..ba26f1b73 100644 --- a/lib/crates/fabro-cli/tests/it/cmd/install.rs +++ b/lib/crates/fabro-cli/tests/it/cmd/install.rs @@ -19,6 +19,7 @@ fn help() { --debug Enable DEBUG-level logging (default is INFO) [env: FABRO_DEBUG=] --web-url Base URL for the web UI (used for OAuth callback URLs) [default: http://localhost:3000] --no-upgrade-check Disable automatic upgrade check [env: FABRO_NO_UPGRADE_CHECK=true] + --non-interactive Run install without prompts; use hidden scripted flags for inputs --quiet Suppress non-essential output [env: FABRO_QUIET=] --verbose Enable verbose output [env: FABRO_VERBOSE=] -h, --help Print help @@ -39,3 +40,33 @@ fn install_rejects_json() { let stderr = String::from_utf8(output.stderr).unwrap(); assert!(stderr.contains("--json is not supported for this command")); } + +#[test] +fn non_interactive_without_inputs_prints_scripted_usage_and_fails() { + let context = test_context!(); + let output = context + .command() + .args(["install", "--non-interactive"]) + .output() + .expect("command should run"); + + assert!(!output.status.success()); + let stderr = String::from_utf8(output.stderr).unwrap(); + assert!(stderr.contains("Non-interactive install requires additional flags")); + assert!(stderr.contains("--llm-provider")); + assert!(stderr.contains("--github-strategy")); +} + +#[test] +fn hidden_non_interactive_args_require_non_interactive() { + let context = test_context!(); + let output = context + .command() + .args(["install", "--llm-provider", "anthropic"]) + .output() + .expect("command should run"); + + assert!(!output.status.success()); + let stderr = String::from_utf8(output.stderr).unwrap(); + assert!(stderr.contains("requires --non-interactive")); +} diff --git a/lib/crates/fabro-cli/tests/it/cmd/provider_login.rs b/lib/crates/fabro-cli/tests/it/cmd/provider_login.rs index 3693af3a9..124e6c366 100644 --- a/lib/crates/fabro-cli/tests/it/cmd/provider_login.rs +++ b/lib/crates/fabro-cli/tests/it/cmd/provider_login.rs @@ -18,6 +18,7 @@ fn help() { --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=] --provider LLM provider to authenticate with + --api-key-stdin Read an API key from stdin instead of prompting --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=] diff --git a/lib/crates/fabro-workflow/src/handler/llm/api.rs b/lib/crates/fabro-workflow/src/handler/llm/api.rs index bb934a5bc..27ad86ae8 100644 --- a/lib/crates/fabro-workflow/src/handler/llm/api.rs +++ b/lib/crates/fabro-workflow/src/handler/llm/api.rs @@ -7,6 +7,7 @@ use fabro_agent::{ AgentEvent, AgentProfile, AnthropicProfile, GeminiProfile, OpenAiProfile, Sandbox, Session, SessionOptions, Turn, }; +use fabro_auth::{CredentialResolver, CredentialUsage, ResolveError, ResolvedCredential}; use fabro_graphviz::graph::Node; use fabro_llm::client::Client; use fabro_llm::types::{Message, Request, TokenCounts}; @@ -34,6 +35,65 @@ fn build_profile(model: &str, provider: Provider) -> Box { } } +pub(crate) struct LlmClientBuildResult { + pub(crate) client: Client, + pub(crate) auth_issues: Vec<(Provider, ResolveError)>, +} + +pub(crate) async fn build_llm_client( + resolver: Option<&CredentialResolver>, +) -> Result { + let Some(resolver) = resolver else { + let client = Client::from_env() + .await + .map_err(|e| Error::handler(format!("Failed to create LLM client: {e}")))?; + return Ok(LlmClientBuildResult { + client, + auth_issues: Vec::new(), + }); + }; + + let mut api_credentials = Vec::new(); + let mut auth_issues = Vec::new(); + + for provider in Provider::ALL { + match resolver + .resolve(*provider, CredentialUsage::ApiRequest) + .await + { + Ok(ResolvedCredential::Api(credential)) => api_credentials.push(credential), + Ok(ResolvedCredential::Cli(_)) | Err(ResolveError::NotConfigured(_)) => {} + Err(err) => auth_issues.push((*provider, err)), + } + } + + let client = Client::from_credentials(api_credentials) + .await + .map_err(|e| Error::handler(format!("Failed to create LLM client: {e}")))?; + + Ok(LlmClientBuildResult { + client, + auth_issues, + }) +} + +pub(crate) fn auth_issue_message(provider: Provider, err: &ResolveError) -> String { + match err { + ResolveError::NotConfigured(_) => { + format!("{} is not configured", provider.display_name()) + } + ResolveError::RefreshFailed { source, .. } => format!( + "{} requires re-authentication: {}", + provider.display_name(), + source + ), + ResolveError::RefreshTokenMissing(_) => format!( + "{} requires re-authentication: refresh token missing", + provider.display_name() + ), + } +} + /// Shared state for tracking file modifications from agent tool calls. struct FileTracking { /// Maps tool_call_id → file_path for in-flight write/edit calls. @@ -122,11 +182,17 @@ pub struct AgentApiBackend { sessions: Mutex>, env: HashMap, mcp_servers: Vec, + resolver: Option, } impl AgentApiBackend { #[must_use] - pub fn new(model: String, provider: Provider, fallback_chain: Vec) -> Self { + pub fn new( + model: String, + provider: Provider, + fallback_chain: Vec, + resolver: CredentialResolver, + ) -> Self { Self { model, provider, @@ -134,6 +200,24 @@ impl AgentApiBackend { sessions: Mutex::new(HashMap::new()), env: HashMap::new(), mcp_servers: Vec::new(), + resolver: Some(resolver), + } + } + + #[must_use] + pub fn new_from_env( + model: String, + provider: Provider, + fallback_chain: Vec, + ) -> Self { + Self { + model, + provider, + fallback_chain, + sessions: Mutex::new(HashMap::new()), + env: HashMap::new(), + mcp_servers: Vec::new(), + resolver: None, } } @@ -165,6 +249,7 @@ impl AgentApiBackend { provider, node, sandbox, + self.resolver.as_ref(), &self.env, tool_hooks, self.mcp_servers.clone(), @@ -177,13 +262,12 @@ impl AgentApiBackend { provider: Provider, node: &Node, sandbox: &Arc, + resolver: Option<&CredentialResolver>, env: &HashMap, tool_hooks: Option>, mcp_servers: Vec, ) -> Result { - let client = Client::from_env() - .await - .map_err(|e| Error::handler(format!("Failed to create LLM client: {e}")))?; + let client = build_llm_client(resolver).await?.client; let mut profile = build_profile(model, provider); @@ -264,9 +348,7 @@ impl CodergenBackend for AgentApiBackend { prompt: &str, system_prompt: Option<&str>, ) -> Result { - let client = Client::from_env() - .await - .map_err(|e| Error::handler(format!("Failed to create LLM client: {e}")))?; + let client = build_llm_client(self.resolver.as_ref()).await?.client; let model = node.model().unwrap_or(&self.model); let provider = node @@ -507,6 +589,7 @@ impl CodergenBackend for AgentApiBackend { target_provider, node, sandbox, + self.resolver.as_ref(), &self.env, tool_hooks.clone(), self.mcp_servers.clone(), @@ -616,20 +699,26 @@ impl CodergenBackend for AgentApiBackend { #[cfg(test)] mod tests { use fabro_agent::subagent::SessionFactory; + use fabro_auth::{AuthCredential, AuthDetails, CredentialResolver}; + use fabro_vault::{SecretType, Vault}; + use tokio::sync::RwLock as AsyncRwLock; use super::*; #[test] fn agent_backend_stores_config() { - let backend = - AgentApiBackend::new("claude-opus-4-6".to_string(), Provider::OpenAi, Vec::new()); + let backend = AgentApiBackend::new_from_env( + "claude-opus-4-6".to_string(), + Provider::OpenAi, + Vec::new(), + ); assert_eq!(backend.model, "claude-opus-4-6"); assert_eq!(backend.provider, Provider::OpenAi); } #[test] fn agent_backend_initializes_empty_sessions() { - let backend = AgentApiBackend::new( + let backend = AgentApiBackend::new_from_env( "claude-opus-4-6".to_string(), Provider::Anthropic, Vec::new(), @@ -758,4 +847,33 @@ mod tests { assert!(names.contains(&"wait".to_string())); assert!(names.contains(&"close_agent".to_string())); } + + #[tokio::test] + async fn build_llm_client_uses_resolver_credentials() { + let dir = tempfile::tempdir().unwrap(); + let mut vault = Vault::load(dir.path().join("secrets.json")).unwrap(); + vault + .set( + "anthropic", + &serde_json::to_string(&AuthCredential { + provider: Provider::Anthropic, + details: AuthDetails::ApiKey { + key: "anthropic-key".to_string(), + }, + }) + .unwrap(), + SecretType::Credential, + None, + ) + .unwrap(); + let resolver = CredentialResolver::with_env_lookup( + Arc::new(AsyncRwLock::new(vault)), + Arc::new(|_| None), + ); + + let result = build_llm_client(Some(&resolver)).await.unwrap(); + + assert_eq!(result.client.provider_names(), vec!["anthropic"]); + assert!(result.auth_issues.is_empty()); + } } diff --git a/lib/crates/fabro-workflow/src/pipeline/initialize.rs b/lib/crates/fabro-workflow/src/pipeline/initialize.rs index 40e8a6151..dafac31de 100644 --- a/lib/crates/fabro-workflow/src/pipeline/initialize.rs +++ b/lib/crates/fabro-workflow/src/pipeline/initialize.rs @@ -26,6 +26,7 @@ use crate::devcontainer_bridge::{devcontainer_to_snapshot_config, run_devcontain use crate::error::Error; use crate::event::{Emitter, Event, RunNoticeLevel}; use crate::git::{self, GitSyncStatus, MetadataStore}; +use crate::handler::llm::api::{auth_issue_message, build_llm_client}; use crate::handler::llm::{AgentApiBackend, AgentCliBackend, BackendRouter}; use crate::handler::{HandlerRegistry, default_registry, sandbox_cancel_token}; use crate::run_options::GitCheckpointOptions; @@ -295,24 +296,56 @@ async fn build_registry( .values() .any(|n| graph::is_llm_handler_type(n.handler_type())); - match Client::from_env().await { - Ok(client) if client.provider_names().is_empty() => { + let resolver = vault.map(CredentialResolver::new); + + match build_llm_client(resolver.as_ref()).await { + Ok(result) if result.client.provider_names().is_empty() => { if graph_needs_llm { - return Err(Error::Precondition( - "No LLM providers configured. Set ANTHROPIC_API_KEY or OPENAI_API_KEY, or pass --dry-run to simulate.".to_string(), - )); + let detail = (!result.auth_issues.is_empty()).then(|| { + result + .auth_issues + .iter() + .map(|(provider, issue)| auth_issue_message(*provider, issue)) + .collect::>() + .join("; ") + }); + let prefix = detail.map_or_else( + || "No LLM providers configured".to_string(), + |detail| format!("No usable LLM providers configured: {detail}"), + ); + return Err(Error::Precondition(format!( + "{prefix}. Set ANTHROPIC_API_KEY or OPENAI_API_KEY, or pass --dry-run to simulate." + ))); } Ok((build_no_backend(), None, false)) } - Ok(client) => { + Ok(result) => { let env = sandbox_env.clone(); let model = spec.model.clone(); let provider = spec.provider; let fallback_chain = spec.fallback_chain.clone(); let mcp_servers = spec.mcp_servers.clone(); - let resolver = vault.map(CredentialResolver::new); + let client = result.client; let registry = Arc::new(default_registry(interviewer, move || { - let api = AgentApiBackend::new(model.clone(), provider, fallback_chain.clone()) + let api = resolver + .clone() + .map_or_else( + || { + AgentApiBackend::new_from_env( + model.clone(), + provider, + fallback_chain.clone(), + ) + }, + |resolver| { + AgentApiBackend::new( + model.clone(), + provider, + fallback_chain.clone(), + resolver, + ) + }, + ) .with_env(env.clone()) .with_mcp_servers(mcp_servers.clone()); let cli = resolver @@ -723,13 +756,16 @@ mod tests { use std::sync::atomic::AtomicBool; use std::time::Duration; + use fabro_auth::{AuthCredential, AuthDetails}; use fabro_graphviz::graph::{AttrValue, Edge, Graph, Node}; use fabro_interview::AutoApproveInterviewer; use fabro_sandbox::SandboxSpec; use fabro_store::Database; use fabro_types::settings::SettingsLayer; use fabro_types::{RunId, fixtures}; + use fabro_vault::{SecretType, Vault}; use object_store::memory::InMemory; + use tokio::sync::RwLock as AsyncRwLock; use super::*; use crate::event::StoreProgressLogger; @@ -773,6 +809,38 @@ mod tests { (graph, source) } + fn llm_graph() -> (Graph, String) { + let source = r#"digraph test { + start [shape=Mdiamond]; + writer [shape=box]; + exit [shape=Msquare]; + start -> writer; + writer -> exit; +}"# + .to_string(); + let mut graph = Graph::new("test"); + let mut start = Node::new("start"); + start.attrs.insert( + "shape".to_string(), + AttrValue::String("Mdiamond".to_string()), + ); + let mut writer = Node::new("writer"); + writer + .attrs + .insert("shape".to_string(), AttrValue::String("box".to_string())); + let mut exit = Node::new("exit"); + exit.attrs.insert( + "shape".to_string(), + AttrValue::String("Msquare".to_string()), + ); + graph.nodes.insert("start".to_string(), start); + graph.nodes.insert("writer".to_string(), writer); + graph.nodes.insert("exit".to_string(), exit); + graph.edges.push(Edge::new("start", "writer")); + graph.edges.push(Edge::new("writer", "exit")); + (graph, source) + } + fn test_settings(run_dir: &std::path::Path) -> RunOptions { RunOptions { settings: SettingsLayer::default(), @@ -882,6 +950,46 @@ mod tests { assert!(initialized.llm_client.is_none()); } + #[tokio::test] + async fn build_registry_accepts_vault_only_llm_provider() { + let dir = tempfile::tempdir().unwrap(); + let mut vault = Vault::load(dir.path().join("secrets.json")).unwrap(); + vault + .set( + "anthropic", + &serde_json::to_string(&AuthCredential { + provider: fabro_llm::Provider::Anthropic, + details: AuthDetails::ApiKey { + key: "anthropic-key".to_string(), + }, + }) + .unwrap(), + SecretType::Credential, + None, + ) + .unwrap(); + let (graph, _) = llm_graph(); + + let (_, llm_client, effective_dry_run) = build_registry( + &LlmSpec { + model: "claude-opus-4-6".to_string(), + provider: fabro_llm::Provider::Anthropic, + fallback_chain: Vec::new(), + mcp_servers: Vec::new(), + dry_run: false, + }, + Arc::new(AutoApproveInterviewer), + &HashMap::new(), + &graph, + Some(Arc::new(AsyncRwLock::new(vault))), + ) + .await + .unwrap(); + + assert!(!effective_dry_run); + assert!(llm_client.unwrap().provider_names().contains(&"anthropic")); + } + #[tokio::test] async fn initialize_runs_setup_commands() { let temp = tempfile::tempdir().unwrap();