diff --git a/crates/goose-cli/src/commands/configure.rs b/crates/goose-cli/src/commands/configure.rs index 0c9ae1638d..ff47ee1711 100644 --- a/crates/goose-cli/src/commands/configure.rs +++ b/crates/goose-cli/src/commands/configure.rs @@ -583,6 +583,9 @@ pub fn configure_extensions_dialog() -> Result<(), Box> { cliclack::confirm("Would you like to add environment variables?").interact()?; let mut envs = HashMap::new(); + let mut env_keys = Vec::new(); + let config = Config::global(); + if add_env { loop { let key: String = cliclack::input("Environment variable name:") @@ -593,7 +596,18 @@ pub fn configure_extensions_dialog() -> Result<(), Box> { .mask('▪') .interact()?; - envs.insert(key, value); + // Try to store in keychain + let keychain_key = key.to_string(); + match config.set_secret(&keychain_key, Value::String(value.clone())) { + Ok(_) => { + // Successfully stored in keychain, add to env_keys + env_keys.push(keychain_key); + } + Err(_) => { + // Failed to store in keychain, store directly in envs + envs.insert(key, value); + } + } if !cliclack::confirm("Add another environment variable?").interact()? { break; @@ -608,6 +622,7 @@ pub fn configure_extensions_dialog() -> Result<(), Box> { cmd, args, envs: Envs::new(envs), + env_keys, description, timeout: Some(timeout), bundled: None, @@ -671,6 +686,9 @@ pub fn configure_extensions_dialog() -> Result<(), Box> { cliclack::confirm("Would you like to add environment variables?").interact()?; let mut envs = HashMap::new(); + let mut env_keys = Vec::new(); + let config = Config::global(); + if add_env { loop { let key: String = cliclack::input("Environment variable name:") @@ -681,7 +699,18 @@ pub fn configure_extensions_dialog() -> Result<(), Box> { .mask('▪') .interact()?; - envs.insert(key, value); + // Try to store in keychain + let keychain_key = key.to_string(); + match config.set_secret(&keychain_key, Value::String(value.clone())) { + Ok(_) => { + // Successfully stored in keychain, add to env_keys + env_keys.push(keychain_key); + } + Err(_) => { + // Failed to store in keychain, store directly in envs + envs.insert(key, value); + } + } if !cliclack::confirm("Add another environment variable?").interact()? { break; @@ -695,6 +724,7 @@ pub fn configure_extensions_dialog() -> Result<(), Box> { name: name.clone(), uri, envs: Envs::new(envs), + env_keys, description, timeout: Some(timeout), bundled: None, diff --git a/crates/goose-cli/src/session/mod.rs b/crates/goose-cli/src/session/mod.rs index 5431fc8130..8c88598e38 100644 --- a/crates/goose-cli/src/session/mod.rs +++ b/crates/goose-cli/src/session/mod.rs @@ -158,6 +158,7 @@ impl Session { cmd, args: parts.iter().map(|s| s.to_string()).collect(), envs: Envs::new(envs), + env_keys: Vec::new(), description: Some(goose::config::DEFAULT_EXTENSION_DESCRIPTION.to_string()), // TODO: should set timeout timeout: Some(goose::config::DEFAULT_EXTENSION_TIMEOUT), @@ -190,6 +191,7 @@ impl Session { name, uri: extension_url, envs: Envs::new(HashMap::new()), + env_keys: Vec::new(), description: Some(goose::config::DEFAULT_EXTENSION_DESCRIPTION.to_string()), // TODO: should set timeout timeout: Some(goose::config::DEFAULT_EXTENSION_TIMEOUT), diff --git a/crates/goose-server/src/openapi.rs b/crates/goose-server/src/openapi.rs index 33607ade1b..d8064c3cd0 100644 --- a/crates/goose-server/src/openapi.rs +++ b/crates/goose-server/src/openapi.rs @@ -12,6 +12,7 @@ use utoipa::OpenApi; #[derive(OpenApi)] #[openapi( paths( + super::routes::config_management::backup_config, super::routes::config_management::init_config, super::routes::config_management::upsert_config, super::routes::config_management::remove_config, diff --git a/crates/goose-server/src/routes/config_management.rs b/crates/goose-server/src/routes/config_management.rs index bb09a2e5a8..7a78bc3909 100644 --- a/crates/goose-server/src/routes/config_management.rs +++ b/crates/goose-server/src/routes/config_management.rs @@ -292,7 +292,9 @@ pub async fn read_all_config( let config = Config::global(); // Load values from config file - let values = config.load_values().unwrap_or_default(); + let values = config + .load_values() + .map_err(|_| StatusCode::UNPROCESSABLE_ENTITY)?; Ok(Json(ConfigResponse { config: values })) } @@ -429,6 +431,54 @@ pub async fn upsert_permissions( Ok(Json("Permissions updated successfully".to_string())) } +use etcetera::{choose_app_strategy, AppStrategy, AppStrategyArgs}; +use once_cell::sync::Lazy; +pub static APP_STRATEGY: Lazy = Lazy::new(|| AppStrategyArgs { + top_level_domain: "Block".to_string(), + author: "Block".to_string(), + app_name: "goose".to_string(), +}); + +#[utoipa::path( + post, + path = "/config/backup", + responses( + (status = 200, description = "Config file backed up", body = String), + (status = 500, description = "Internal server error") + ) +)] +pub async fn backup_config( + State(state): State, + headers: HeaderMap, +) -> Result, StatusCode> { + verify_secret_key(&headers, &state)?; + + let config_dir = choose_app_strategy(APP_STRATEGY.clone()) + .expect("goose requires a home dir") + .config_dir(); + + let config_path = config_dir.join("config.yaml"); + + if config_path.exists() { + let file_name = config_path + .file_name() + .ok_or(StatusCode::INTERNAL_SERVER_ERROR)?; + + // Append ".bak" to the file name + let mut backup_name = file_name.to_os_string(); + backup_name.push(".bak"); + + // Construct the new path with the same parent directory + let backup = config_path.with_file_name(backup_name); + match std::fs::rename(&config_path, &backup) { + Ok(_) => Ok(Json(format!("Moved {:?} to {:?}", config_path, backup))), + Err(_) => Err(StatusCode::INTERNAL_SERVER_ERROR), + } + } else { + Err(StatusCode::INTERNAL_SERVER_ERROR) + } +} + pub fn routes(state: AppState) -> Router { Router::new() .route("/config", get(read_all_config)) @@ -440,6 +490,7 @@ pub fn routes(state: AppState) -> Router { .route("/config/extensions/:name", delete(remove_extension)) .route("/config/providers", get(providers)) .route("/config/init", post(init_config)) + .route("/config/backup", post(backup_config)) .route("/config/permissions", post(upsert_permissions)) .with_state(state) } diff --git a/crates/goose-server/src/routes/extension.rs b/crates/goose-server/src/routes/extension.rs index 107e5e70e3..71489e6eec 100644 --- a/crates/goose-server/src/routes/extension.rs +++ b/crates/goose-server/src/routes/extension.rs @@ -1,14 +1,10 @@ -use std::collections::HashMap; use std::env; use std::path::Path; use std::sync::OnceLock; use crate::state::AppState; use axum::{extract::State, routing::post, Json, Router}; -use goose::{ - agents::{extension::Envs, ExtensionConfig}, - config::Config, -}; +use goose::agents::{extension::Envs, ExtensionConfig}; use http::{HeaderMap, StatusCode}; use serde::{Deserialize, Serialize}; use tracing; @@ -24,6 +20,9 @@ enum ExtensionConfigRequest { name: String, /// The URI endpoint for the SSE extension. uri: String, + #[serde(default)] + /// Map of environment variable key to values. + envs: Envs, /// List of environment variable keys. The server will fetch their values from the keyring. #[serde(default)] env_keys: Vec, @@ -39,6 +38,9 @@ enum ExtensionConfigRequest { /// Arguments for the command. #[serde(default)] args: Vec, + #[serde(default)] + /// Map of environment variable key to values. + envs: Envs, /// List of environment variable keys. The server will fetch their values from the keyring. #[serde(default)] env_keys: Vec, @@ -162,55 +164,28 @@ async fn add_extension( } } - // Load the configuration - let config = Config::global(); - - // Initialize a vector to collect any missing keys. - let mut missing_keys = Vec::new(); - // Construct ExtensionConfig with Envs populated from keyring based on provided env_keys. let extension_config: ExtensionConfig = match request { ExtensionConfigRequest::Sse { name, uri, + envs, env_keys, timeout, - } => { - let mut env_map = HashMap::new(); - for key in env_keys { - match config.get_secret(&key) { - Ok(value) => { - env_map.insert(key, value); - } - Err(_) => { - missing_keys.push(key); - } - } - } - - if !missing_keys.is_empty() { - return Ok(Json(ExtensionResponse { - error: true, - message: Some(format!( - "Missing secrets for keys: {}", - missing_keys.join(", ") - )), - })); - } - - ExtensionConfig::Sse { - name, - uri, - envs: Envs::new(env_map), - description: None, - timeout, - bundled: None, - } - } + } => ExtensionConfig::Sse { + name, + uri, + envs, + env_keys, + description: None, + timeout, + bundled: None, + }, ExtensionConfigRequest::Stdio { name, cmd, args, + envs, env_keys, timeout, } => { @@ -226,34 +201,13 @@ async fn add_extension( // })); // } - let mut env_map = HashMap::new(); - for key in env_keys { - match config.get_secret(&key) { - Ok(value) => { - env_map.insert(key, value); - } - Err(_) => { - missing_keys.push(key); - } - } - } - - if !missing_keys.is_empty() { - return Ok(Json(ExtensionResponse { - error: true, - message: Some(format!( - "Missing secrets for keys: {}", - missing_keys.join(", ") - )), - })); - } - ExtensionConfig::Stdio { name, cmd, args, description: None, - envs: Envs::new(env_map), + envs, + env_keys, timeout, bundled: None, } diff --git a/crates/goose/src/agents/extension.rs b/crates/goose/src/agents/extension.rs index dbbea2d0a1..ae00d4793f 100644 --- a/crates/goose/src/agents/extension.rs +++ b/crates/goose/src/agents/extension.rs @@ -24,6 +24,8 @@ pub enum ExtensionError { Transport(#[from] mcp_client::transport::Error), #[error("Environment variable `{0}` is not allowed to be overridden.")] InvalidEnvVar(String), + #[error("Error during extension setup: {0}")] + SetupError(String), #[error("Join error occurred during task execution: {0}")] TaskJoinError(#[from] tokio::task::JoinError), } @@ -128,6 +130,8 @@ pub enum ExtensionConfig { uri: String, #[serde(default)] envs: Envs, + #[serde(default)] + env_keys: Vec, description: Option, // NOTE: set timeout to be optional for compatibility. // However, new configurations should include this field. @@ -145,6 +149,8 @@ pub enum ExtensionConfig { args: Vec, #[serde(default)] envs: Envs, + #[serde(default)] + env_keys: Vec, timeout: Option, description: Option, /// Whether this extension is bundled with Goose @@ -194,6 +200,7 @@ impl ExtensionConfig { name: name.into(), uri: uri.into(), envs: Envs::default(), + env_keys: Vec::new(), description: Some(description.into()), timeout: Some(timeout.into()), bundled: None, @@ -211,6 +218,7 @@ impl ExtensionConfig { cmd: cmd.into(), args: vec![], envs: Envs::default(), + env_keys: Vec::new(), description: Some(description.into()), timeout: Some(timeout.into()), bundled: None, @@ -227,6 +235,7 @@ impl ExtensionConfig { name, cmd, envs, + env_keys, timeout, description, bundled, @@ -235,6 +244,7 @@ impl ExtensionConfig { name, cmd, envs, + env_keys, args: args.into_iter().map(Into::into).collect(), description, timeout, diff --git a/crates/goose/src/agents/extension_manager.rs b/crates/goose/src/agents/extension_manager.rs index ccd3364f26..11de5ce77b 100644 --- a/crates/goose/src/agents/extension_manager.rs +++ b/crates/goose/src/agents/extension_manager.rs @@ -10,10 +10,11 @@ use std::sync::LazyLock; use std::time::Duration; use tokio::sync::Mutex; use tokio::task; -use tracing::debug; +use tracing::{debug, error, warn}; use super::extension::{ExtensionConfig, ExtensionError, ExtensionInfo, ExtensionResult, ToolInfo}; -use crate::config::ExtensionConfigManager; +use crate::agents::extension::Envs; +use crate::config::{Config, ExtensionConfigManager}; use crate::prompt_template; use mcp_client::client::{ClientCapabilities, ClientInfo, McpClient, McpClientTrait}; use mcp_client::transport::{SseTransport, StdioTransport, Transport}; @@ -113,11 +114,74 @@ impl ExtensionManager { // TODO IMPORTANT need to ensure this times out if the extension command is broken! pub async fn add_extension(&mut self, config: ExtensionConfig) -> ExtensionResult<()> { let sanitized_name = normalize(config.key().to_string()); + + /// Helper function to merge environment variables from direct envs and keychain-stored env_keys + async fn merge_environments( + envs: &Envs, + env_keys: &[String], + ext_name: &str, + ) -> Result, ExtensionError> { + let mut all_envs = envs.get_env(); + let config_instance = Config::global(); + + for key in env_keys { + // If the Envs payload already contains the key, prefer that value + // over looking into the keychain/secret store + if all_envs.contains_key(key) { + continue; + } + + match config_instance.get(key, true) { + Ok(value) => { + if value.is_null() { + warn!( + key = %key, + ext_name = %ext_name, + "Secret key not found in config (returned null)." + ); + continue; + } + + // Try to get string value + if let Some(str_val) = value.as_str() { + all_envs.insert(key.clone(), str_val.to_string()); + } else { + warn!( + key = %key, + ext_name = %ext_name, + value_type = %value.get("type").and_then(|t| t.as_str()).unwrap_or("unknown"), + "Secret value is not a string; skipping." + ); + } + } + Err(e) => { + error!( + key = %key, + ext_name = %ext_name, + error = %e, + "Failed to fetch secret from config." + ); + return Err(ExtensionError::SetupError(format!( + "Failed to fetch secret '{}' from config: {}", + key, e + ))); + } + } + } + + Ok(all_envs) + } + let mut client: Box = match &config { ExtensionConfig::Sse { - uri, envs, timeout, .. + uri, + envs, + env_keys, + timeout, + .. } => { - let transport = SseTransport::new(uri, envs.get_env()); + let all_envs = merge_environments(envs, env_keys, &sanitized_name).await?; + let transport = SseTransport::new(uri, all_envs); let handle = transport.start().await?; let service = McpService::with_timeout( handle, @@ -131,10 +195,12 @@ impl ExtensionManager { cmd, args, envs, + env_keys, timeout, .. } => { - let transport = StdioTransport::new(cmd, args.to_vec(), envs.get_env()); + let all_envs = merge_environments(envs, env_keys, &sanitized_name).await?; + let transport = StdioTransport::new(cmd, args.to_vec(), all_envs); let handle = transport.start().await?; let service = McpService::with_timeout( handle, @@ -150,7 +216,6 @@ impl ExtensionManager { timeout, bundled: _, } => { - // For builtin extensions, we run the current executable with mcp and extension name let cmd = std::env::current_exe() .expect("should find the current executable") .to_str() @@ -185,19 +250,16 @@ impl ExtensionManager { .await .map_err(|e| ExtensionError::Initialization(config.clone(), e))?; - // Store instructions if provided if let Some(instructions) = init_result.instructions { self.instructions .insert(sanitized_name.clone(), instructions); } - // if the server is capable if resources we track it if init_result.capabilities.resources.is_some() { self.resource_capable_extensions .insert(sanitized_name.clone()); } - // Store the client using the provided name self.clients .insert(sanitized_name.clone(), Arc::new(Mutex::new(client))); diff --git a/crates/goose/src/config/extensions.rs b/crates/goose/src/config/extensions.rs index a5a688d70d..9d9147eba1 100644 --- a/crates/goose/src/config/extensions.rs +++ b/crates/goose/src/config/extensions.rs @@ -126,9 +126,7 @@ impl ExtensionConfigManager { /// Get all extensions and their configurations pub fn get_all() -> Result> { let config = Config::global(); - let extensions: HashMap = config - .get_param("extensions") - .unwrap_or_else(|_| HashMap::new()); + let extensions: HashMap = config.get_param("extensions")?; Ok(Vec::from_iter(extensions.values().cloned())) } diff --git a/ui/desktop/openapi.json b/ui/desktop/openapi.json index 00207ddf58..050e40fdd9 100644 --- a/ui/desktop/openapi.json +++ b/ui/desktop/openapi.json @@ -77,6 +77,29 @@ } } }, + "/config/backup": { + "post": { + "tags": [ + "super::routes::config_management" + ], + "operationId": "backup_config", + "responses": { + "200": { + "description": "Config file backed up", + "content": { + "text/plain": { + "schema": { + "type": "string" + } + } + } + }, + "500": { + "description": "Internal server error" + } + } + } + }, "/config/extensions": { "get": { "tags": [ @@ -466,6 +489,12 @@ "type": "string", "nullable": true }, + "env_keys": { + "type": "array", + "items": { + "type": "string" + } + }, "envs": { "$ref": "#/components/schemas/Envs" }, @@ -518,6 +547,12 @@ "type": "string", "nullable": true }, + "env_keys": { + "type": "array", + "items": { + "type": "string" + } + }, "envs": { "$ref": "#/components/schemas/Envs" }, diff --git a/ui/desktop/src/App.tsx b/ui/desktop/src/App.tsx index 7e9a62f3cc..51bb773182 100644 --- a/ui/desktop/src/App.tsx +++ b/ui/desktop/src/App.tsx @@ -34,7 +34,7 @@ import { addExtension as addExtensionDirect, FullExtensionConfig } from './exten import 'react-toastify/dist/ReactToastify.css'; import { useConfig, MalformedConfigError } from './components/ConfigContext'; import { addExtensionFromDeepLink as addExtensionFromDeepLinkV2 } from './components/settings_v2/extensions'; -import { initConfig } from './api/sdk.gen'; +import { backupConfig, initConfig, readAllConfig } from './api/sdk.gen'; import PermissionSettingsView from './components/settings_v2/permission/PermissionSetting'; // Views and their options @@ -240,7 +240,7 @@ export default function App() { console.log('Finished enabling bot config extensions'); }; - const enableRecipeConfigExtensionsV2 = useCallback( + const _enableRecipeConfigExtensionsV2 = useCallback( async (extensions: FullExtensionConfig[]) => { if (!extensions?.length) { console.log('No extensions to enable from bot config'); @@ -299,9 +299,25 @@ export default function App() { const initializeApp = async () => { try { - // Initialize config first + // checks if there is a config, and if not creates it await initConfig(); + // now try to read config, if we fail and are migrating backup, then re-init config + try { + await readAllConfig({ throwOnError: true }); + } catch (error) { + // NOTE: we do this check here and in providerUtils.ts, be sure to clean up both in the future + const configVersion = localStorage.getItem('configVersion'); + const shouldMigrateExtensions = !configVersion || parseInt(configVersion, 10) < 3; + if (shouldMigrateExtensions) { + await backupConfig({ throwOnError: true }); + await initConfig(); + } else { + // if we've migrated throw this back up + throw new Error('Unable to read config file, it may be malformed'); + } + } + // note: if in a non recipe session, recipeConfig is undefined, otherwise null if error if (recipeConfig === null) { setFatalError('Cannot read recipe config. Please check the deeplink and try again.'); @@ -309,10 +325,10 @@ export default function App() { } // Handle bot config extensions first - if (recipeConfig?.extensions?.length > 0 && viewType != 'recipeEditor') { - console.log('Found extensions in bot config:', recipeConfig.extensions); - await enableRecipeConfigExtensionsV2(recipeConfig.extensions); - } + // if (recipeConfig?.extensions?.length > 0 && viewType != 'recipeEditor') { + // console.log('Found extensions in bot config:', recipeConfig.extensions); + // await enableRecipeConfigExtensionsV2(recipeConfig.extensions); + // } const config = window.electron.getConfig(); diff --git a/ui/desktop/src/api/sdk.gen.ts b/ui/desktop/src/api/sdk.gen.ts index 5cfb61fb85..048c09fc82 100644 --- a/ui/desktop/src/api/sdk.gen.ts +++ b/ui/desktop/src/api/sdk.gen.ts @@ -1,7 +1,7 @@ // This file is auto-generated by @hey-api/openapi-ts import type { Options as ClientOptions, TDataShape, Client } from '@hey-api/client-fetch'; -import type { GetToolsData, GetToolsResponse, ReadAllConfigData, ReadAllConfigResponse, GetExtensionsData, GetExtensionsResponse, AddExtensionData, AddExtensionResponse, RemoveExtensionData, RemoveExtensionResponse, InitConfigData, InitConfigResponse, UpsertPermissionsData, UpsertPermissionsResponse, ProvidersData, ProvidersResponse2, ReadConfigData, RemoveConfigData, RemoveConfigResponse, UpsertConfigData, UpsertConfigResponse, ConfirmPermissionData } from './types.gen'; +import type { GetToolsData, GetToolsResponse, ReadAllConfigData, ReadAllConfigResponse, BackupConfigData, BackupConfigResponse, GetExtensionsData, GetExtensionsResponse, AddExtensionData, AddExtensionResponse, RemoveExtensionData, RemoveExtensionResponse, InitConfigData, InitConfigResponse, UpsertPermissionsData, UpsertPermissionsResponse, ProvidersData, ProvidersResponse2, ReadConfigData, RemoveConfigData, RemoveConfigResponse, UpsertConfigData, UpsertConfigResponse, ConfirmPermissionData } from './types.gen'; import { client as _heyApiClient } from './client.gen'; export type Options = ClientOptions & { @@ -32,6 +32,13 @@ export const readAllConfig = (options?: Op }); }; +export const backupConfig = (options?: Options) => { + return (options?.client ?? _heyApiClient).post({ + url: '/config/backup', + ...options + }); +}; + export const getExtensions = (options?: Options) => { return (options?.client ?? _heyApiClient).get({ url: '/config/extensions', diff --git a/ui/desktop/src/api/types.gen.ts b/ui/desktop/src/api/types.gen.ts index f8d97be520..873d78cc68 100644 --- a/ui/desktop/src/api/types.gen.ts +++ b/ui/desktop/src/api/types.gen.ts @@ -29,6 +29,7 @@ export type ExtensionConfig = { */ bundled?: boolean | null; description?: string | null; + env_keys?: Array; envs?: Envs; /** * The name used to identify this extension @@ -45,6 +46,7 @@ export type ExtensionConfig = { bundled?: boolean | null; cmd: string; description?: string | null; + env_keys?: Array; envs?: Envs; /** * The name used to identify this extension @@ -327,6 +329,29 @@ export type ReadAllConfigResponses = { export type ReadAllConfigResponse = ReadAllConfigResponses[keyof ReadAllConfigResponses]; +export type BackupConfigData = { + body?: never; + path?: never; + query?: never; + url: '/config/backup'; +}; + +export type BackupConfigErrors = { + /** + * Internal server error + */ + 500: unknown; +}; + +export type BackupConfigResponses = { + /** + * Config file backed up + */ + 200: string; +}; + +export type BackupConfigResponse = BackupConfigResponses[keyof BackupConfigResponses]; + export type GetExtensionsData = { body?: never; path?: never; diff --git a/ui/desktop/src/components/RecipeEditor.tsx b/ui/desktop/src/components/RecipeEditor.tsx index 44d5777cdb..1713f73344 100644 --- a/ui/desktop/src/components/RecipeEditor.tsx +++ b/ui/desktop/src/components/RecipeEditor.tsx @@ -7,10 +7,10 @@ import Back from './icons/Back'; import { Bars } from './icons/Bars'; import { Geese } from './icons/Geese'; import Copy from './icons/Copy'; +import { Check } from 'lucide-react'; import { useConfig } from './ConfigContext'; import { FixedExtensionEntry } from './ConfigContext'; -import ExtensionList from './settings_v2/extensions/subcomponents/ExtensionList'; -import { Check } from 'lucide-react'; +// import ExtensionList from './settings_v2/extensions/subcomponents/ExtensionList'; interface RecipeEditorProps { config?: Recipe; @@ -30,11 +30,11 @@ export default function RecipeEditor({ config }: RecipeEditorProps) { const [instructions, setInstructions] = useState(config?.instructions || ''); const [activities, setActivities] = useState(config?.activities || []); const [extensionOptions, setExtensionOptions] = useState([]); - const [copied, setCopied] = useState(false); const [extensionsLoaded, setExtensionsLoaded] = useState(false); + const [copied, setCopied] = useState(false); // Initialize selected extensions for the recipe from config or localStorage - const [recipeExtensions, setRecipeExtensions] = useState(() => { + const [recipeExtensions] = useState(() => { // First try to get from localStorage const stored = localStorage.getItem('recipe_editor_extensions'); if (stored) { @@ -95,20 +95,20 @@ export default function RecipeEditor({ config }: RecipeEditorProps) { // eslint-disable-next-line react-hooks/exhaustive-deps }, [recipeExtensions, extensionsLoaded]); - const handleExtensionToggle = (extension: FixedExtensionEntry) => { - console.log('Toggling extension:', extension.name); - setRecipeExtensions((prev) => { - const isSelected = prev.includes(extension.name); - const newState = isSelected - ? prev.filter((extName) => extName !== extension.name) - : [...prev, extension.name]; + // const handleExtensionToggle = (extension: FixedExtensionEntry) => { + // console.log('Toggling extension:', extension.name); + // setRecipeExtensions((prev) => { + // const isSelected = prev.includes(extension.name); + // const newState = isSelected + // ? prev.filter((extName) => extName !== extension.name) + // : [...prev, extension.name]; - // Persist to localStorage - localStorage.setItem('recipe_editor_extensions', JSON.stringify(newState)); + // // Persist to localStorage + // localStorage.setItem('recipe_editor_extensions', JSON.stringify(newState)); - return newState; - }); - }; + // return newState; + // }); + // }; const handleAddActivity = () => { if (newActivity.trim()) { @@ -143,14 +143,9 @@ export default function RecipeEditor({ config }: RecipeEditorProps) { // Create a clean copy of the extension configuration const cleanExtension = { ...extension }; delete cleanExtension.enabled; - - // If the extension has env_keys, preserve keys but clear values - if (cleanExtension.env_keys) { - cleanExtension.env_keys = Object.fromEntries( - Object.keys(cleanExtension.env_keys).map((key) => [key, '']) - ); - } - + // Remove legacy envs which could potentially include secrets + // env_keys will work but rely on the end user having setup those keys themselves + delete cleanExtension.envs; return cleanExtension; }) .filter(Boolean) as FullExtensionConfig[], @@ -173,31 +168,13 @@ export default function RecipeEditor({ config }: RecipeEditorProps) { return Object.keys(newErrors).length === 0; }; - const handleOpenAgent = () => { - if (validateForm()) { - const updatedConfig = getCurrentConfig(); - // Clear stored extensions when submitting - localStorage.removeItem('recipe_editor_extensions'); - window.electron.createChatWindow( - undefined, - undefined, - undefined, - undefined, - updatedConfig, - undefined - ); - } - }; - const deeplink = generateDeepLink(getCurrentConfig()); const handleCopy = () => { - // Copy the text to the clipboard navigator.clipboard .writeText(deeplink) .then(() => { - setCopied(true); // Show the check mark - // Reset to normal after 2 seconds (2000 milliseconds) + setCopied(true); setTimeout(() => setCopied(false), 2000); }) .catch((err) => { @@ -293,30 +270,30 @@ export default function RecipeEditor({ config }: RecipeEditorProps) { ); - case 'extensions': - return ( -
- -
- -
-
-

Extensions

-

Select extensions to bundle in the recipe

-
- {extensionsLoaded ? ( - - ) : ( -
Loading extensions...
- )} -
- ); + // case 'extensions': + // return ( + //
+ // + //
+ // + //
+ //
+ //

Extensions

+ //

Select extensions to bundle in the recipe

+ //
+ // {extensionsLoaded ? ( + // + // ) : ( + //
Loading extensions...
+ // )} + //
+ // ); default: return ( @@ -385,7 +362,7 @@ export default function RecipeEditor({ config }: RecipeEditorProps) { - + */} {/* Deep Link Display */} -
- {deeplink} - +
+
- {copied ? ( - - ) : ( - - )} - + {deeplink} +
{/* Action Buttons */}
- + )} + {(envVar.value !== '••••••••' || envVar.isEdited) && ( +
/* Empty div to maintain grid spacing */ + )} @@ -140,13 +176,15 @@ export default function EnvVarsSection({ invalidFields.value && 'border-red-500 focus:border-red-500' )} /> - +
+ +
{validationError &&
{validationError}
} diff --git a/ui/desktop/src/components/settings_v2/extensions/modal/ExtensionModal.tsx b/ui/desktop/src/components/settings_v2/extensions/modal/ExtensionModal.tsx index d6d757229d..17e2d3ce68 100644 --- a/ui/desktop/src/components/settings_v2/extensions/modal/ExtensionModal.tsx +++ b/ui/desktop/src/components/settings_v2/extensions/modal/ExtensionModal.tsx @@ -7,6 +7,7 @@ import ExtensionConfigFields from './ExtensionConfigFields'; import { PlusIcon, Edit, Trash2, AlertTriangle } from 'lucide-react'; import ExtensionInfoFields from './ExtensionInfoFields'; import ExtensionTimeoutField from './ExtensionTimeoutField'; +import { upsertConfig } from '../../../../api/sdk.gen'; interface ExtensionModalProps { title: string; @@ -34,7 +35,7 @@ export default function ExtensionModal({ const handleAddEnvVar = (key: string, value: string) => { setFormData({ ...formData, - envVars: [...formData.envVars, { key, value }], + envVars: [...formData.envVars, { key, value, isEdited: true }], }); }; @@ -50,12 +51,35 @@ export default function ExtensionModal({ const handleEnvVarChange = (index: number, field: 'key' | 'value', value: string) => { const newEnvVars = [...formData.envVars]; newEnvVars[index][field] = value; + + // Mark as edited if it's a value change + if (field === 'value') { + newEnvVars[index].isEdited = true; + } + setFormData({ ...formData, envVars: newEnvVars, }); }; + // Function to store a secret value + const storeSecret = async (key: string, value: string) => { + try { + await upsertConfig({ + body: { + is_secret: true, + key: key, + value: value, + }, + }); + return true; + } catch (error) { + console.error('Failed to store secret:', error); + return false; + } + }; + // Function to determine which icon to display with proper styling const getModalIcon = () => { if (showDeleteConfirmation) { @@ -104,23 +128,36 @@ export default function ExtensionModal({ return isNameValid() && isConfigValid() && isEnvVarsValid() && isTimeoutValid(); }; - // Handle submit with validation - const handleSubmit = () => { + // Handle submit with validation and secret storage + const handleSubmit = async () => { setSubmitAttempted(true); if (isFormValid()) { - const dataToSubmit = { ...formData }; + // Only store env vars that have been edited (which includes new) + const secretPromises = formData.envVars + .filter((envVar) => envVar.isEdited) + .map(({ key, value }) => storeSecret(key, value)); - // Convert the timeout to a number if it's a string - if (typeof dataToSubmit.timeout === 'string') { - dataToSubmit.timeout = Number(dataToSubmit.timeout); + try { + // Wait for all secrets to be stored + const results = await Promise.all(secretPromises); + + if (results.every((success) => success)) { + // Convert timeout to number if needed + const dataToSubmit = { + ...formData, + timeout: + typeof formData.timeout === 'string' ? Number(formData.timeout) : formData.timeout, + }; + onSubmit(dataToSubmit); + onClose(); + } else { + console.error('Failed to store one or more secrets'); + } + } catch (error) { + console.error('Error during submission:', error); } - - // Submit the data with converted timeout - onSubmit(dataToSubmit); - onClose(); // Only close the modal if the form is valid } else { - // Optional: Add some feedback that validation failed (like a toast notification) console.log('Form validation failed'); } }; @@ -241,7 +278,7 @@ export default function ExtensionModal({ envVars={formData.envVars} onAdd={handleAddEnvVar} onRemove={handleRemoveEnvVar} - onChange={Object.assign(handleEnvVarChange, { setSubmitAttempted })} + onChange={handleEnvVarChange} submitAttempted={submitAttempted} /> diff --git a/ui/desktop/src/components/settings_v2/extensions/utils.ts b/ui/desktop/src/components/settings_v2/extensions/utils.ts index f06faa2649..5626d9cc53 100644 --- a/ui/desktop/src/components/settings_v2/extensions/utils.ts +++ b/ui/desktop/src/components/settings_v2/extensions/utils.ts @@ -26,7 +26,11 @@ export interface ExtensionFormData { endpoint?: string; enabled: boolean; timeout?: number; - envVars: { key: string; value: string }[]; + envVars: { + key: string; + value: string; + isEdited?: boolean; + }[]; } export function getDefaultFormData(): ExtensionFormData { @@ -46,13 +50,30 @@ export function extensionToFormData(extension: FixedExtensionEntry): ExtensionFo // Type guard: Check if 'envs' property exists for this variant const hasEnvs = extension.type === 'sse' || extension.type === 'stdio'; - const envVars = - hasEnvs && extension.envs - ? Object.entries(extension.envs).map(([key, value]) => ({ - key, - value: value as string, - })) - : []; + // Handle both envs (legacy) and env_keys (new secrets) + let envVars = []; + + // Add legacy envs with their values + if (hasEnvs && extension.envs) { + envVars.push( + ...Object.entries(extension.envs).map(([key, value]) => ({ + key, + value: value as string, + isEdited: true, // We want to submit legacy values as secrets to migrate forward + })) + ); + } + + // Add env_keys with placeholder values + if (hasEnvs && extension.env_keys) { + envVars.push( + ...extension.env_keys.map((key) => ({ + key, + value: '••••••••', // Placeholder for secret values + isEdited: false, // Mark as not edited initially + })) + ); + } return { name: extension.name, @@ -68,15 +89,8 @@ export function extensionToFormData(extension: FixedExtensionEntry): ExtensionFo } export function createExtensionConfig(formData: ExtensionFormData): ExtensionConfig { - const envs = formData.envVars.reduce( - (acc, { key, value }) => { - if (key) { - acc[key] = value; - } - return acc; - }, - {} as Record - ); + // Extract just the keys from env vars + const env_keys = formData.envVars.map(({ key }) => key).filter((key) => key.length > 0); if (formData.type === 'stdio') { // we put the cmd + args all in the form cmd field but need to split out into cmd + args @@ -89,7 +103,7 @@ export function createExtensionConfig(formData: ExtensionFormData): ExtensionCon cmd: cmd, args: args, timeout: formData.timeout, - ...(Object.keys(envs).length > 0 ? { envs } : {}), + ...(env_keys.length > 0 ? { env_keys } : {}), }; } else if (formData.type === 'sse') { return { @@ -97,8 +111,8 @@ export function createExtensionConfig(formData: ExtensionFormData): ExtensionCon name: formData.name, description: formData.description, timeout: formData.timeout, - uri: formData.endpoint, // Assuming endpoint maps to uri for SSE type - ...(Object.keys(envs).length > 0 ? { envs } : {}), + uri: formData.endpoint, + ...(env_keys.length > 0 ? { env_keys } : {}), }; } else { // For other types diff --git a/ui/desktop/src/utils/providerUtils.ts b/ui/desktop/src/utils/providerUtils.ts index 3d5c064708..9bf728c574 100644 --- a/ui/desktop/src/utils/providerUtils.ts +++ b/ui/desktop/src/utils/providerUtils.ts @@ -94,8 +94,8 @@ There may be (but not always) some tools mentioned in the instructions which you * * @param addExtension Function to add extension to config.yaml */ -export const migrateExtensionsToSettingsV2 = async () => { - console.log('need to perform extension migration'); +export const migrateExtensionsToSettingsV3 = async () => { + console.log('need to perform extension migration v3'); const userSettingsStr = localStorage.getItem('user_settings'); let localStorageExtensions: FullExtensionConfig[] = []; @@ -140,8 +140,8 @@ export const migrateExtensionsToSettingsV2 = async () => { } if (migrationErrors.length === 0) { - localStorage.setItem('configVersion', '2'); - console.log('Extension migration complete. Config version set to 2.'); + localStorage.setItem('configVersion', '3'); + console.log('Extension migration complete. Config version set to 3.'); } else { const errorSummaryStr = migrationErrors .map(({ name, error }) => `- ${name}: ${JSON.stringify(error)}`) @@ -209,11 +209,11 @@ export const initializeSystem = async ( // NOTE: remove when we want to stop migration logic // Check if we need to migrate extensions from localStorage to config.yaml const configVersion = localStorage.getItem('configVersion'); - const shouldMigrateExtensions = !configVersion || parseInt(configVersion, 10) < 2; + const shouldMigrateExtensions = !configVersion || parseInt(configVersion, 10) < 3; console.log(`shouldMigrateExtensions is ${shouldMigrateExtensions}`); if (shouldMigrateExtensions) { - await migrateExtensionsToSettingsV2(); + await migrateExtensionsToSettingsV3(); } /* NOTE: