From cfa99b3e083446f748f4849c696b93892b68de07 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kyle=20=F0=9F=90=86?= Date: Sun, 20 Jul 2025 20:44:49 -0400 Subject: [PATCH] fix(ui): enable selection of zero-config providers in desktop GUI (#3378) Signed-off-by: Kyle Santiago --- crates/goose-server/src/routes/utils.rs | 22 +++ crates/goose/src/providers/claude_code.rs | 136 +++++++++++++++--- crates/goose/src/providers/gemini_cli.rs | 93 ++++++++++-- documentation/docs/guides/cli-providers.md | 1 + .../components/ModelAndProviderContext.tsx | 15 +- .../models/bottom_bar/ModelsBottomBar.tsx | 32 ++++- .../subcomponents/ProviderSetupActions.tsx | 26 ++-- .../handlers/DefaultSubmitHandler.tsx | 32 +++++ 8 files changed, 313 insertions(+), 44 deletions(-) diff --git a/crates/goose-server/src/routes/utils.rs b/crates/goose-server/src/routes/utils.rs index a6659e5e..94df7df2 100644 --- a/crates/goose-server/src/routes/utils.rs +++ b/crates/goose-server/src/routes/utils.rs @@ -109,6 +109,13 @@ pub fn inspect_keys( pub fn check_provider_configured(metadata: &ProviderMetadata) -> bool { let config = Config::global(); + // Special case: Zero-config providers (no config keys) + if metadata.config_keys.is_empty() { + // Check if the provider has been explicitly configured via the UI + let configured_marker = format!("{}_configured", metadata.name); + return config.get_param::(&configured_marker).is_ok(); + } + // Get all required keys let required_keys: Vec<&ConfigKey> = metadata .config_keys @@ -128,6 +135,21 @@ pub fn check_provider_configured(metadata: &ProviderMetadata) -> bool { return is_set_in_env || is_set_in_config; } + // Special case: If a provider has only optional keys with defaults, + // check if a configuration marker exists + if required_keys.is_empty() && !metadata.config_keys.is_empty() { + let all_optional_with_defaults = metadata + .config_keys + .iter() + .all(|key| !key.required && key.default.is_some()); + + if all_optional_with_defaults { + // Check if the provider has been explicitly configured via the UI + let configured_marker = format!("{}_configured", metadata.name); + return config.get_param::(&configured_marker).is_ok(); + } + } + // For providers with multiple keys or keys without defaults: // Find required keys that don't have default values let required_non_default_keys: Vec<&ConfigKey> = required_keys diff --git a/crates/goose/src/providers/claude_code.rs b/crates/goose/src/providers/claude_code.rs index 563ea89d..4c03af61 100644 --- a/crates/goose/src/providers/claude_code.rs +++ b/crates/goose/src/providers/claude_code.rs @@ -1,6 +1,7 @@ use anyhow::Result; use async_trait::async_trait; use serde_json::{json, Value}; +use std::path::PathBuf; use std::process::Stdio; use tokio::io::{AsyncBufReadExt, BufReader}; use tokio::process::Command; @@ -8,13 +9,14 @@ use tokio::process::Command; use super::base::{ConfigKey, Provider, ProviderMetadata, ProviderUsage, Usage}; use super::errors::ProviderError; use super::utils::emit_debug_trace; +use crate::config::Config; use crate::message::{Message, MessageContent}; use crate::model::ModelConfig; use mcp_core::tool::Tool; -use rmcp::model::Role; +use mcp_core::Role; -pub const CLAUDE_CODE_DEFAULT_MODEL: &str = "default"; -pub const CLAUDE_CODE_KNOWN_MODELS: &[&str] = &["default"]; +pub const CLAUDE_CODE_DEFAULT_MODEL: &str = "claude-3-5-sonnet-latest"; +pub const CLAUDE_CODE_KNOWN_MODELS: &[&str] = &["sonnet", "opus", "claude-3-5-sonnet-latest"]; pub const CLAUDE_CODE_DOC_URL: &str = "https://claude.ai/cli"; @@ -38,7 +40,71 @@ impl ClaudeCodeProvider { .get_param("CLAUDE_CODE_COMMAND") .unwrap_or_else(|_| "claude".to_string()); - Ok(Self { command, model }) + let resolved_command = if !command.contains('/') { + Self::find_claude_executable(&command).unwrap_or(command) + } else { + command + }; + + Ok(Self { + command: resolved_command, + model, + }) + } + + /// 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 } /// Filter out the Extensions section from the system prompt @@ -97,8 +163,13 @@ impl ClaudeCodeProvider { // Convert tool result contents to text let content_text = tool_contents .iter() - .filter_map(|content| content.as_text().map(|t| t.text.clone())) - .collect::>() + .filter_map(|content| match &content.raw { + rmcp::model::RawContent::Text(text_content) => { + Some(text_content.text.as_str()) + } + _ => None, + }) + .collect::>() .join("\n"); content_parts.push(json!({ @@ -215,11 +286,12 @@ impl ClaudeCodeProvider { let message_content = vec![MessageContent::text(combined_text)]; - let response_message = Message::new( - Role::Assistant, - chrono::Utc::now().timestamp(), - message_content, - ); + let response_message = Message { + id: None, + role: Role::Assistant, + created: chrono::Utc::now().timestamp(), + content: message_content, + }; Ok((response_message, usage)) } @@ -261,10 +333,20 @@ impl ClaudeCodeProvider { .arg(messages_json.to_string()) .arg("--system-prompt") .arg(&filtered_system) + .arg("--model") + .arg(&self.model.model_name) .arg("--verbose") .arg("--output-format") .arg("json"); + // Add permission mode based on GOOSE_MODE setting + let config = Config::global(); + if let Ok(goose_mode) = config.get_param::("GOOSE_MODE") { + if goose_mode.as_str() == "auto" { + cmd.arg("--permission-mode").arg("acceptEdits"); + } + } + cmd.stdout(Stdio::piped()).stderr(Stdio::piped()); let mut child = cmd @@ -326,7 +408,7 @@ impl ClaudeCodeProvider { // Extract the first user message text let description = messages .iter() - .find(|m| m.role == rmcp::model::Role::User) + .find(|m| m.role == mcp_core::Role::User) .and_then(|m| { m.content.iter().find_map(|c| match c { MessageContent::Text(text_content) => Some(&text_content.text), @@ -349,11 +431,12 @@ impl ClaudeCodeProvider { println!("================================"); } - let message = Message::new( - rmcp::model::Role::Assistant, - chrono::Utc::now().timestamp(), - vec![MessageContent::text(description.clone())], - ); + let message = Message { + id: None, + role: mcp_core::Role::Assistant, + created: chrono::Utc::now().timestamp(), + content: vec![MessageContent::text(description.clone())], + }; let usage = Usage::default(); @@ -384,8 +467,8 @@ impl Provider for ClaudeCodeProvider { } fn get_model_config(&self) -> ModelConfig { - // Return a custom config with 200K token limit for Claude Code - ModelConfig::new("claude-3-5-sonnet-latest".to_string()).with_context_limit(Some(200_000)) + // Return the model config with appropriate context limit for Claude models + self.model.clone() } #[tracing::instrument( @@ -439,6 +522,19 @@ mod tests { let config = provider.get_model_config(); assert_eq!(config.model_name, "claude-3-5-sonnet-latest"); - assert_eq!(config.context_limit(), 200_000); + // Context limit should be set by the ModelConfig + assert!(config.context_limit() > 0); + } + + #[test] + fn test_permission_mode_flag_construction() { + // Test that in auto mode, the --permission-mode acceptEdits flag is added + std::env::set_var("GOOSE_MODE", "auto"); + + let config = Config::global(); + let goose_mode: String = config.get_param("GOOSE_MODE").unwrap(); + assert_eq!(goose_mode, "auto"); + + std::env::remove_var("GOOSE_MODE"); } } diff --git a/crates/goose/src/providers/gemini_cli.rs b/crates/goose/src/providers/gemini_cli.rs index f3175b7d..8f1f123a 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::path::PathBuf; use std::process::Stdio; use tokio::io::{AsyncBufReadExt, BufReader}; use tokio::process::Command; @@ -13,8 +14,8 @@ use crate::model::ModelConfig; use mcp_core::tool::Tool; use rmcp::model::Role; -pub const GEMINI_CLI_DEFAULT_MODEL: &str = "default"; -pub const GEMINI_CLI_KNOWN_MODELS: &[&str] = &["default"]; +pub const GEMINI_CLI_DEFAULT_MODEL: &str = "gemini-2.5-pro"; +pub const GEMINI_CLI_KNOWN_MODELS: &[&str] = &["gemini-2.5-pro"]; pub const GEMINI_CLI_DOC_URL: &str = "https://ai.google.dev/gemini-api/docs"; @@ -33,9 +34,76 @@ impl Default for GeminiCliProvider { impl GeminiCliProvider { pub fn from_env(model: ModelConfig) -> Result { - let command = "gemini".to_string(); // Fixed command, no configuration needed + let config = crate::config::Config::global(); + let command: String = config + .get_param("GEMINI_CLI_COMMAND") + .unwrap_or_else(|_| "gemini".to_string()); - Ok(Self { command, model }) + let resolved_command = if !command.contains('/') { + Self::find_gemini_executable(&command).unwrap_or(command) + } else { + command + }; + + Ok(Self { + command: resolved_command, + model, + }) + } + + /// 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 } /// Filter out the Extensions section from the system prompt @@ -102,7 +170,11 @@ impl GeminiCliProvider { } let mut cmd = Command::new(&self.command); - cmd.arg("-p").arg(&full_prompt).arg("--yolo"); + cmd.arg("-m") + .arg(&self.model.model_name) + .arg("-p") + .arg(&full_prompt) + .arg("--yolo"); cmd.stdout(Stdio::piped()).stderr(Stdio::piped()); @@ -125,7 +197,7 @@ impl GeminiCliProvider { Ok(0) => break, // EOF Ok(_) => { let trimmed = line.trim(); - if !trimmed.is_empty() { + if !trimmed.is_empty() && !trimmed.starts_with("Loaded cached credentials") { lines.push(trimmed.to_string()); } } @@ -240,8 +312,8 @@ impl Provider for GeminiCliProvider { } fn get_model_config(&self) -> ModelConfig { - // Return a custom config with 1M token limit for Gemini CLI - ModelConfig::new("gemini-1.5-pro".to_string()).with_context_limit(Some(1_000_000)) + // Return the model config with appropriate context limit for Gemini models + self.model.clone() } #[tracing::instrument( @@ -294,7 +366,8 @@ mod tests { let provider = GeminiCliProvider::default(); let config = provider.get_model_config(); - assert_eq!(config.model_name, "gemini-1.5-pro"); - assert_eq!(config.context_limit(), 1_000_000); + assert_eq!(config.model_name, "gemini-2.5-pro"); + // Context limit should be set by the ModelConfig + assert!(config.context_limit() > 0); } } diff --git a/documentation/docs/guides/cli-providers.md b/documentation/docs/guides/cli-providers.md index a1964aa3..0432be1f 100644 --- a/documentation/docs/guides/cli-providers.md +++ b/documentation/docs/guides/cli-providers.md @@ -176,6 +176,7 @@ goose session | Environment Variable | Description | Default | |---------------------|-------------|---------| | `GOOSE_PROVIDER` | Set to `gemini-cli` to use this provider | None | +| `GEMINI_CLI_COMMAND` | Path to the Gemini CLI command | `gemini` | ## How It Works diff --git a/ui/desktop/src/components/ModelAndProviderContext.tsx b/ui/desktop/src/components/ModelAndProviderContext.tsx index 0227bafb..27d3164b 100644 --- a/ui/desktop/src/components/ModelAndProviderContext.tsx +++ b/ui/desktop/src/components/ModelAndProviderContext.tsx @@ -44,7 +44,7 @@ const ModelAndProviderContext = createContext = ({ children }) => { const [currentModel, setCurrentModel] = useState(null); const [currentProvider, setCurrentProvider] = useState(null); - const { read, upsert, getProviders } = useConfig(); + const { read, upsert, getProviders, config } = useConfig(); const changeModel = useCallback( async (model: Model) => { @@ -183,6 +183,19 @@ export const ModelAndProviderProvider: React.FC = refreshCurrentModelAndProvider(); }, [refreshCurrentModelAndProvider]); + // Extract config values for dependency array + const configObj = config as Record; + const gooseModel = configObj?.GOOSE_MODEL; + const gooseProvider = configObj?.GOOSE_PROVIDER; + + // Listen for config changes and refresh when GOOSE_MODEL or GOOSE_PROVIDER changes + useEffect(() => { + // Only refresh if the config has loaded and model/provider values exist + if (config && Object.keys(config).length > 0 && (gooseModel || gooseProvider)) { + refreshCurrentModelAndProvider(); + } + }, [config, gooseModel, gooseProvider, refreshCurrentModelAndProvider]); + const contextValue = useMemo( () => ({ currentModel, diff --git a/ui/desktop/src/components/settings/models/bottom_bar/ModelsBottomBar.tsx b/ui/desktop/src/components/settings/models/bottom_bar/ModelsBottomBar.tsx index 832b1296..120ab68b 100644 --- a/ui/desktop/src/components/settings/models/bottom_bar/ModelsBottomBar.tsx +++ b/ui/desktop/src/components/settings/models/bottom_bar/ModelsBottomBar.tsx @@ -13,6 +13,7 @@ import { } from '../../../ui/dropdown-menu'; import { useCurrentModelInfo } from '../../../BaseChat'; import { useConfig } from '../../../ConfigContext'; +import { getProviderMetadata } from '../modelInterface'; import { Alert } from '../../../alerts'; import BottomMenuAlertPopover from '../../../bottom_menu/BottomMenuAlertPopover'; import { Recipe } from '../../../../recipe'; @@ -42,12 +43,13 @@ export default function ModelsBottomBar({ getCurrentProviderDisplayName, } = useModelAndProvider(); const currentModelInfo = useCurrentModelInfo(); - const { read } = useConfig(); + const { read, getProviders } = useConfig(); const [displayProvider, setDisplayProvider] = useState(null); const [displayModelName, setDisplayModelName] = useState('Select Model'); const [isAddModelModalOpen, setIsAddModelModalOpen] = useState(false); const [isLeadWorkerModalOpen, setIsLeadWorkerModalOpen] = useState(false); const [isLeadWorkerActive, setIsLeadWorkerActive] = useState(false); + const [providerDefaultModel, setProviderDefaultModel] = useState(null); // Save recipe dialog state (like in RecipeEditor.tsx) const [showSaveDialog, setShowSaveDialog] = useState(false); @@ -91,10 +93,6 @@ export default function ModelsBottomBar({ checkLeadWorker(); }; - // Determine which model to display - activeModel takes priority when lead/worker is active - const displayModel = - isLeadWorkerActive && currentModelInfo?.model ? currentModelInfo.model : displayModelName; - // Since currentModelInfo.mode is not working, let's determine mode differently // We'll need to get the lead model and compare it with the current model const [leadModelName, setLeadModelName] = useState(''); @@ -122,6 +120,12 @@ export default function ModelsBottomBar({ : 'worker' : undefined; + // Determine which model to display - activeModel takes priority when lead/worker is active + const displayModel = + isLeadWorkerActive && currentModelInfo?.model + ? currentModelInfo.model + : currentModel || providerDefaultModel || displayModelName; + // Update display provider when current provider changes useEffect(() => { if (currentProvider) { @@ -137,6 +141,24 @@ export default function ModelsBottomBar({ } }, [currentProvider, getCurrentProviderDisplayName, getCurrentModelAndProviderForDisplay]); + // Fetch provider default model when provider changes and no current model + useEffect(() => { + if (currentProvider && !currentModel) { + (async () => { + try { + const metadata = await getProviderMetadata(currentProvider, getProviders); + setProviderDefaultModel(metadata.default_model); + } catch (error) { + console.error('Failed to get provider default model:', error); + setProviderDefaultModel(null); + } + })(); + } else if (currentModel) { + // Clear provider default when we have a current model + setProviderDefaultModel(null); + } + }, [currentProvider, currentModel, getProviders]); + // Update display model name when current model changes useEffect(() => { (async () => { diff --git a/ui/desktop/src/components/settings/providers/modal/subcomponents/ProviderSetupActions.tsx b/ui/desktop/src/components/settings/providers/modal/subcomponents/ProviderSetupActions.tsx index 8fe19e10..0b667589 100644 --- a/ui/desktop/src/components/settings/providers/modal/subcomponents/ProviderSetupActions.tsx +++ b/ui/desktop/src/components/settings/providers/modal/subcomponents/ProviderSetupActions.tsx @@ -116,14 +116,24 @@ export default function ProviderSetupActions({ ) : ( - + <> + + + )} ); 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 933b1e23..3356f4a8 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 @@ -5,6 +5,7 @@ export const DefaultSubmitHandler = async ( upsertFn: (key: string, value: unknown, isSecret: boolean) => Promise, provider: { + name: string; metadata: { config_keys?: Array<{ name: string; @@ -18,6 +19,37 @@ export const DefaultSubmitHandler = 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( + (param) => !param.required && param.default !== undefined + ); + 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) { + const value = + configValues[param.name] !== undefined ? configValues[param.name] : param.default; + promises.push(upsertFn(param.name, value, param.secret === true)); + } + } + + return Promise.all(promises); + } + } + 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