From 25dfd768e5f804c293f369af256e1ef4476a86c0 Mon Sep 17 00:00:00 2001 From: Jack Amadeo Date: Fri, 7 Nov 2025 19:35:26 -0800 Subject: [PATCH] Better search paths and handling of CLI providers (#5554) Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- Cargo.lock | 18 +++ crates/goose-cli/src/commands/configure.rs | 51 +------ crates/goose-server/src/openapi.rs | 4 + crates/goose-server/src/routes/agent.rs | 29 ++-- .../src/routes/config_management.rs | 49 ++++++ crates/goose/Cargo.toml | 1 + crates/goose/src/agents/extension_manager.rs | 16 +- crates/goose/src/config/base.rs | 89 +++++++++-- crates/goose/src/config/search_path.rs | 140 +++++++++++++++--- crates/goose/src/lib.rs | 1 + crates/goose/src/providers/base.rs | 11 ++ crates/goose/src/providers/claude_code.rs | 126 ++-------------- crates/goose/src/providers/cursor_agent.rs | 105 +++---------- crates/goose/src/providers/factory.rs | 32 ++-- crates/goose/src/providers/gemini_cli.rs | 115 +++----------- crates/goose/src/providers/mod.rs | 5 +- .../goose/src/providers/provider_registry.rs | 9 ++ crates/goose/src/providers/provider_test.rs | 58 ++++++++ crates/goose/src/subprocess.rs | 10 ++ ui/desktop/openapi.json | 64 ++++++++ ui/desktop/src/api/sdk.gen.ts | 24 ++- ui/desktop/src/api/types.gen.ts | 23 +++ ui/desktop/src/components/ErrorBoundary.tsx | 2 +- .../components/ModelAndProviderContext.tsx | 93 ++++++------ .../providers/ProviderSettingsPage.tsx | 22 ++- .../modal/ProviderConfiguationModal.tsx | 119 +++++++++------ .../handlers/DefaultSubmitHandler.tsx | 43 +++--- 27 files changed, 721 insertions(+), 538 deletions(-) create mode 100644 crates/goose/src/providers/provider_test.rs create mode 100644 crates/goose/src/subprocess.rs diff --git a/Cargo.lock b/Cargo.lock index 516d6f3eae..74cd7078d7 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2130,6 +2130,12 @@ dependencies = [ "regex", ] +[[package]] +name = "env_home" +version = "0.1.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "c7f84e12ccf0a7ddc17a6c41c93326024c42920d7ee630d04950e6926645c0fe" + [[package]] name = "env_logger" version = "0.11.8" @@ -2685,6 +2691,7 @@ dependencies = [ "utoipa", "uuid", "webbrowser 0.8.15", + "which 8.0.0", "winapi", "wiremock", "zip 0.6.6", @@ -7875,6 +7882,17 @@ dependencies = [ "winsafe", ] +[[package]] +name = "which" +version = "8.0.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "d3fabb953106c3c8eea8306e4393700d7657561cb43122571b172bbfb7c7ba1d" +dependencies = [ + "env_home", + "rustix 1.0.7", + "winsafe", +] + [[package]] name = "whoami" version = "1.6.1" diff --git a/crates/goose-cli/src/commands/configure.rs b/crates/goose-cli/src/commands/configure.rs index d0d670f97b..c28a9ce4f7 100644 --- a/crates/goose-cli/src/commands/configure.rs +++ b/crates/goose-cli/src/commands/configure.rs @@ -19,9 +19,8 @@ use goose::config::{ }; use goose::conversation::message::Message; use goose::model::ModelConfig; +use goose::providers::provider_test::test_provider_configuration; use goose::providers::{create, providers}; -use rmcp::model::{Tool, ToolAnnotations}; -use rmcp::object; use serde_json::Value; use std::collections::HashMap; @@ -601,54 +600,14 @@ pub async fn configure_provider_dialog() -> anyhow::Result { let spin = spinner(); spin.start("Checking your configuration..."); - // Create model config with env var settings let toolshim_enabled = std::env::var("GOOSE_TOOLSHIM") .map(|val| val == "1" || val.to_lowercase() == "true") .unwrap_or(false); + let toolshim_model = std::env::var("GOOSE_TOOLSHIM_OLLAMA_MODEL").ok(); - let model_config = ModelConfig::new(&model)? - .with_max_tokens(Some(50)) - .with_toolshim(toolshim_enabled) - .with_toolshim_model(std::env::var("GOOSE_TOOLSHIM_OLLAMA_MODEL").ok()); - - let provider = create(provider_name, model_config).await?; - - let messages = - vec![Message::user().with_text("What is the weather like in San Francisco today?")]; - // Only add the sample tool if toolshim is not enabled - let tools = if !toolshim_enabled { - let sample_tool = Tool::new( - "get_weather".to_string(), - "Get current temperature for a given location.".to_string(), - object!({ - "type": "object", - "required": ["location"], - "properties": { - "location": {"type": "string"} - } - }), - ) - .annotate(ToolAnnotations { - title: Some("Get weather".to_string()), - read_only_hint: Some(true), - destructive_hint: Some(false), - idempotent_hint: Some(false), - open_world_hint: Some(false), - }); - vec![sample_tool] - } else { - vec![] - }; - - let result = provider - .complete( - "You are an AI agent called goose. You use tools of connected extensions to solve problems.", - &messages, - &tools.into_iter().collect::>() - ).await; - - match result { - Ok((_message, _usage)) => { + match test_provider_configuration(provider_name, &model, toolshim_enabled, toolshim_model).await + { + Ok(()) => { config.set_goose_provider(provider_name)?; config.set_goose_model(&model)?; print_config_file_saved()?; diff --git a/crates/goose-server/src/openapi.rs b/crates/goose-server/src/openapi.rs index 4667b93135..b3f9aedb14 100644 --- a/crates/goose-server/src/openapi.rs +++ b/crates/goose-server/src/openapi.rs @@ -344,6 +344,8 @@ derive_utoipa!(Icon as IconSchema); super::routes::config_management::get_custom_provider, super::routes::config_management::update_custom_provider, super::routes::config_management::remove_custom_provider, + super::routes::config_management::check_provider, + super::routes::config_management::set_config_provider, super::routes::agent::start_agent, super::routes::agent::resume_agent, super::routes::agent::get_tools, @@ -394,6 +396,8 @@ derive_utoipa!(Icon as IconSchema); super::routes::config_management::ToolPermission, super::routes::config_management::UpsertPermissionsQuery, super::routes::config_management::UpdateCustomProviderRequest, + super::routes::config_management::CheckProviderRequest, + super::routes::config_management::SetProviderRequest, super::routes::reply::PermissionConfirmationRequest, super::routes::reply::ChatRequest, super::routes::session::ImportSessionRequest, diff --git a/crates/goose-server/src/routes/agent.rs b/crates/goose-server/src/routes/agent.rs index 031b7096b2..90c6db9a51 100644 --- a/crates/goose-server/src/routes/agent.rs +++ b/crates/goose-server/src/routes/agent.rs @@ -3,6 +3,7 @@ use crate::routes::recipe_utils::{ apply_recipe_to_agent, build_recipe_with_parameter_values, load_recipe_by_id, validate_recipe, }; use crate::state::AppState; +use axum::response::IntoResponse; use axum::{ extract::{Query, State}, http::StatusCode, @@ -399,36 +400,42 @@ async fn get_tools( async fn update_agent_provider( State(state): State>, Json(payload): Json, -) -> Result { +) -> Result<(), impl IntoResponse> { let agent = state .get_agent_for_route(payload.session_id.clone()) - .await?; + .await + .map_err(|e| (e, "No agent for session id".to_owned()))?; let config = Config::global(); let model = match payload.model.or_else(|| config.get_goose_model().ok()) { Some(m) => m, None => { - tracing::error!("No model specified"); - return Err(StatusCode::BAD_REQUEST); + return Err((StatusCode::BAD_REQUEST, "No model specified".to_owned())); } }; let model_config = ModelConfig::new(&model).map_err(|e| { - tracing::error!("Invalid model config: {}", e); - StatusCode::BAD_REQUEST + ( + StatusCode::BAD_REQUEST, + format!("Invalid model config: {}", e), + ) })?; let new_provider = create(&payload.provider, model_config).await.map_err(|e| { - tracing::error!("Failed to create provider: {}", e); - StatusCode::BAD_REQUEST + ( + StatusCode::BAD_REQUEST, + format!("Failed to create {} provider: {}", &payload.provider, e), + ) })?; agent.update_provider(new_provider).await.map_err(|e| { - tracing::error!("Failed to update provider: {}", e); - StatusCode::INTERNAL_SERVER_ERROR + ( + StatusCode::INTERNAL_SERVER_ERROR, + format!("Failed to update provider: {}", e), + ) })?; - Ok(StatusCode::OK) + Ok(()) } #[utoipa::path( diff --git a/crates/goose-server/src/routes/config_management.rs b/crates/goose-server/src/routes/config_management.rs index f40e86b2f2..98f1ef0b4f 100644 --- a/crates/goose-server/src/routes/config_management.rs +++ b/crates/goose-server/src/routes/config_management.rs @@ -12,6 +12,7 @@ use goose::config::ExtensionEntry; use goose::config::{Config, ConfigError}; use goose::model::ModelConfig; use goose::providers::base::{ProviderMetadata, ProviderType}; +use goose::providers::create_with_default_model; use goose::providers::pricing::{ get_all_pricing, get_model_pricing, parse_model_id, refresh_pricing, }; @@ -88,6 +89,17 @@ pub struct UpdateCustomProviderRequest { pub supports_streaming: Option, } +#[derive(Deserialize, ToSchema)] +pub struct CheckProviderRequest { + pub provider: String, +} + +#[derive(Deserialize, ToSchema)] +pub struct SetProviderRequest { + pub provider: String, + pub model: String, +} + #[derive(Serialize, ToSchema)] #[serde(rename_all = "camelCase")] pub struct MaskedSecret { @@ -734,6 +746,41 @@ pub async fn update_custom_provider( Ok(Json(format!("Updated custom provider: {}", id))) } +#[utoipa::path( + post, + path = "/config/check_provider", + request_body = CheckProviderRequest, +)] +pub async fn check_provider( + Json(CheckProviderRequest { provider }): Json, +) -> Result<(), (StatusCode, String)> { + create_with_default_model(&provider) + .await + .map_err(|err| (StatusCode::BAD_REQUEST, err.to_string()))?; + Ok(()) +} + +#[utoipa::path( + post, + path = "/config/set_provider", + request_body = SetProviderRequest, +)] +pub async fn set_config_provider( + Json(SetProviderRequest { provider, model }): Json, +) -> Result<(), (StatusCode, String)> { + create_with_default_model(&provider) + .await + .and_then(|_| { + let config = Config::global(); + config + .set_goose_provider(provider) + .and_then(|_| config.set_goose_model(model)) + .map_err(|e| anyhow::anyhow!(e)) + }) + .map_err(|err| (StatusCode::BAD_REQUEST, err.to_string()))?; + Ok(()) +} + pub fn routes(state: Arc) -> Router { Router::new() .route("/config", get(read_all_config)) @@ -758,6 +805,8 @@ pub fn routes(state: Arc) -> Router { ) .route("/config/custom-providers/{id}", put(update_custom_provider)) .route("/config/custom-providers/{id}", get(get_custom_provider)) + .route("/config/check_provider", post(check_provider)) + .route("/config/set_provider", post(set_config_provider)) .with_state(state) } diff --git a/crates/goose/Cargo.toml b/crates/goose/Cargo.toml index d5a175892f..efc80530e8 100644 --- a/crates/goose/Cargo.toml +++ b/crates/goose/Cargo.toml @@ -110,6 +110,7 @@ paste = "1.0.0" shellexpand = "3.1.1" indexmap = "2.12.0" ignore = "0.4.25" +which = "8.0.0" [target.'cfg(target_os = "windows")'.dependencies] diff --git a/crates/goose/src/agents/extension_manager.rs b/crates/goose/src/agents/extension_manager.rs index a3a660a1b9..3ae559fcae 100644 --- a/crates/goose/src/agents/extension_manager.rs +++ b/crates/goose/src/agents/extension_manager.rs @@ -34,10 +34,11 @@ use super::types::SharedProvider; use crate::agents::extension::{Envs, ProcessExit}; use crate::agents::extension_malware_check; use crate::agents::mcp_client::{McpClient, McpClientTrait}; -use crate::config::search_path::search_path_var; +use crate::config::search_path::SearchPaths; use crate::config::{get_all_extensions, Config}; use crate::oauth::oauth_flow; use crate::prompt_template; +use crate::subprocess::configure_command_no_window; use rmcp::model::{ CallToolRequestParam, Content, ErrorCode, ErrorData, GetPromptResult, Prompt, ResourceContents, ServerInfo, Tool, @@ -129,9 +130,6 @@ impl ResourceItem { } } -#[cfg(windows)] -const CREATE_NO_WINDOW_FLAG: u32 = 0x08000000; - /// Sanitizes a string by replacing invalid characters with underscores. /// Valid characters match [a-zA-Z0-9_-] fn normalize(input: String) -> String { @@ -185,13 +183,11 @@ async fn child_process_client( ) -> ExtensionResult { #[cfg(unix)] command.process_group(0); - #[cfg(windows)] - command.creation_flags(CREATE_NO_WINDOW_FLAG); + configure_command_no_window(&mut command); - command.env( - "PATH", - search_path_var().map_err(|e| ExtensionError::ConfigError(format!("{}", e)))?, - ); + if let Ok(path) = SearchPaths::builder().path() { + command.env("PATH", path); + } let (transport, mut stderr) = TokioChildProcess::builder(command) .stderr(Stdio::piped()) diff --git a/crates/goose/src/config/base.rs b/crates/goose/src/config/base.rs index 0ad26428d3..a7938900f0 100644 --- a/crates/goose/src/config/base.rs +++ b/crates/goose/src/config/base.rs @@ -8,6 +8,7 @@ use serde_json::Value; use serde_yaml::Mapping; use std::collections::HashMap; use std::env; +use std::ffi::OsString; use std::fs::OpenOptions; use std::io::Write; use std::path::{Path, PathBuf}; @@ -138,17 +139,77 @@ impl Default for Config { } } -macro_rules! declare_param { - ($param_name:ident, $param_type:ty) => { - paste::paste! { - pub fn [](&self) -> Result<$param_type, ConfigError> { - self.get_param(stringify!($param_name)) +pub trait ConfigValue { + const KEY: &'static str; + const DEFAULT: &'static str; +} + +macro_rules! config_value { + ($key:ident, $type:ty) => { + impl Config { + paste::paste! { + pub fn [](&self) -> Result<$type, ConfigError> { + self.get_param(stringify!($key)) + } + } + paste::paste! { + pub fn [](&self, v: impl Into<$type>) -> Result<(), ConfigError> { + self.set_param(stringify!($key), &v.into()) + } } } + }; + + ($key:ident, $inner:ty, $default:expr) => { paste::paste! { - pub fn [](&self, v: impl Into<$param_type>) -> Result<(), ConfigError> { - self.set_param(stringify!($param_name), &v.into()) + #[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] + #[serde(transparent)] + pub struct [<$key:camel>]($inner); + + impl ConfigValue for [<$key:camel>] { + const KEY: &'static str = stringify!($key); + const DEFAULT: &'static str = $default; } + + impl Default for [<$key:camel>] { + fn default() -> Self { + [<$key:camel>]($default.into()) + } + } + + impl std::ops::Deref for [<$key:camel>] { + type Target = $inner; + + fn deref(&self) -> &Self::Target { + &self.0 + } + } + + impl std::ops::DerefMut for [<$key:camel>] { + fn deref_mut(&mut self) -> &mut Self::Target { + &mut self.0 + } + } + + impl std::fmt::Display for [<$key:camel>] { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + write!(f, "{:?}", self.0) + } + } + + impl From<$inner> for [<$key:camel>] { + fn from(value: $inner) -> Self { + [<$key:camel>](value) + } + } + + impl From<[<$key:camel>]> for $inner { + fn from(value: [<$key:camel>]) -> $inner { + value.0 + } + } + + config_value!($key, [<$key:camel>]); } }; } @@ -738,13 +799,17 @@ impl Config { }; Ok(()) } - - declare_param!(GOOSE_SEARCH_PATHS, Vec); - declare_param!(GOOSE_MODE, GooseMode); - declare_param!(GOOSE_PROVIDER, String); - declare_param!(GOOSE_MODEL, String); } +config_value!(CLAUDE_CODE_COMMAND, OsString, "claude"); +config_value!(GEMINI_CLI_COMMAND, OsString, "gemini"); +config_value!(CURSOR_AGENT_COMMAND, OsString, "cursor-agent"); + +config_value!(GOOSE_SEARCH_PATHS, Vec); +config_value!(GOOSE_MODE, GooseMode); +config_value!(GOOSE_PROVIDER, String); +config_value!(GOOSE_MODEL, String); + /// Load init-config.yaml from workspace root if it exists. /// This function is shared between the config recovery and the init_config endpoint. pub fn load_init_config_from_workspace() -> Result { diff --git a/crates/goose/src/config/search_path.rs b/crates/goose/src/config/search_path.rs index d500a4bb51..a8cfdc56a5 100644 --- a/crates/goose/src/config/search_path.rs +++ b/crates/goose/src/config/search_path.rs @@ -1,25 +1,123 @@ -use std::{env, ffi::OsString, path::PathBuf}; +use std::{ + env::{self}, + ffi::{OsStr, OsString}, + path::PathBuf, +}; -use crate::config::{Config, ConfigError}; +use anyhow::{Context, Result}; -pub fn search_path_var() -> Result { - let paths = Config::global() - .get_goose_search_paths() - .or_else(|err| match err { - ConfigError::NotFound(_) => Ok(vec![]), - err => Err(err), - })? - .into_iter() - .map(|s| PathBuf::from(shellexpand::tilde(&s).as_ref())); +use crate::config::Config; - env::join_paths( - paths.chain( - env::var_os("PATH") - .as_ref() - .map(env::split_paths) - .into_iter() - .flatten(), - ), - ) - .map_err(|e| ConfigError::DeserializeError(format!("{}", e))) +pub struct SearchPaths { + paths: Vec, +} + +impl SearchPaths { + pub fn builder() -> Self { + let mut paths = Config::global() + .get_goose_search_paths() + .unwrap_or_default(); + + paths.push("~/.local/bin".into()); + + #[cfg(unix)] + { + paths.push("/usr/local/bin".into()); + } + + if cfg!(target_os = "macos") { + paths.push("/opt/homebrew/bin".into()); + paths.push("/opt/local/bin".into()); + } + + Self { + paths: paths + .into_iter() + .map(|s| PathBuf::from(shellexpand::tilde(&s).as_ref())) + .collect(), + } + } + + pub fn with_npm(mut self) -> Self { + if cfg!(windows) { + if let Some(appdata) = dirs::data_dir() { + self.paths.push(appdata.join("npm")); + } + } else if let Some(home) = dirs::home_dir() { + self.paths.push(home.join(".npm-global/bin")); + } + self + } + + pub fn path(self) -> Result { + env::join_paths( + self.paths.into_iter().chain( + env::var_os("PATH") + .as_ref() + .map(env::split_paths) + .into_iter() + .flatten(), + ), + ) + .map_err(Into::into) + } + + pub fn resolve(self, name: N) -> Result + where + N: AsRef, + { + which::which_in_global(name.as_ref(), Some(self.path()?))? + .next() + .with_context(|| { + format!( + "could not resolve command '{}': file does not exist", + name.as_ref().to_string_lossy() + ) + }) + } +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn test_path_preserves_existing_path() { + let search_paths = SearchPaths::builder(); + let combined_path = search_paths.path().unwrap(); + + if let Some(existing_path) = env::var_os("PATH") { + let combined_str = combined_path.to_string_lossy(); + let existing_str = existing_path.to_string_lossy(); + + assert!(combined_str.contains(&existing_str.to_string())); + } + } + + #[test] + fn test_resolve_nonexistent_executable() { + let search_paths = SearchPaths::builder(); + + let result = search_paths.resolve("nonexistent_executable_12345_abcdef"); + + assert!( + result.is_err(), + "Resolving nonexistent executable should return an error" + ); + } + + #[test] + fn test_resolve_common_executable() { + let search_paths = SearchPaths::builder(); + + #[cfg(unix)] + let test_executable = "sh"; + + #[cfg(windows)] + let test_executable = "cmd"; + + search_paths + .resolve(test_executable) + .expect("should resolve sh (or cmd on Windows)"); + } } diff --git a/crates/goose/src/lib.rs b/crates/goose/src/lib.rs index 80da2e839c..1d6c86fcb3 100644 --- a/crates/goose/src/lib.rs +++ b/crates/goose/src/lib.rs @@ -19,6 +19,7 @@ pub mod scheduler_trait; pub mod security; pub mod session; pub mod session_context; +pub mod subprocess; pub mod token_counter; pub mod tool_inspection; pub mod tool_monitor; diff --git a/crates/goose/src/providers/base.rs b/crates/goose/src/providers/base.rs index ae7de6fbfe..e908cd88c9 100644 --- a/crates/goose/src/providers/base.rs +++ b/crates/goose/src/providers/base.rs @@ -4,6 +4,7 @@ use serde::{Deserialize, Serialize}; use super::errors::ProviderError; use super::retry::RetryConfig; +use crate::config::base::ConfigValue; use crate::conversation::message::Message; use crate::conversation::Conversation; use crate::model::ModelConfig; @@ -200,6 +201,16 @@ impl ConfigKey { } } + pub fn from_value_type(required: bool, secret: bool) -> Self { + Self { + name: T::KEY.to_string(), + required, + secret, + default: Some(T::DEFAULT.to_string()), + oauth_flow: false, + } + } + /// Create a new ConfigKey that uses OAuth device code flow for configuration /// /// This is used for providers that support OAuth authentication instead of manual API key entry. diff --git a/crates/goose/src/providers/claude_code.rs b/crates/goose/src/providers/claude_code.rs index 30f49a701e..6dc0c09602 100644 --- a/crates/goose/src/providers/claude_code.rs +++ b/crates/goose/src/providers/claude_code.rs @@ -2,6 +2,7 @@ use anyhow::Result; use async_trait::async_trait; use rmcp::model::Role; use serde_json::{json, Value}; +use std::ffi::OsString; use std::path::PathBuf; use std::process::Stdio; use tokio::io::{AsyncBufReadExt, BufReader}; @@ -10,9 +11,12 @@ use tokio::process::Command; use super::base::{ConfigKey, Provider, ProviderMetadata, ProviderUsage, Usage}; use super::errors::ProviderError; use super::utils::{filter_extensions_from_system_prompt, RequestLog}; +use crate::config::base::ClaudeCodeCommand; +use crate::config::search_path::SearchPaths; use crate::config::{Config, GooseMode}; use crate::conversation::message::{Message, MessageContent}; use crate::model::ModelConfig; +use crate::subprocess::configure_command_no_window; use rmcp::model::Tool; pub const CLAUDE_CODE_DEFAULT_MODEL: &str = "claude-sonnet-4-20250514"; @@ -21,7 +25,7 @@ pub const CLAUDE_CODE_DOC_URL: &str = "https://code.claude.com/docs/en/setup"; #[derive(Debug, serde::Serialize)] pub struct ClaudeCodeProvider { - command: String, + command: PathBuf, model: ModelConfig, #[serde(skip)] name: String, @@ -30,15 +34,8 @@ pub struct ClaudeCodeProvider { impl ClaudeCodeProvider { pub async fn from_env(model: ModelConfig) -> Result { let config = crate::config::Config::global(); - let command: String = config - .get_param("CLAUDE_CODE_COMMAND") - .unwrap_or_else(|_| "claude".to_string()); - - let resolved_command = if !command.contains('/') { - Self::find_claude_executable(&command).unwrap_or(command) - } else { - command - }; + let command: OsString = config.get_claude_code_command().unwrap_or_default().into(); + let resolved_command = SearchPaths::builder().with_npm().resolve(command)?; Ok(Self { command: resolved_command, @@ -47,61 +44,6 @@ impl ClaudeCodeProvider { }) } - /// Search for claude executable in common installation locations - fn find_claude_executable(command_name: &str) -> Option { - let home = std::env::var("HOME").ok()?; - - let search_paths = vec![ - format!("{}/.claude/local/{}", home, command_name), - format!("{}/.local/bin/{}", home, command_name), - format!("{}/bin/{}", home, command_name), - format!("/usr/local/bin/{}", command_name), - format!("/usr/bin/{}", command_name), - format!("/opt/claude/{}", command_name), - ]; - - for path in search_paths { - let path_buf = PathBuf::from(&path); - if path_buf.exists() && path_buf.is_file() { - #[cfg(unix)] - { - use std::os::unix::fs::PermissionsExt; - if let Ok(metadata) = std::fs::metadata(&path_buf) { - let permissions = metadata.permissions(); - if permissions.mode() & 0o111 != 0 { - tracing::info!("Found claude executable at: {}", path); - return Some(path); - } - } - } - #[cfg(not(unix))] - { - tracing::info!("Found claude executable at: {}", path); - return Some(path); - } - } - } - - if let Ok(path_var) = std::env::var("PATH") { - #[cfg(unix)] - let path_separator = ':'; - #[cfg(windows)] - let path_separator = ';'; - - for dir in path_var.split(path_separator) { - let path_buf = PathBuf::from(dir).join(command_name); - if path_buf.exists() && path_buf.is_file() { - let full_path = path_buf.to_string_lossy().to_string(); - tracing::info!("Found claude executable in PATH at: {}", full_path); - return Some(full_path); - } - } - } - - tracing::warn!("Could not find claude executable in common locations"); - None - } - /// Convert goose messages to the format expected by claude CLI fn messages_to_claude_format(&self, _system: &str, messages: &[Message]) -> Result { let mut claude_messages = Vec::new(); @@ -312,7 +254,7 @@ impl ClaudeCodeProvider { if std::env::var("GOOSE_CLAUDE_CODE_DEBUG").is_ok() { println!("=== CLAUDE CODE PROVIDER DEBUG ==="); - println!("Command: {}", self.command); + println!("Command: {:?}", self.command); println!("Original system prompt length: {} chars", system.len()); println!( "Filtered system prompt length: {} chars", @@ -328,6 +270,7 @@ impl ClaudeCodeProvider { } let mut cmd = Command::new(&self.command); + configure_command_no_window(&mut cmd); cmd.arg("-p") .arg(messages_json.to_string()) .arg("--system-prompt") @@ -345,18 +288,12 @@ impl ClaudeCodeProvider { cmd.stdout(Stdio::piped()).stderr(Stdio::piped()); - let mut child = cmd - .spawn() - .map_err(|e| ProviderError::RequestFailed(format!( - "\n\n ## Unable to find Claude Code CLI on the path.\n\n\ - **Error details:** Failed to spawn command '{}': {}\n\n\ - **Please ensure:**\n\ - - Claude Code CLI is installed and logged in\n\ - - The command is in your PATH, or set `CLAUDE_CODE_COMMAND` in your config\n\n\ - **For full features, please use the Anthropic provider with an API key if possible.**\n\n\ - Visit {} for installation instructions.", - self.command, e, CLAUDE_CODE_DOC_URL - )))?; + let mut child = cmd.spawn().map_err(|e| { + ProviderError::RequestFailed(format!( + "Failed to spawn Claude CLI command '{:?}': {}.", + self.command, e + )) + })?; let stdout = child .stdout @@ -461,12 +398,7 @@ impl Provider for ClaudeCodeProvider { CLAUDE_CODE_DEFAULT_MODEL, CLAUDE_CODE_KNOWN_MODELS.to_vec(), CLAUDE_CODE_DOC_URL, - vec![ConfigKey::new( - "CLAUDE_CODE_COMMAND", - false, - false, - Some("claude"), - )], + vec![ConfigKey::from_value_type::(true, false)], ) } @@ -521,29 +453,3 @@ impl Provider for ClaudeCodeProvider { )) } } - -#[cfg(test)] -mod tests { - use super::ModelConfig; - use super::*; - - #[tokio::test] - async fn test_claude_code_invalid_model_no_fallback() { - // Test that an invalid model is kept as-is (no fallback) - let invalid_model = ModelConfig::new_or_fail("invalid-model"); - let provider = ClaudeCodeProvider::from_env(invalid_model).await.unwrap(); - let config = provider.get_model_config(); - - assert_eq!(config.model_name, "invalid-model"); - } - - #[tokio::test] - async fn test_claude_code_valid_model() { - // Test that a valid model is preserved - let valid_model = ModelConfig::new_or_fail("sonnet"); - let provider = ClaudeCodeProvider::from_env(valid_model).await.unwrap(); - let config = provider.get_model_config(); - - assert_eq!(config.model_name, "sonnet"); - } -} diff --git a/crates/goose/src/providers/cursor_agent.rs b/crates/goose/src/providers/cursor_agent.rs index 66469c83c2..c6cdd0ab0f 100644 --- a/crates/goose/src/providers/cursor_agent.rs +++ b/crates/goose/src/providers/cursor_agent.rs @@ -2,6 +2,7 @@ use anyhow::Result; use async_trait::async_trait; use rmcp::model::Role; use serde_json::{json, Value}; +use std::ffi::OsString; use std::path::PathBuf; use std::process::Stdio; use tokio::io::{AsyncBufReadExt, BufReader}; @@ -10,8 +11,11 @@ use tokio::process::Command; use super::base::{ConfigKey, Provider, ProviderMetadata, ProviderUsage, Usage}; use super::errors::ProviderError; use super::utils::{filter_extensions_from_system_prompt, RequestLog}; +use crate::config::base::CursorAgentCommand; +use crate::config::search_path::SearchPaths; use crate::conversation::message::{Message, MessageContent}; use crate::model::ModelConfig; +use crate::subprocess::configure_command_no_window; use rmcp::model::Tool; pub const CURSOR_AGENT_DEFAULT_MODEL: &str = "auto"; @@ -21,7 +25,7 @@ pub const CURSOR_AGENT_DOC_URL: &str = "https://docs.cursor.com/en/cli/overview" #[derive(Debug, serde::Serialize)] pub struct CursorAgentProvider { - command: String, + command: PathBuf, model: ModelConfig, #[serde(skip)] name: String, @@ -30,15 +34,8 @@ pub struct CursorAgentProvider { impl CursorAgentProvider { pub async fn from_env(model: ModelConfig) -> Result { let config = crate::config::Config::global(); - let command: String = config - .get_param("CURSOR_AGENT_COMMAND") - .unwrap_or_else(|_| "cursor-agent".to_string()); - - let resolved_command = if !command.contains('/') { - Self::find_cursor_agent_executable(&command).unwrap_or(command) - } else { - command - }; + let command: OsString = config.get_cursor_agent_command().unwrap_or_default().into(); + let resolved_command = SearchPaths::builder().with_npm().resolve(command)?; Ok(Self { command: resolved_command, @@ -58,60 +55,6 @@ impl CursorAgentProvider { .unwrap_or(false) } - /// Search for cursor-agent executable in common installation locations - fn find_cursor_agent_executable(command_name: &str) -> Option { - let home = std::env::var("HOME").ok()?; - - let search_paths = vec![ - format!("/opt/homebrew/bin/{}", command_name), - format!("/usr/bin/{}", command_name), - format!("/usr/local/bin/{}", command_name), - format!("{}/.local/bin/{}", home, command_name), - format!("{}/bin/{}", home, command_name), - ]; - - for path in search_paths { - let path_buf = PathBuf::from(&path); - if path_buf.exists() && path_buf.is_file() { - #[cfg(unix)] - { - use std::os::unix::fs::PermissionsExt; - if let Ok(metadata) = std::fs::metadata(&path_buf) { - let permissions = metadata.permissions(); - if permissions.mode() & 0o111 != 0 { - tracing::info!("Found cursor-agent executable at: {}", path); - return Some(path); - } - } - } - #[cfg(not(unix))] - { - tracing::info!("Found cursor-agent executable at: {}", path); - return Some(path); - } - } - } - - if let Ok(path_var) = std::env::var("PATH") { - #[cfg(unix)] - let path_separator = ':'; - #[cfg(windows)] - let path_separator = ';'; - - for dir in path_var.split(path_separator) { - let path_buf = PathBuf::from(dir).join(command_name); - if path_buf.exists() && path_buf.is_file() { - let full_path = path_buf.to_string_lossy().to_string(); - tracing::info!("Found cursor-agent executable in PATH at: {}", full_path); - return Some(full_path); - } - } - } - - tracing::warn!("Could not find cursor-agent executable in common locations"); - None - } - /// Convert goose messages to a simple prompt format for cursor-agent CLI fn messages_to_cursor_agent_format(&self, system: &str, messages: &[Message]) -> String { let mut full_prompt = String::new(); @@ -240,7 +183,7 @@ impl CursorAgentProvider { if std::env::var("GOOSE_CURSOR_AGENT_DEBUG").is_ok() { println!("=== CURSOR AGENT PROVIDER DEBUG ==="); - println!("Command: {}", self.command); + println!("Command: {:?}", self.command); println!("Original system prompt length: {} chars", system.len()); println!( "Filtered system prompt length: {} chars", @@ -252,6 +195,11 @@ impl CursorAgentProvider { } let mut cmd = Command::new(&self.command); + configure_command_no_window(&mut cmd); + + if let Ok(path) = SearchPaths::builder().with_npm().path() { + cmd.env("PATH", path); + } // Only pass model parameter if it's in the known models list if CURSOR_AGENT_KNOWN_MODELS.contains(&self.model.model_name.as_str()) { @@ -269,8 +217,8 @@ impl CursorAgentProvider { let mut child = cmd .spawn() .map_err(|e| ProviderError::RequestFailed(format!( - "Failed to spawn cursor-agent CLI command '{}': {}. \ - Make sure the cursor-agent CLI is installed and in your PATH, or set CURSOR_AGENT_COMMAND in your config to the correct path.", + "Failed to spawn cursor-agent CLI command '{:?}': {}. \ + Make sure the cursor-agent CLI is installed and available in the configured search paths, or set CURSOR_AGENT_COMMAND in your config to the correct path.", self.command, e )))?; @@ -382,11 +330,8 @@ impl Provider for CursorAgentProvider { CURSOR_AGENT_DEFAULT_MODEL, CURSOR_AGENT_KNOWN_MODELS.to_vec(), CURSOR_AGENT_DOC_URL, - vec![ConfigKey::new( - "CURSOR_AGENT_COMMAND", - false, - false, - Some("cursor-agent"), + vec![ConfigKey::from_value_type::( + true, false, )], ) } @@ -442,19 +387,3 @@ impl Provider for CursorAgentProvider { )) } } - -#[cfg(test)] -mod tests { - use super::ModelConfig; - use super::*; - - #[tokio::test] - async fn test_cursor_agent_valid_model() { - // Test that a valid model is preserved - let valid_model = ModelConfig::new_or_fail("gpt-5"); - let provider = CursorAgentProvider::from_env(valid_model).await.unwrap(); - let config = provider.get_model_config(); - - assert_eq!(config.model_name, "gpt-5"); - } -} diff --git a/crates/goose/src/providers/factory.rs b/crates/goose/src/providers/factory.rs index bdc8f569ec..005cec21b9 100644 --- a/crates/goose/src/providers/factory.rs +++ b/crates/goose/src/providers/factory.rs @@ -24,9 +24,12 @@ use super::{ venice::VeniceProvider, xai::XaiProvider, }; -use crate::config::declarative_providers::register_declarative_providers; use crate::model::ModelConfig; use crate::providers::base::ProviderType; +use crate::{ + config::declarative_providers::register_declarative_providers, + providers::provider_registry::ProviderEntry, +}; use anyhow::Result; use tokio::sync::OnceCell; @@ -111,6 +114,15 @@ pub async fn refresh_custom_providers() -> Result<()> { Ok(()) } +async fn get_from_registry(name: &str) -> Result { + let guard = get_registry().await.read().unwrap(); + guard + .entries + .get(name) + .ok_or_else(|| anyhow::anyhow!("Unknown provider: {}", name)) + .cloned() +} + pub async fn create(name: &str, model: ModelConfig) -> Result> { let config = crate::config::Config::global(); @@ -119,19 +131,17 @@ pub async fn create(name: &str, model: ModelConfig) -> Result> return create_lead_worker_from_env(name, &model, &lead_model_name).await; } - let registry = get_registry().await; - let constructor = { - let guard = registry.read().unwrap(); - guard - .entries - .get(name) - .ok_or_else(|| anyhow::anyhow!("Unknown provider: {}", name))? - .constructor - .clone() - }; + let constructor = get_from_registry(name).await?.constructor.clone(); constructor(model).await } +pub async fn create_with_default_model(name: impl AsRef) -> Result> { + get_from_registry(name.as_ref()) + .await? + .create_with_default_model() + .await +} + pub async fn create_with_named_model( provider_name: &str, model_name: &str, diff --git a/crates/goose/src/providers/gemini_cli.rs b/crates/goose/src/providers/gemini_cli.rs index af0016cac5..4c2fa9ebd0 100644 --- a/crates/goose/src/providers/gemini_cli.rs +++ b/crates/goose/src/providers/gemini_cli.rs @@ -1,6 +1,7 @@ use anyhow::Result; use async_trait::async_trait; use serde_json::json; +use std::ffi::OsString; use std::path::PathBuf; use std::process::Stdio; use tokio::io::{AsyncBufReadExt, BufReader}; @@ -9,9 +10,13 @@ use tokio::process::Command; use super::base::{Provider, ProviderMetadata, ProviderUsage, Usage}; use super::errors::ProviderError; use super::utils::{filter_extensions_from_system_prompt, RequestLog}; +use crate::config::base::GeminiCliCommand; +use crate::config::search_path::SearchPaths; +use crate::config::Config; use crate::conversation::message::{Message, MessageContent}; - use crate::model::ModelConfig; +use crate::providers::base::ConfigKey; +use crate::subprocess::configure_command_no_window; use rmcp::model::Role; use rmcp::model::Tool; @@ -22,7 +27,7 @@ pub const GEMINI_CLI_DOC_URL: &str = "https://ai.google.dev/gemini-api/docs"; #[derive(Debug, serde::Serialize)] pub struct GeminiCliProvider { - command: String, + command: PathBuf, model: ModelConfig, #[serde(skip)] name: String, @@ -30,16 +35,9 @@ pub struct GeminiCliProvider { impl GeminiCliProvider { pub async fn from_env(model: ModelConfig) -> Result { - let config = crate::config::Config::global(); - let command: String = config - .get_param("GEMINI_CLI_COMMAND") - .unwrap_or_else(|_| "gemini".to_string()); - - let resolved_command = if !command.contains('/') { - Self::find_gemini_executable(&command).unwrap_or(command) - } else { - command - }; + let config = Config::global(); + let command: OsString = config.get_gemini_cli_command().unwrap_or_default().into(); + let resolved_command = SearchPaths::builder().with_npm().resolve(command)?; Ok(Self { command: resolved_command, @@ -48,61 +46,6 @@ impl GeminiCliProvider { }) } - /// Search for gemini executable in common installation locations - fn find_gemini_executable(command_name: &str) -> Option { - let home = std::env::var("HOME").ok()?; - - // Common locations where gemini might be installed - let search_paths = vec![ - format!("{}/.gemini/local/{}", home, command_name), - format!("{}/.local/bin/{}", home, command_name), - format!("{}/bin/{}", home, command_name), - format!("/usr/local/bin/{}", command_name), - format!("/usr/bin/{}", command_name), - format!("/opt/gemini/{}", command_name), - format!("/opt/google/{}", command_name), - ]; - - for path in search_paths { - let path_buf = PathBuf::from(&path); - if path_buf.exists() && path_buf.is_file() { - // Check if it's executable - #[cfg(unix)] - { - use std::os::unix::fs::PermissionsExt; - if let Ok(metadata) = std::fs::metadata(&path_buf) { - let permissions = metadata.permissions(); - if permissions.mode() & 0o111 != 0 { - tracing::info!("Found gemini executable at: {}", path); - return Some(path); - } - } - } - #[cfg(not(unix))] - { - // On non-Unix systems, just check if file exists - tracing::info!("Found gemini executable at: {}", path); - return Some(path); - } - } - } - - // If not found in common locations, check if it's in PATH - if let Ok(path_var) = std::env::var("PATH") { - for dir in path_var.split(':') { - let full_path = format!("{}/{}", dir, command_name); - let path_buf = PathBuf::from(&full_path); - if path_buf.exists() && path_buf.is_file() { - tracing::info!("Found gemini executable in PATH at: {}", full_path); - return Some(full_path); - } - } - } - - tracing::warn!("Could not find gemini executable in common locations"); - None - } - /// Execute gemini CLI command with simple text prompt async fn execute_command( &self, @@ -138,12 +81,17 @@ impl GeminiCliProvider { if std::env::var("GOOSE_GEMINI_CLI_DEBUG").is_ok() { println!("=== GEMINI CLI PROVIDER DEBUG ==="); - println!("Command: {}", self.command); + println!("Command: {:?}", self.command); println!("Full prompt: {}", full_prompt); println!("================================"); } let mut cmd = Command::new(&self.command); + configure_command_no_window(&mut cmd); + + if let Ok(path) = SearchPaths::builder().with_npm().path() { + cmd.env("PATH", path); + } // Only pass model parameter if it's in the known models list if GEMINI_CLI_KNOWN_MODELS.contains(&self.model.model_name.as_str()) { @@ -156,8 +104,8 @@ impl GeminiCliProvider { let mut child = cmd.spawn().map_err(|e| { ProviderError::RequestFailed(format!( - "Failed to spawn Gemini CLI command '{}': {}. \ - Make sure the Gemini CLI is installed and in your PATH.", + "Failed to spawn Gemini CLI command '{:?}': {}. \ + Make sure the Gemini CLI is installed and available in the configured search paths.", self.command, e )) })?; @@ -287,7 +235,7 @@ impl Provider for GeminiCliProvider { GEMINI_CLI_DEFAULT_MODEL, GEMINI_CLI_KNOWN_MODELS.to_vec(), GEMINI_CLI_DOC_URL, - vec![], // No configuration needed + vec![ConfigKey::from_value_type::(true, false)], ) } @@ -347,28 +295,3 @@ impl Provider for GeminiCliProvider { )) } } - -#[cfg(test)] -mod tests { - use super::*; - - #[tokio::test] - async fn test_gemini_cli_invalid_model_no_fallback() { - // Test that an invalid model is kept as-is (no fallback) - let invalid_model = ModelConfig::new_or_fail("invalid-model"); - let provider = GeminiCliProvider::from_env(invalid_model).await.unwrap(); - let config = provider.get_model_config(); - - assert_eq!(config.model_name, "invalid-model"); - } - - #[tokio::test] - async fn test_gemini_cli_valid_model() { - // Test that a valid model is preserved - let valid_model = ModelConfig::new_or_fail(GEMINI_CLI_DEFAULT_MODEL); - let provider = GeminiCliProvider::from_env(valid_model).await.unwrap(); - let config = provider.get_model_config(); - - assert_eq!(config.model_name, GEMINI_CLI_DEFAULT_MODEL); - } -} diff --git a/crates/goose/src/providers/mod.rs b/crates/goose/src/providers/mod.rs index 5658bdaff8..dd554f3dc8 100644 --- a/crates/goose/src/providers/mod.rs +++ b/crates/goose/src/providers/mod.rs @@ -24,6 +24,7 @@ pub mod openai; pub mod openrouter; pub mod pricing; pub mod provider_registry; +pub mod provider_test; mod retry; pub mod sagemaker_tgi; pub mod snowflake; @@ -36,4 +37,6 @@ pub mod utils_universal_openai_stream; pub mod venice; pub mod xai; -pub use factory::{create, create_with_named_model, providers, refresh_custom_providers}; +pub use factory::{ + create, create_with_default_model, create_with_named_model, providers, refresh_custom_providers, +}; diff --git a/crates/goose/src/providers/provider_registry.rs b/crates/goose/src/providers/provider_registry.rs index 9e6f568ce4..c297c28dcb 100644 --- a/crates/goose/src/providers/provider_registry.rs +++ b/crates/goose/src/providers/provider_registry.rs @@ -9,12 +9,21 @@ use std::sync::Arc; type ProviderConstructor = Arc BoxFuture<'static, Result>> + Send + Sync>; +#[derive(Clone)] pub struct ProviderEntry { metadata: ProviderMetadata, pub(crate) constructor: ProviderConstructor, provider_type: ProviderType, } +impl ProviderEntry { + pub async fn create_with_default_model(&self) -> Result> { + let default_model = &self.metadata.default_model; + let model_config = ModelConfig::new(default_model.as_str())?; + (self.constructor)(model_config).await + } +} + #[derive(Default)] pub struct ProviderRegistry { pub(crate) entries: HashMap, diff --git a/crates/goose/src/providers/provider_test.rs b/crates/goose/src/providers/provider_test.rs new file mode 100644 index 0000000000..386ddc4918 --- /dev/null +++ b/crates/goose/src/providers/provider_test.rs @@ -0,0 +1,58 @@ +use crate::{conversation::message::Message, model::ModelConfig, providers::create}; +use anyhow::Result; +use rmcp::model::ToolAnnotations; +use rmcp::{model::Tool, object}; + +pub async fn test_provider_configuration( + provider_name: &str, + model: &str, + toolshim_enabled: bool, + toolshim_model: Option, +) -> Result<()> { + let model_config = ModelConfig::new(model)? + .with_max_tokens(Some(50)) + .with_toolshim(toolshim_enabled) + .with_toolshim_model(toolshim_model); + + let provider = create(provider_name, model_config).await?; + + let messages = + vec![Message::user().with_text("What is the weather like in San Francisco today?")]; + + let tools = if !toolshim_enabled { + vec![create_sample_weather_tool()] + } else { + vec![] + }; + + let _result = provider + .complete( + "You are an AI agent called goose. You use tools of connected extensions to solve problems.", + &messages, + &tools.into_iter().collect::>() + ) + .await?; + + Ok(()) +} + +fn create_sample_weather_tool() -> Tool { + Tool::new( + "get_weather".to_string(), + "Get current temperature for a given location.".to_string(), + object!({ + "type": "object", + "required": ["location"], + "properties": { + "location": {"type": "string"} + } + }), + ) + .annotate(ToolAnnotations { + title: Some("Get weather".to_string()), + read_only_hint: Some(true), + destructive_hint: Some(false), + idempotent_hint: Some(false), + open_world_hint: Some(false), + }) +} diff --git a/crates/goose/src/subprocess.rs b/crates/goose/src/subprocess.rs new file mode 100644 index 0000000000..345a63781b --- /dev/null +++ b/crates/goose/src/subprocess.rs @@ -0,0 +1,10 @@ +use tokio::process::Command; + +#[cfg(windows)] +const CREATE_NO_WINDOW_FLAG: u32 = 0x08000000; + +#[allow(unused_variables)] +pub fn configure_command_no_window(command: &mut Command) { + #[cfg(windows)] + command.creation_flags(CREATE_NO_WINDOW_FLAG); +} diff --git a/ui/desktop/openapi.json b/ui/desktop/openapi.json index ba683314cb..112e243818 100644 --- a/ui/desktop/openapi.json +++ b/ui/desktop/openapi.json @@ -382,6 +382,25 @@ } } }, + "/config/check_provider": { + "post": { + "tags": [ + "super::routes::config_management" + ], + "operationId": "check_provider", + "requestBody": { + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/CheckProviderRequest" + } + } + }, + "required": true + }, + "responses": {} + } + }, "/config/custom-providers": { "post": { "tags": [ @@ -840,6 +859,25 @@ } } }, + "/config/set_provider": { + "post": { + "tags": [ + "super::routes::config_management" + ], + "operationId": "set_config_provider", + "requestBody": { + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/SetProviderRequest" + } + } + }, + "required": true + }, + "responses": {} + } + }, "/config/upsert": { "post": { "tags": [ @@ -2152,6 +2190,17 @@ } } }, + "CheckProviderRequest": { + "type": "object", + "required": [ + "provider" + ], + "properties": { + "provider": { + "type": "string" + } + } + }, "ConfigKey": { "type": "object", "description": "Configuration key metadata for provider setup", @@ -4359,6 +4408,21 @@ } } }, + "SetProviderRequest": { + "type": "object", + "required": [ + "provider", + "model" + ], + "properties": { + "model": { + "type": "string" + }, + "provider": { + "type": "string" + } + } + }, "Settings": { "type": "object", "properties": { diff --git a/ui/desktop/src/api/sdk.gen.ts b/ui/desktop/src/api/sdk.gen.ts index 7f65f47dc5..0634e71abc 100644 --- a/ui/desktop/src/api/sdk.gen.ts +++ b/ui/desktop/src/api/sdk.gen.ts @@ -2,7 +2,7 @@ import type { Client, Options as Options2, TDataShape } from './client'; import { client } from './client.gen'; -import type { AddExtensionData, AddExtensionErrors, AddExtensionResponses, AgentAddExtensionData, AgentAddExtensionErrors, AgentAddExtensionResponses, AgentRemoveExtensionData, AgentRemoveExtensionErrors, AgentRemoveExtensionResponses, BackupConfigData, BackupConfigErrors, BackupConfigResponses, ConfirmPermissionData, ConfirmPermissionErrors, ConfirmPermissionResponses, CreateCustomProviderData, CreateCustomProviderErrors, CreateCustomProviderResponses, CreateRecipeData, CreateRecipeErrors, CreateRecipeResponses, CreateScheduleData, CreateScheduleErrors, CreateScheduleResponses, DecodeRecipeData, DecodeRecipeErrors, DecodeRecipeResponses, DeleteRecipeData, DeleteRecipeErrors, DeleteRecipeResponses, DeleteScheduleData, DeleteScheduleErrors, DeleteScheduleResponses, DeleteSessionData, DeleteSessionErrors, DeleteSessionResponses, DiagnosticsData, DiagnosticsErrors, DiagnosticsResponses, EncodeRecipeData, EncodeRecipeErrors, EncodeRecipeResponses, ExportSessionData, ExportSessionErrors, ExportSessionResponses, GetCustomProviderData, GetCustomProviderErrors, GetCustomProviderResponses, GetExtensionsData, GetExtensionsErrors, GetExtensionsResponses, GetProviderModelsData, GetProviderModelsErrors, GetProviderModelsResponses, GetSessionData, GetSessionErrors, GetSessionInsightsData, GetSessionInsightsErrors, GetSessionInsightsResponses, GetSessionResponses, GetToolsData, GetToolsErrors, GetToolsResponses, ImportSessionData, ImportSessionErrors, ImportSessionResponses, InitConfigData, InitConfigErrors, InitConfigResponses, InspectRunningJobData, InspectRunningJobErrors, InspectRunningJobResponses, KillRunningJobData, KillRunningJobResponses, ListRecipesData, ListRecipesErrors, ListRecipesResponses, ListSchedulesData, ListSchedulesErrors, ListSchedulesResponses, ListSessionsData, ListSessionsErrors, ListSessionsResponses, ParseRecipeData, ParseRecipeErrors, ParseRecipeResponses, PauseScheduleData, PauseScheduleErrors, PauseScheduleResponses, ProvidersData, ProvidersResponses, ReadAllConfigData, ReadAllConfigResponses, ReadConfigData, ReadConfigErrors, ReadConfigResponses, RecoverConfigData, RecoverConfigErrors, RecoverConfigResponses, RemoveConfigData, RemoveConfigErrors, RemoveConfigResponses, RemoveCustomProviderData, RemoveCustomProviderErrors, RemoveCustomProviderResponses, RemoveExtensionData, RemoveExtensionErrors, RemoveExtensionResponses, ReplyData, ReplyErrors, ReplyResponses, ResumeAgentData, ResumeAgentErrors, ResumeAgentResponses, RunNowHandlerData, RunNowHandlerErrors, RunNowHandlerResponses, SaveRecipeData, SaveRecipeErrors, SaveRecipeResponses, ScanRecipeData, ScanRecipeResponses, SessionsHandlerData, SessionsHandlerErrors, SessionsHandlerResponses, StartAgentData, StartAgentErrors, StartAgentResponses, StartOpenrouterSetupData, StartOpenrouterSetupResponses, StartTetrateSetupData, StartTetrateSetupResponses, StatusData, StatusResponses, UnpauseScheduleData, UnpauseScheduleErrors, UnpauseScheduleResponses, UpdateAgentProviderData, UpdateAgentProviderErrors, UpdateAgentProviderResponses, UpdateCustomProviderData, UpdateCustomProviderErrors, UpdateCustomProviderResponses, UpdateFromSessionData, UpdateFromSessionErrors, UpdateFromSessionResponses, UpdateRouterToolSelectorData, UpdateRouterToolSelectorErrors, UpdateRouterToolSelectorResponses, UpdateScheduleData, UpdateScheduleErrors, UpdateScheduleResponses, UpdateSessionNameData, UpdateSessionNameErrors, UpdateSessionNameResponses, UpdateSessionUserRecipeValuesData, UpdateSessionUserRecipeValuesErrors, UpdateSessionUserRecipeValuesResponses, UpsertConfigData, UpsertConfigErrors, UpsertConfigResponses, UpsertPermissionsData, UpsertPermissionsErrors, UpsertPermissionsResponses, ValidateConfigData, ValidateConfigErrors, ValidateConfigResponses } from './types.gen'; +import type { AddExtensionData, AddExtensionErrors, AddExtensionResponses, AgentAddExtensionData, AgentAddExtensionErrors, AgentAddExtensionResponses, AgentRemoveExtensionData, AgentRemoveExtensionErrors, AgentRemoveExtensionResponses, BackupConfigData, BackupConfigErrors, BackupConfigResponses, CheckProviderData, ConfirmPermissionData, ConfirmPermissionErrors, ConfirmPermissionResponses, CreateCustomProviderData, CreateCustomProviderErrors, CreateCustomProviderResponses, CreateRecipeData, CreateRecipeErrors, CreateRecipeResponses, CreateScheduleData, CreateScheduleErrors, CreateScheduleResponses, DecodeRecipeData, DecodeRecipeErrors, DecodeRecipeResponses, DeleteRecipeData, DeleteRecipeErrors, DeleteRecipeResponses, DeleteScheduleData, DeleteScheduleErrors, DeleteScheduleResponses, DeleteSessionData, DeleteSessionErrors, DeleteSessionResponses, DiagnosticsData, DiagnosticsErrors, DiagnosticsResponses, EncodeRecipeData, EncodeRecipeErrors, EncodeRecipeResponses, ExportSessionData, ExportSessionErrors, ExportSessionResponses, GetCustomProviderData, GetCustomProviderErrors, GetCustomProviderResponses, GetExtensionsData, GetExtensionsErrors, GetExtensionsResponses, GetProviderModelsData, GetProviderModelsErrors, GetProviderModelsResponses, GetSessionData, GetSessionErrors, GetSessionInsightsData, GetSessionInsightsErrors, GetSessionInsightsResponses, GetSessionResponses, GetToolsData, GetToolsErrors, GetToolsResponses, ImportSessionData, ImportSessionErrors, ImportSessionResponses, InitConfigData, InitConfigErrors, InitConfigResponses, InspectRunningJobData, InspectRunningJobErrors, InspectRunningJobResponses, KillRunningJobData, KillRunningJobResponses, ListRecipesData, ListRecipesErrors, ListRecipesResponses, ListSchedulesData, ListSchedulesErrors, ListSchedulesResponses, ListSessionsData, ListSessionsErrors, ListSessionsResponses, ParseRecipeData, ParseRecipeErrors, ParseRecipeResponses, PauseScheduleData, PauseScheduleErrors, PauseScheduleResponses, ProvidersData, ProvidersResponses, ReadAllConfigData, ReadAllConfigResponses, ReadConfigData, ReadConfigErrors, ReadConfigResponses, RecoverConfigData, RecoverConfigErrors, RecoverConfigResponses, RemoveConfigData, RemoveConfigErrors, RemoveConfigResponses, RemoveCustomProviderData, RemoveCustomProviderErrors, RemoveCustomProviderResponses, RemoveExtensionData, RemoveExtensionErrors, RemoveExtensionResponses, ReplyData, ReplyErrors, ReplyResponses, ResumeAgentData, ResumeAgentErrors, ResumeAgentResponses, RunNowHandlerData, RunNowHandlerErrors, RunNowHandlerResponses, SaveRecipeData, SaveRecipeErrors, SaveRecipeResponses, ScanRecipeData, ScanRecipeResponses, SessionsHandlerData, SessionsHandlerErrors, SessionsHandlerResponses, SetConfigProviderData, StartAgentData, StartAgentErrors, StartAgentResponses, StartOpenrouterSetupData, StartOpenrouterSetupResponses, StartTetrateSetupData, StartTetrateSetupResponses, StatusData, StatusResponses, UnpauseScheduleData, UnpauseScheduleErrors, UnpauseScheduleResponses, UpdateAgentProviderData, UpdateAgentProviderErrors, UpdateAgentProviderResponses, UpdateCustomProviderData, UpdateCustomProviderErrors, UpdateCustomProviderResponses, UpdateFromSessionData, UpdateFromSessionErrors, UpdateFromSessionResponses, UpdateRouterToolSelectorData, UpdateRouterToolSelectorErrors, UpdateRouterToolSelectorResponses, UpdateScheduleData, UpdateScheduleErrors, UpdateScheduleResponses, UpdateSessionNameData, UpdateSessionNameErrors, UpdateSessionNameResponses, UpdateSessionUserRecipeValuesData, UpdateSessionUserRecipeValuesErrors, UpdateSessionUserRecipeValuesResponses, UpsertConfigData, UpsertConfigErrors, UpsertConfigResponses, UpsertPermissionsData, UpsertPermissionsErrors, UpsertPermissionsResponses, ValidateConfigData, ValidateConfigErrors, ValidateConfigResponses } from './types.gen'; export type Options = Options2 & { /** @@ -116,6 +116,17 @@ export const backupConfig = (options?: Opt }); }; +export const checkProvider = (options: Options) => { + return (options.client ?? client).post({ + url: '/config/check_provider', + ...options, + headers: { + 'Content-Type': 'application/json', + ...options.headers + } + }); +}; + export const createCustomProvider = (options: Options) => { return (options.client ?? client).post({ url: '/config/custom-providers', @@ -238,6 +249,17 @@ export const removeConfig = (options: Opti }); }; +export const setConfigProvider = (options: Options) => { + return (options.client ?? client).post({ + url: '/config/set_provider', + ...options, + headers: { + 'Content-Type': 'application/json', + ...options.headers + } + }); +}; + export const upsertConfig = (options: Options) => { return (options.client ?? client).post({ url: '/config/upsert', diff --git a/ui/desktop/src/api/types.gen.ts b/ui/desktop/src/api/types.gen.ts index 50a061d25f..4826182d1b 100644 --- a/ui/desktop/src/api/types.gen.ts +++ b/ui/desktop/src/api/types.gen.ts @@ -32,6 +32,10 @@ export type ChatRequest = { session_id: string; }; +export type CheckProviderRequest = { + provider: string; +}; + /** * Configuration key metadata for provider setup */ @@ -731,6 +735,11 @@ export type SessionsQuery = { limit?: number; }; +export type SetProviderRequest = { + model: string; + provider: string; +}; + export type Settings = { goose_model?: string | null; goose_provider?: string | null; @@ -1212,6 +1221,13 @@ export type BackupConfigResponses = { export type BackupConfigResponse = BackupConfigResponses[keyof BackupConfigResponses]; +export type CheckProviderData = { + body: CheckProviderRequest; + path?: never; + query?: never; + url: '/config/check_provider'; +}; + export type CreateCustomProviderData = { body: UpdateCustomProviderRequest; path?: never; @@ -1578,6 +1594,13 @@ export type RemoveConfigResponses = { export type RemoveConfigResponse = RemoveConfigResponses[keyof RemoveConfigResponses]; +export type SetConfigProviderData = { + body: SetProviderRequest; + path?: never; + query?: never; + url: '/config/set_provider'; +}; + export type UpsertConfigData = { body: UpsertConfigQuery; path?: never; diff --git a/ui/desktop/src/components/ErrorBoundary.tsx b/ui/desktop/src/components/ErrorBoundary.tsx index 81866753d7..5404a2768d 100644 --- a/ui/desktop/src/components/ErrorBoundary.tsx +++ b/ui/desktop/src/components/ErrorBoundary.tsx @@ -34,7 +34,7 @@ export function ErrorUI({ error }: { error: Error }) {

)} - 
+        
           {error.message}
         

 
diff --git a/ui/desktop/src/components/ModelAndProviderContext.tsx b/ui/desktop/src/components/ModelAndProviderContext.tsx
index 60e27ad107..c35f5517d3 100644
--- a/ui/desktop/src/components/ModelAndProviderContext.tsx
+++ b/ui/desktop/src/components/ModelAndProviderContext.tsx
@@ -1,7 +1,7 @@
 import React, { createContext, useContext, useState, useEffect, useMemo, useCallback } from 'react';
 import { toastError, toastSuccess } from '../toasts';
 import Model, { getProviderMetadata } from './settings/models/modelInterface';
-import { ProviderMetadata, updateAgentProvider } from '../api';
+import { ProviderMetadata, setConfigProvider, updateAgentProvider } from '../api';
 import { useConfig } from './ConfigContext';
 import {
   getModelDisplayName,
@@ -12,10 +12,6 @@ import {
 export const UNKNOWN_PROVIDER_TITLE = 'Provider name lookup';
 
 // errors
-const CHANGE_MODEL_ERROR_TITLE = 'Change failed';
-const SWITCH_MODEL_AGENT_ERROR_MSG =
-  'Failed to start agent with selected model -- please try again';
-const CONFIG_UPDATE_ERROR_MSG = 'Failed to update configuration settings -- please try again';
 export const UNKNOWN_PROVIDER_MSG = 'Unknown provider in config -- please inspect your config.yaml';
 
 // success
@@ -43,61 +39,68 @@ const ModelAndProviderContext = createContext = ({ children }) => {
   const [currentModel, setCurrentModel] = useState(null);
   const [currentProvider, setCurrentProvider] = useState(null);
-  const { read, upsert, getProviders } = useConfig();
+  const { read, getProviders } = useConfig();
 
-  const changeModel = useCallback(
-    async (sessionId: string | null, model: Model) => {
-      const modelName = model.name;
-      const providerName = model.provider;
-      let phase = 'agent';
+  const changeModel = useCallback(async (sessionId: string | null, model: Model) => {
+    const modelName = model.name;
+    const providerName = model.provider;
+    let phase = 'agent';
 
-      try {
-        if (sessionId) {
-          await updateAgentProvider({
-            body: {
-              session_id: sessionId,
-              provider: providerName,
-              model: modelName,
-            },
-          });
-        }
-
-        phase = 'config';
-        await upsert('GOOSE_PROVIDER', providerName, false);
-        await upsert('GOOSE_MODEL', modelName, false);
-
-        setCurrentProvider(providerName);
-        setCurrentModel(modelName);
-
-        toastSuccess({
-          title: CHANGE_MODEL_TOAST_TITLE,
-          msg: `${SWITCH_MODEL_SUCCESS_MSG} -- using ${model.alias ?? modelName} from ${model.subtext ?? providerName}`,
-        });
-      } catch (error) {
-        console.error(`Failed to change model at ${phase} step -- ${modelName} ${providerName}`);
-        toastError({
-          title: CHANGE_MODEL_ERROR_TITLE,
-          msg: phase === 'agent' ? SWITCH_MODEL_AGENT_ERROR_MSG : CONFIG_UPDATE_ERROR_MSG,
-          traceback: error instanceof Error ? error.message : String(error),
+    try {
+      if (sessionId) {
+        await updateAgentProvider({
+          body: {
+            session_id: sessionId,
+            provider: providerName,
+            model: modelName,
+          },
         });
       }
-    },
-    [upsert]
-  );
+
+      phase = 'config';
+      await setConfigProvider({
+        body: {
+          provider: providerName,
+          model: modelName,
+        },
+        throwOnError: true,
+      });
+
+      setCurrentProvider(providerName);
+      setCurrentModel(modelName);
+
+      toastSuccess({
+        title: CHANGE_MODEL_TOAST_TITLE,
+        msg: `${SWITCH_MODEL_SUCCESS_MSG} -- using ${model.alias ?? modelName} from ${model.subtext ?? providerName}`,
+      });
+    } catch (error) {
+      console.error(`Failed to change model at ${phase} step -- ${modelName} ${providerName}`);
+      toastError({
+        title: `${providerName}/${modelName} failed`,
+        msg: `${error}`,
+        traceback: error instanceof Error ? error.message : String(error),
+      });
+    }
+  }, []);
 
   const getFallbackModelAndProvider = useCallback(async () => {
     const provider = window.appConfig.get('GOOSE_DEFAULT_PROVIDER') as string;
     const model = window.appConfig.get('GOOSE_DEFAULT_MODEL') as string;
     if (provider && model) {
       try {
-        await upsert('GOOSE_MODEL', model, false);
-        await upsert('GOOSE_PROVIDER', provider, false);
+        await setConfigProvider({
+          body: {
+            provider: provider,
+            model: model,
+          },
+          throwOnError: true,
+        });
       } catch (error) {
         console.error('[getFallbackModelAndProvider] Failed to write to config', error);
       }
     }
     return { model: model, provider: provider };
-  }, [upsert]);
+  }, []);
 
   const getCurrentModelAndProvider = useCallback(async () => {
     let model: string;
diff --git a/ui/desktop/src/components/settings/providers/ProviderSettingsPage.tsx b/ui/desktop/src/components/settings/providers/ProviderSettingsPage.tsx
index 4b437ae95a..5488b90b3c 100644
--- a/ui/desktop/src/components/settings/providers/ProviderSettingsPage.tsx
+++ b/ui/desktop/src/components/settings/providers/ProviderSettingsPage.tsx
@@ -3,7 +3,7 @@ import { ScrollArea } from '../../ui/scroll-area';
 import BackButton from '../../ui/BackButton';
 import ProviderGrid from './ProviderGrid';
 import { useConfig } from '../../ConfigContext';
-import { ProviderDetails } from '../../../api';
+import { ProviderDetails, setConfigProvider } from '../../../api';
 import { toastService } from '../../../toasts';
 
 interface ProviderSettingsProps {
@@ -12,7 +12,7 @@ interface ProviderSettingsProps {
 }
 
 export default function ProviderSettings({ onClose, isOnboarding }: ProviderSettingsProps) {
-  const { getProviders, upsert } = useConfig();
+  const { getProviders } = useConfig();
   const [loading, setLoading] = useState(true);
   const [providers, setProviders] = useState([]);
   const initialLoadDone = useRef(false);
@@ -56,15 +56,13 @@ export default function ProviderSettings({ onClose, isOnboarding }: ProviderSett
       const model = provider.metadata.default_model;
 
       try {
-        // update the config
-        // set GOOSE_PROVIDER in the config file
-        upsert('GOOSE_PROVIDER', provider_name, false).then((_) =>
-          console.log('Setting GOOSE_PROVIDER to', provider_name)
-        );
-        // set GOOSE_MODEL in the config file
-        upsert('GOOSE_MODEL', model, false).then((_) =>
-          console.log('Setting GOOSE_MODEL to', model)
-        );
+        await setConfigProvider({
+          body: {
+            provider: provider_name,
+            model,
+          },
+          throwOnError: true,
+        });
 
         toastService.configure({ silent: false });
         toastService.success({
@@ -85,7 +83,7 @@ export default function ProviderSettings({ onClose, isOnboarding }: ProviderSett
         });
       }
     },
-    [onClose, upsert]
+    [onClose]
   );
 
   return (
diff --git a/ui/desktop/src/components/settings/providers/modal/ProviderConfiguationModal.tsx b/ui/desktop/src/components/settings/providers/modal/ProviderConfiguationModal.tsx
index e04b5ec49a..1858589eb8 100644
--- a/ui/desktop/src/components/settings/providers/modal/ProviderConfiguationModal.tsx
+++ b/ui/desktop/src/components/settings/providers/modal/ProviderConfiguationModal.tsx
@@ -18,6 +18,7 @@ import { useConfig } from '../../../ConfigContext';
 import { useModelAndProvider } from '../../../ModelAndProviderContext';
 import { AlertTriangle } from 'lucide-react';
 import { ProviderDetails, removeCustomProvider } from '../../../../api';
+import { Button } from '../../../../components/ui/button';
 
 interface ProviderConfigurationModalProps {
   provider: ProviderDetails;
@@ -34,6 +35,7 @@ export default function ProviderConfigurationModal({
   const [configValues, setConfigValues] = useState>({});
   const [showDeleteConfirmation, setShowDeleteConfirmation] = useState(false);
   const [isActiveProvider, setIsActiveProvider] = useState(false);
+  const [error, setError] = useState(null);
 
   const requiredParameters = provider.metadata.config_keys.filter(
     (param) => param.required === true
@@ -79,8 +81,12 @@ export default function ProviderConfigurationModal({
         .map(([k, entry]) => [k, entry.value || ''])
     );
 
-    await providerConfigSubmitHandler(upsert, provider, toSubmit);
-    onClose();
+    try {
+      await providerConfigSubmitHandler(upsert, provider, toSubmit);
+      onClose();
+    } catch (error) {
+      setError(`${error}`);
+    }
   };
 
   const handleCancel = () => {
@@ -137,54 +143,71 @@ export default function ProviderConfigurationModal({
   };
 
   return (
-     !open && onClose()}>
-      
-        
-          
-            {getModalIcon()}
-            {headerText}
-          
-          {descriptionText}
-        
+    <>
+       !open && setError(null)}>
+        
+          Error
+          
+            There was an error checking this provider configuration.
+          
+          
{error}

+          
Check your configuration again to use this provider.

+          
+            
+          
+        
+      
+       !open && onClose()}>
+        
+          
+            
+              {getModalIcon()}
+              {headerText}
+            
+            {descriptionText}
+          
 
-        

-          {/* Contains information used to set up each provider */}
-          {/* Only show the form when NOT in delete confirmation mode */}
-          {!showDeleteConfirmation ? (
-            <>
-              {/* Contains information used to set up each provider */}
-              
+          

+            {/* Contains information used to set up each provider */}
+            {/* Only show the form when NOT in delete confirmation mode */}
+            {!showDeleteConfirmation ? (
+              <>
+                {/* Contains information used to set up each provider */}
+                
 
-              {requiredParameters.length > 0 &&
-                provider.metadata.config_keys &&
-                provider.metadata.config_keys.length > 0 && }
-            
-          ) : null}
-        

+                {requiredParameters.length > 0 &&
+                  provider.metadata.config_keys &&
+                  provider.metadata.config_keys.length > 0 && }
+              
+            ) : null}
+          

 
-        
-           {
-              setIsActiveProvider(false);
-              setShowDeleteConfirmation(false);
-            }}
-            canDelete={isConfigured && !isActiveProvider}
-            providerName={provider.metadata.display_name}
-            isActiveProvider={isActiveProvider}
-          />
-        
-      
-    
+          
+             {
+                setIsActiveProvider(false);
+                setShowDeleteConfirmation(false);
+              }}
+              canDelete={isConfigured && !isActiveProvider}
+              providerName={provider.metadata.display_name}
+              isActiveProvider={isActiveProvider}
+            />
+          
+        
+      
+    
   );
 }
diff --git a/ui/desktop/src/components/settings/providers/modal/subcomponents/handlers/DefaultSubmitHandler.tsx b/ui/desktop/src/components/settings/providers/modal/subcomponents/handlers/DefaultSubmitHandler.tsx
index 2c9bb82c15..b5220aa814 100644
--- a/ui/desktop/src/components/settings/providers/modal/subcomponents/handlers/DefaultSubmitHandler.tsx
+++ b/ui/desktop/src/components/settings/providers/modal/subcomponents/handlers/DefaultSubmitHandler.tsx
@@ -1,3 +1,5 @@
+import { checkProvider } from '../../../../../../api';
+
 /**
  * Standalone function to submit provider configuration
  * Useful for components that don't want to use the hook
@@ -19,15 +21,6 @@ export const providerConfigSubmitHandler = async (
 ) => {
   const parameters = provider.metadata.config_keys || [];
 
-  if (parameters.length === 0) {
-    // For zero-config providers, mark them as configured
-    const configKey = `${provider.name}_configured`;
-    await upsertFn(configKey, true, false);
-
-    await upsertFn('GOOSE_PROVIDER', provider.name, false);
-    return;
-  }
-
   const requiredParams = parameters.filter((param) => param.required);
   if (requiredParams.length === 0 && parameters.length > 0) {
     const allOptionalWithDefaults = parameters.every(
@@ -35,8 +28,6 @@ export const providerConfigSubmitHandler = async (
     );
     if (allOptionalWithDefaults) {
       const promises: Promise[] = [];
-      const configKey = `${provider.name}_configured`;
-      promises.push(upsertFn(configKey, true, false));
 
       for (const param of parameters) {
         if (param.default !== undefined) {
@@ -46,39 +37,41 @@ export const providerConfigSubmitHandler = async (
         }
       }
 
-      return Promise.all(promises);
+      await Promise.all(promises);
+      return;
     }
   }
 
   const upsertPromises = parameters.map(
-    (parameter: { name: string; required?: boolean; default?: unknown; secret?: boolean }) => {
-      // Skip parameters that don't have a value and aren't required
+    async (parameter: {
+      name: string;
+      required?: boolean;
+      default?: unknown;
+      secret?: boolean;
+    }) => {
       if (!configValues[parameter.name] && !parameter.required) {
-        return Promise.resolve();
+        return;
       }
 
-      // For required parameters with no value, use the default if available
       const value =
         configValues[parameter.name] !== undefined
           ? configValues[parameter.name]
           : parameter.default;
 
-      // Skip if there's still no value
       if (value === undefined || value === null) {
-        return Promise.resolve();
+        return;
       }
 
-      // Create the provider-specific config key
       const configKey = `${parameter.name}`;
-
-      // Explicitly define is_secret as a boolean (true/false)
       const isSecret = parameter.secret === true;
 
-      // Pass the is_secret flag from the parameter definition
-      return upsertFn(configKey, value, isSecret);
+      await upsertFn(configKey, value, isSecret);
     }
   );
 
-  // Wait for all upsert operations to complete
-  return Promise.all(upsertPromises);
+  await Promise.all(upsertPromises);
+  await checkProvider({
+    body: { provider: provider.name },
+    throwOnError: true,
+  });
 };