From 899762a422be3f5fd7c98b513b4234629a47f981 Mon Sep 17 00:00:00 2001 From: Kalvin C Date: Wed, 29 Apr 2026 09:05:32 -0700 Subject: [PATCH] refactor: update goose2 credential management behind provider-scoped ACP/core API (#8887) Signed-off-by: morgmart <98432065+morgmart@users.noreply.github.com> Co-authored-by: morgmart <98432065+morgmart@users.noreply.github.com> --- crates/goose-sdk/src/custom_requests.rs | 90 +++ crates/goose/acp-meta.json | 20 + crates/goose/acp-schema.json | 232 ++++++ crates/goose/src/acp/server.rs | 675 +++++++++++++++--- crates/goose/src/config/base.rs | 161 +++-- crates/goose/src/providers/inventory/mod.rs | 268 ++++++- .../acp_secret_cache_invalidation_test.rs | 314 ++++++++ documentation/docs/guides/config-files.md | 2 +- ui/goose2/AGENTS.md | 7 +- ui/goose2/src-tauri/Cargo.lock | 87 --- ui/goose2/src-tauri/Cargo.toml | 1 - .../src-tauri/src/commands/credentials.rs | 50 -- ui/goose2/src-tauri/src/commands/mod.rs | 1 - ui/goose2/src-tauri/src/lib.rs | 9 +- .../src-tauri/src/services/goose_config.rs | 452 ------------ ui/goose2/src-tauri/src/services/mod.rs | 2 - .../src-tauri/src/services/provider_defs.rs | 150 ---- ui/goose2/src/app/hooks/useAppStartup.ts | 31 +- .../providers/api/credentials.test.ts | 126 ++++ .../src/features/providers/api/credentials.ts | 48 +- .../providers/api/inventorySync.test.ts | 143 ++++ .../features/providers/api/inventorySync.ts | 115 +++ .../providers/hooks/useCredentials.test.tsx | 248 +++++++ .../providers/hooks/useCredentials.ts | 225 +++++- .../settings/ui/AgentProviderCard.tsx | 54 +- .../settings/ui/ModelProviderPanels.tsx | 53 ++ .../features/settings/ui/ModelProviderRow.tsx | 56 +- .../settings/ui/ProvidersSettings.tsx | 57 +- .../ui/__tests__/AgentProviderCard.test.tsx | 51 +- .../ui/__tests__/ModelProviderRow.test.tsx | 113 +++ .../ui/__tests__/ProvidersSettings.test.tsx | 91 +++ .../shared/api/__tests__/dictation.test.ts | 19 + ui/goose2/src/shared/api/dictation.ts | 18 +- .../src/shared/i18n/locales/en/settings.json | 8 +- .../src/shared/i18n/locales/es/settings.json | 8 +- ui/sdk/src/generated/client.gen.ts | 56 ++ ui/sdk/src/generated/index.ts | 22 +- ui/sdk/src/generated/types.gen.ts | 64 +- ui/sdk/src/generated/zod.gen.ts | 70 ++ 39 files changed, 3079 insertions(+), 1118 deletions(-) create mode 100644 crates/goose/tests/acp_secret_cache_invalidation_test.rs delete mode 100644 ui/goose2/src-tauri/src/commands/credentials.rs delete mode 100644 ui/goose2/src-tauri/src/services/goose_config.rs delete mode 100644 ui/goose2/src-tauri/src/services/provider_defs.rs create mode 100644 ui/goose2/src/features/providers/api/credentials.test.ts create mode 100644 ui/goose2/src/features/providers/api/inventorySync.test.ts create mode 100644 ui/goose2/src/features/providers/api/inventorySync.ts create mode 100644 ui/goose2/src/features/providers/hooks/useCredentials.test.tsx create mode 100644 ui/goose2/src/features/settings/ui/__tests__/ModelProviderRow.test.tsx create mode 100644 ui/goose2/src/features/settings/ui/__tests__/ProvidersSettings.test.tsx diff --git a/crates/goose-sdk/src/custom_requests.rs b/crates/goose-sdk/src/custom_requests.rs index c8fc781c..a3fc9e89 100644 --- a/crates/goose-sdk/src/custom_requests.rs +++ b/crates/goose-sdk/src/custom_requests.rs @@ -294,6 +294,96 @@ pub struct ProviderConfigKey { pub primary: bool, } +#[derive(Debug, Default, Clone, Serialize, Deserialize, JsonSchema)] +#[serde(rename_all = "camelCase")] +pub struct ProviderConfigFieldValueDto { + pub key: String, + #[serde(default)] + pub value: Option, + pub is_set: bool, + pub is_secret: bool, + pub required: bool, +} + +#[derive(Debug, Default, Clone, Serialize, Deserialize, JsonSchema)] +#[serde(rename_all = "camelCase")] +pub struct ProviderConfigStatusDto { + pub provider_id: String, + pub is_configured: bool, +} + +#[derive(Debug, Default, Clone, Serialize, Deserialize, JsonSchema)] +#[serde(rename_all = "camelCase")] +pub struct ProviderConfigFieldUpdate { + pub key: String, + pub value: String, +} + +/// Read saved configuration field values for one provider. +#[derive(Debug, Default, Clone, Serialize, Deserialize, JsonSchema, JsonRpcRequest)] +#[request( + method = "_goose/providers/config/read", + response = ProviderConfigReadResponse +)] +#[serde(rename_all = "camelCase")] +pub struct ProviderConfigReadRequest { + pub provider_id: String, +} + +#[derive(Debug, Default, Clone, Serialize, Deserialize, JsonSchema, JsonRpcResponse)] +#[serde(rename_all = "camelCase")] +pub struct ProviderConfigReadResponse { + pub fields: Vec, +} + +/// Return provider configured statuses. Empty provider_ids means all providers. +#[derive(Debug, Default, Clone, Serialize, Deserialize, JsonSchema, JsonRpcRequest)] +#[request( + method = "_goose/providers/config/status", + response = ProviderConfigStatusResponse +)] +#[serde(rename_all = "camelCase")] +pub struct ProviderConfigStatusRequest { + #[serde(default)] + pub provider_ids: Vec, +} + +#[derive(Debug, Default, Clone, Serialize, Deserialize, JsonSchema, JsonRpcResponse)] +#[serde(rename_all = "camelCase")] +pub struct ProviderConfigStatusResponse { + pub statuses: Vec, +} + +/// Save provider configuration fields and start an inventory refresh when supported. +#[derive(Debug, Default, Clone, Serialize, Deserialize, JsonSchema, JsonRpcRequest)] +#[request( + method = "_goose/providers/config/save", + response = ProviderConfigChangeResponse +)] +#[serde(rename_all = "camelCase")] +pub struct ProviderConfigSaveRequest { + pub provider_id: String, + pub fields: Vec, +} + +/// Delete provider configuration fields and start an inventory refresh when supported. +#[derive(Debug, Default, Clone, Serialize, Deserialize, JsonSchema, JsonRpcRequest)] +#[request( + method = "_goose/providers/config/delete", + response = ProviderConfigChangeResponse +)] +#[serde(rename_all = "camelCase")] +pub struct ProviderConfigDeleteRequest { + pub provider_id: String, +} + +#[derive(Debug, Default, Clone, Serialize, Deserialize, JsonSchema, JsonRpcResponse)] +#[serde(rename_all = "camelCase")] +pub struct ProviderConfigChangeResponse { + pub status: ProviderConfigStatusDto, + pub refresh: RefreshProviderInventoryResponse, +} + /// The type of source entity. #[derive( Debug, Default, Clone, Copy, PartialEq, Eq, PartialOrd, Ord, Serialize, Deserialize, JsonSchema, diff --git a/crates/goose/acp-meta.json b/crates/goose/acp-meta.json index 252639c0..1d13987b 100644 --- a/crates/goose/acp-meta.json +++ b/crates/goose/acp-meta.json @@ -65,6 +65,26 @@ "requestType": "RefreshProviderInventoryRequest", "responseType": "RefreshProviderInventoryResponse" }, + { + "method": "_goose/providers/config/read", + "requestType": "ProviderConfigReadRequest", + "responseType": "ProviderConfigReadResponse" + }, + { + "method": "_goose/providers/config/status", + "requestType": "ProviderConfigStatusRequest", + "responseType": "ProviderConfigStatusResponse" + }, + { + "method": "_goose/providers/config/save", + "requestType": "ProviderConfigSaveRequest", + "responseType": "ProviderConfigChangeResponse" + }, + { + "method": "_goose/providers/config/delete", + "requestType": "ProviderConfigDeleteRequest", + "responseType": "ProviderConfigChangeResponse" + }, { "method": "_goose/config/read", "requestType": "ReadConfigRequest", diff --git a/crates/goose/acp-schema.json b/crates/goose/acp-schema.json index 1931938e..be922800 100644 --- a/crates/goose/acp-schema.json +++ b/crates/goose/acp-schema.json @@ -537,6 +537,178 @@ "already_refreshing" ] }, + "ProviderConfigReadRequest": { + "type": "object", + "properties": { + "providerId": { + "type": "string" + } + }, + "required": [ + "providerId" + ], + "description": "Read saved configuration field values for one provider.", + "x-side": "agent", + "x-method": "_goose/providers/config/read" + }, + "ProviderConfigReadResponse": { + "type": "object", + "properties": { + "fields": { + "type": "array", + "items": { + "$ref": "#/$defs/ProviderConfigFieldValueDto" + } + } + }, + "required": [ + "fields" + ], + "x-side": "agent", + "x-method": "_goose/providers/config/read" + }, + "ProviderConfigFieldValueDto": { + "type": "object", + "properties": { + "key": { + "type": "string" + }, + "value": { + "type": [ + "string", + "null" + ], + "default": null + }, + "isSet": { + "type": "boolean" + }, + "isSecret": { + "type": "boolean" + }, + "required": { + "type": "boolean" + } + }, + "required": [ + "key", + "isSet", + "isSecret", + "required" + ] + }, + "ProviderConfigStatusRequest": { + "type": "object", + "properties": { + "providerIds": { + "type": "array", + "items": { + "type": "string" + }, + "default": [] + } + }, + "description": "Return provider configured statuses. Empty provider_ids means all providers.", + "x-side": "agent", + "x-method": "_goose/providers/config/status" + }, + "ProviderConfigStatusResponse": { + "type": "object", + "properties": { + "statuses": { + "type": "array", + "items": { + "$ref": "#/$defs/ProviderConfigStatusDto" + } + } + }, + "required": [ + "statuses" + ], + "x-side": "agent", + "x-method": "_goose/providers/config/status" + }, + "ProviderConfigStatusDto": { + "type": "object", + "properties": { + "providerId": { + "type": "string" + }, + "isConfigured": { + "type": "boolean" + } + }, + "required": [ + "providerId", + "isConfigured" + ] + }, + "ProviderConfigSaveRequest": { + "type": "object", + "properties": { + "providerId": { + "type": "string" + }, + "fields": { + "type": "array", + "items": { + "$ref": "#/$defs/ProviderConfigFieldUpdate" + } + } + }, + "required": [ + "providerId", + "fields" + ], + "description": "Save provider configuration fields and start an inventory refresh when supported.", + "x-side": "agent", + "x-method": "_goose/providers/config/save" + }, + "ProviderConfigFieldUpdate": { + "type": "object", + "properties": { + "key": { + "type": "string" + }, + "value": { + "type": "string" + } + }, + "required": [ + "key", + "value" + ] + }, + "ProviderConfigChangeResponse": { + "type": "object", + "properties": { + "status": { + "$ref": "#/$defs/ProviderConfigStatusDto" + }, + "refresh": { + "$ref": "#/$defs/RefreshProviderInventoryResponse" + } + }, + "required": [ + "status", + "refresh" + ], + "x-side": "agent" + }, + "ProviderConfigDeleteRequest": { + "type": "object", + "properties": { + "providerId": { + "type": "string" + } + }, + "required": [ + "providerId" + ], + "description": "Delete provider configuration fields and start an inventory refresh when supported.", + "x-side": "agent", + "x-method": "_goose/providers/config/delete" + }, "ReadConfigRequest": { "type": "object", "properties": { @@ -1519,6 +1691,42 @@ "description": "Params for _goose/providers/inventory/refresh", "title": "RefreshProviderInventoryRequest" }, + { + "allOf": [ + { + "$ref": "#/$defs/ProviderConfigReadRequest" + } + ], + "description": "Params for _goose/providers/config/read", + "title": "ProviderConfigReadRequest" + }, + { + "allOf": [ + { + "$ref": "#/$defs/ProviderConfigStatusRequest" + } + ], + "description": "Params for _goose/providers/config/status", + "title": "ProviderConfigStatusRequest" + }, + { + "allOf": [ + { + "$ref": "#/$defs/ProviderConfigSaveRequest" + } + ], + "description": "Params for _goose/providers/config/save", + "title": "ProviderConfigSaveRequest" + }, + { + "allOf": [ + { + "$ref": "#/$defs/ProviderConfigDeleteRequest" + } + ], + "description": "Params for _goose/providers/config/delete", + "title": "ProviderConfigDeleteRequest" + }, { "allOf": [ { @@ -1839,6 +2047,30 @@ ], "title": "RefreshProviderInventoryResponse" }, + { + "allOf": [ + { + "$ref": "#/$defs/ProviderConfigReadResponse" + } + ], + "title": "ProviderConfigReadResponse" + }, + { + "allOf": [ + { + "$ref": "#/$defs/ProviderConfigStatusResponse" + } + ], + "title": "ProviderConfigStatusResponse" + }, + { + "allOf": [ + { + "$ref": "#/$defs/ProviderConfigChangeResponse" + } + ], + "title": "ProviderConfigChangeResponse" + }, { "allOf": [ { diff --git a/crates/goose/src/acp/server.rs b/crates/goose/src/acp/server.rs index 08dc02bd..1f94d253 100644 --- a/crates/goose/src/acp/server.rs +++ b/crates/goose/src/acp/server.rs @@ -25,7 +25,8 @@ use crate::permission::permission_confirmation::PrincipalType; use crate::permission::{Permission, PermissionConfirmation}; use crate::providers::base::Provider; use crate::providers::inventory::{ - ProviderInventoryEntry, ProviderInventoryService, RefreshSkipReason, + InventoryIdentity, ProviderInventoryEntry, ProviderInventoryService, RefreshJobPlan, + RefreshPlan, RefreshSkipReason, }; use crate::session::session_manager::SessionType; use crate::session::{EnabledExtensionsState, Session, SessionManager}; @@ -33,6 +34,8 @@ use crate::utils::sanitize_unicode_tags; use anyhow::Result; use fs_err as fs; use futures::future::{BoxFuture, Either}; +use futures::stream::{self, StreamExt}; +use futures::FutureExt; use goose_acp_macros::custom_methods; use rmcp::model::{ AnnotateAble, CallToolResult, RawContent, RawTextContent, ResourceContents, Role, @@ -62,6 +65,7 @@ use sacp::{ }; use serde::Deserialize; use std::collections::HashMap; +use std::panic::AssertUnwindSafe; use std::sync::Arc; use strum::{EnumMessage, VariantNames}; use tokio::sync::{Mutex, OnceCell}; @@ -116,6 +120,21 @@ const ELEVENLABS_TRANSCRIPTION_MODEL_CONFIG_KEY: &str = "ELEVENLABS_TRANSCRIPTIO const OPENAI_TRANSCRIPTION_MODEL: &str = "whisper-1"; const GROQ_TRANSCRIPTION_MODEL: &str = "whisper-large-v3-turbo"; const ELEVENLABS_TRANSCRIPTION_MODEL: &str = "scribe_v1"; +const PROVIDER_CONFIG_STATUS_CHECK_CONCURRENCY: usize = 16; + +async fn ensure_refresh_identity_current( + provider_id: &str, + planned_identity: &InventoryIdentity, +) -> Result<()> { + let current_identity = crate::providers::inventory_identity(provider_id) + .await? + .into_identity()?; + if current_identity != *planned_identity { + anyhow::bail!("provider inventory identity changed before refresh completed"); + } + + Ok(()) +} /// In-memory state for an active ACP session. /// @@ -613,6 +632,96 @@ fn provider_config_key_to_dto(key: crate::providers::base::ConfigKey) -> Provide } } +const SECRET_MASK_PREFIX_LEN: usize = 4; +const SECRET_MASK_SUFFIX_LEN: usize = 3; +const SECRET_MASK_FALLBACK: &str = "***"; + +fn mask_secret_value(value: &str) -> String { + let prefix: String = value.chars().take(SECRET_MASK_PREFIX_LEN).collect(); + let suffix_chars: Vec = value.chars().rev().take(SECRET_MASK_SUFFIX_LEN).collect(); + let suffix: String = suffix_chars.into_iter().rev().collect(); + + if prefix.is_empty() + || suffix.is_empty() + || value.chars().count() <= SECRET_MASK_PREFIX_LEN + SECRET_MASK_SUFFIX_LEN + { + return SECRET_MASK_FALLBACK.to_string(); + } + + format!("{prefix}...{suffix}") +} + +fn config_value_to_string(value: &serde_json::Value) -> Option { + match value { + serde_json::Value::Null => None, + serde_json::Value::String(value) if value.is_empty() => None, + serde_json::Value::String(value) => Some(value.clone()), + other => serde_json::to_string(other).ok(), + } +} + +fn provider_config_field_value( + config: &Config, + key: &crate::providers::base::ConfigKey, + secrets: Option<&HashMap>, +) -> ProviderConfigFieldValueDto { + let value = if key.secret { + std::env::var(key.name.to_uppercase()).ok().or_else(|| { + secrets + .and_then(|values| values.get(&key.name)) + .and_then(config_value_to_string) + }) + } else { + config + .get_param::(&key.name) + .ok() + .and_then(|value| config_value_to_string(&value)) + }; + + ProviderConfigFieldValueDto { + key: key.name.clone(), + value: value.as_deref().map(|value| { + if key.secret { + mask_secret_value(value) + } else { + value.to_string() + } + }), + is_set: value.is_some(), + is_secret: key.secret, + required: key.required, + } +} + +fn refresh_skip_reason_to_dto(reason: RefreshSkipReason) -> RefreshProviderInventorySkipReasonDto { + match reason { + RefreshSkipReason::UnknownProvider => { + RefreshProviderInventorySkipReasonDto::UnknownProvider + } + RefreshSkipReason::NotConfigured => RefreshProviderInventorySkipReasonDto::NotConfigured, + RefreshSkipReason::DoesNotSupportRefresh => { + RefreshProviderInventorySkipReasonDto::DoesNotSupportRefresh + } + RefreshSkipReason::AlreadyRefreshing => { + RefreshProviderInventorySkipReasonDto::AlreadyRefreshing + } + } +} + +fn refresh_plan_to_response(refresh_plan: RefreshPlan) -> RefreshProviderInventoryResponse { + RefreshProviderInventoryResponse { + started: refresh_plan.started, + skipped: refresh_plan + .skipped + .into_iter() + .map(|entry| RefreshProviderInventorySkipDto { + provider_id: entry.provider_id, + reason: refresh_skip_reason_to_dto(entry.reason), + }) + .collect(), + } +} + fn build_model_state(current_model: &str, inventory: &ProviderInventoryEntry) -> SessionModelState { let mut available_models = inventory .models @@ -921,6 +1030,7 @@ impl GooseAcpAgent { Some(&goose_session.extension_data), &config, ); + Config::global().invalidate_secrets_cache(); match self .create_provider(provider_name, model_config.clone(), ext_state) .await @@ -930,37 +1040,87 @@ impl GooseAcpAgent { prebuilt_provider = Some(provider.clone()); match self .provider_inventory - .plan_refresh(std::slice::from_ref(&provider_id)) + .plan_refresh_jobs(std::slice::from_ref(&provider_id)) .await { - Ok(plan) if plan.started.iter().any(|id| id == &provider_id) => { - match provider.fetch_recommended_models().await { - Ok(models) => { - if let Err(error) = self - .provider_inventory - .store_refreshed_models(&provider_id, &models) - .await + Ok(plan) + if plan + .started + .iter() + .any(|job| job.provider_id == provider_id) => + { + let refresh_job = plan + .started + .into_iter() + .find(|job| job.provider_id == provider_id); + if let Some(refresh_job) = refresh_job { + let mut refresh_guard = self + .provider_inventory + .refresh_guard(&refresh_job.identity); + let fetch_result: Result> = + match ensure_refresh_identity_current( + &provider_id, + &refresh_job.identity, + ) + .await { - warn!( - provider = %provider_id, - error = %error, - "failed to store refreshed provider inventory during session init" - ); - } - } - Err(error) => { - if let Err(store_error) = self - .provider_inventory - .store_refresh_error( - &provider_id, - error.to_string(), + Ok(()) => match AssertUnwindSafe( + provider.fetch_recommended_models(), ) + .catch_unwind() .await - { + { + Ok(Ok(models)) => Ok(models), + Ok(Err(error)) => { + Err(anyhow::anyhow!(error.to_string())) + } + Err(_) => Err(anyhow::anyhow!( + "provider inventory refresh task panicked" + )), + }, + Err(error) => Err(error), + }; + match fetch_result { + Ok(models) => { + if let Err(error) = self + .provider_inventory + .store_refreshed_models_for_identity( + &refresh_job.identity, + &models, + ) + .await + { + warn!( + provider = %provider_id, + error = %error, + "failed to store refreshed provider inventory during session init" + ); + } else { + refresh_guard.complete(); + } + } + Err(error) => { + let error_message = error.to_string(); + if let Err(store_error) = self + .provider_inventory + .store_refresh_error_for_identity( + &refresh_job.identity, + error_message.clone(), + ) + .await + { + warn!( + provider = %provider_id, + error = %store_error, + "failed to store provider inventory refresh error during session init" + ); + } else { + refresh_guard.complete(); + } warn!( provider = %provider_id, - error = %store_error, - "failed to store provider inventory refresh error during session init" + error = %error_message, + "provider inventory refresh failed during session init" ); } } @@ -2311,8 +2471,6 @@ impl GooseAcpAgent { .await .internal_err_ctx("Error getting agent reply")?; - use futures::StreamExt; - let mut was_cancelled = false; let mut first_event_logged = false; let mut event_count: u32 = 0; @@ -3003,68 +3161,273 @@ impl GooseAcpAgent { }) } + async fn provider_config_status(provider_id: String) -> ProviderConfigStatusDto { + let is_configured = match crate::providers::get_from_registry(&provider_id).await { + Ok(entry) => { + match tokio::task::spawn_blocking(move || entry.inventory_configured()).await { + Ok(is_configured) => is_configured, + Err(error) => { + warn!( + provider = %provider_id, + error = %error, + "provider config status check failed" + ); + false + } + } + } + Err(_) => false, + }; + + ProviderConfigStatusDto { + provider_id, + is_configured, + } + } + + async fn provider_config_statuses(provider_ids: &[String]) -> Vec { + let mut ids = if provider_ids.is_empty() { + crate::providers::providers() + .await + .into_iter() + .map(|(metadata, _)| metadata.name) + .collect::>() + } else { + provider_ids.to_vec() + }; + ids.sort(); + ids.dedup(); + + let mut statuses = stream::iter(ids) + .map(Self::provider_config_status) + .buffer_unordered(PROVIDER_CONFIG_STATUS_CHECK_CONCURRENCY) + .collect::>() + .await; + statuses.sort_by(|a, b| a.provider_id.cmp(&b.provider_id)); + statuses + } + + fn spawn_provider_inventory_refresh_jobs(&self, refresh_plan: &RefreshJobPlan) { + for refresh_job in refresh_plan.started.iter().cloned() { + let provider_inventory = self.provider_inventory.clone(); + let provider_factory = Arc::clone(&self.provider_factory); + let provider_id = refresh_job.provider_id.clone(); + let identity = refresh_job.identity.clone(); + tokio::spawn(async move { + let mut refresh_guard = provider_inventory.refresh_guard(&identity); + let provider_result = AssertUnwindSafe(async { + let metadata = crate::providers::get_from_registry(&provider_id).await?; + let model_config = + crate::model::ModelConfig::new(&metadata.metadata().default_model)? + .with_canonical_limits(&provider_id); + provider_factory(provider_id.clone(), model_config, Vec::new()).await + }) + .catch_unwind() + .await; + + let fetch_result: Result> = match provider_result { + Ok(Ok(provider)) => { + match ensure_refresh_identity_current(&provider_id, &identity).await { + Ok(()) => match AssertUnwindSafe(provider.fetch_recommended_models()) + .catch_unwind() + .await + { + Ok(Ok(models)) => Ok(models), + Ok(Err(error)) => Err(anyhow::anyhow!(error.to_string())), + Err(_) => { + Err(anyhow::anyhow!("provider inventory refresh task panicked")) + } + }, + Err(error) => Err(error), + } + } + Ok(Err(error)) => Err(error), + Err(_) => Err(anyhow::anyhow!("provider inventory refresh task panicked")), + }; + + match fetch_result { + Ok(models) => match provider_inventory + .store_refreshed_models_for_identity(&identity, &models) + .await + { + Ok(()) => refresh_guard.complete(), + Err(error) => warn!( + provider = %provider_id, + error = %error, + "failed to store refreshed provider inventory" + ), + }, + Err(error) => { + let error_message = error.to_string(); + match provider_inventory + .store_refresh_error_for_identity(&identity, error_message.clone()) + .await + { + Ok(()) => refresh_guard.complete(), + Err(store_error) => warn!( + provider = %provider_id, + error = %store_error, + refresh_error = %error_message, + "failed to store provider inventory refresh error" + ), + } + warn!(provider = %provider_id, error = %error_message, "provider inventory refresh failed"); + } + } + }); + } + } + + async fn start_provider_inventory_refresh( + &self, + provider_ids: &[String], + ) -> Result { + let refresh_job_plan = self + .provider_inventory + .plan_refresh_jobs(provider_ids) + .await + .internal_err()?; + self.spawn_provider_inventory_refresh_jobs(&refresh_job_plan); + Ok(refresh_plan_to_response( + refresh_job_plan.into_public_plan(), + )) + } + #[custom_method(RefreshProviderInventoryRequest)] async fn on_refresh_provider_inventory( &self, req: RefreshProviderInventoryRequest, ) -> Result { - let refresh_plan = self - .provider_inventory - .plan_refresh(&req.provider_ids) - .await; - let refresh_plan = refresh_plan.internal_err()?; - for provider_id in &refresh_plan.started { - let provider_inventory = self.provider_inventory.clone(); - let provider_factory = Arc::clone(&self.provider_factory); - let provider_id = provider_id.clone(); - tokio::spawn(async move { - let result = async { - let metadata = crate::providers::get_from_registry(&provider_id).await?; - let model_config = - crate::model::ModelConfig::new(&metadata.metadata().default_model)? - .with_canonical_limits(&provider_id); - let provider = - provider_factory(provider_id.clone(), model_config, Vec::new()).await?; - let models = provider.fetch_recommended_models().await?; - provider_inventory - .store_refreshed_models(&provider_id, &models) - .await - } - .await; - if let Err(error) = result { - let _ = provider_inventory - .store_refresh_error(&provider_id, error.to_string()) - .await; - warn!(provider = %provider_id, error = %error, "provider inventory refresh failed"); - } - }); - } - Ok(RefreshProviderInventoryResponse { - started: refresh_plan.started, - skipped: refresh_plan - .skipped - .into_iter() - .map(|entry| RefreshProviderInventorySkipDto { - provider_id: entry.provider_id, - reason: match entry.reason { - RefreshSkipReason::UnknownProvider => { - RefreshProviderInventorySkipReasonDto::UnknownProvider - } - RefreshSkipReason::NotConfigured => { - RefreshProviderInventorySkipReasonDto::NotConfigured - } - RefreshSkipReason::DoesNotSupportRefresh => { - RefreshProviderInventorySkipReasonDto::DoesNotSupportRefresh - } - RefreshSkipReason::AlreadyRefreshing => { - RefreshProviderInventorySkipReasonDto::AlreadyRefreshing - } - }, - }) + Config::global().invalidate_secrets_cache(); + self.start_provider_inventory_refresh(&req.provider_ids) + .await + } + + #[custom_method(ProviderConfigReadRequest)] + async fn on_read_provider_config( + &self, + req: ProviderConfigReadRequest, + ) -> Result { + let entry = crate::providers::get_from_registry(&req.provider_id) + .await + .invalid_params_err_ctx("Unknown provider")?; + let config = Config::global(); + let config_keys = &entry.metadata().config_keys; + let secrets = if config_keys.iter().any(|key| key.secret) { + Some(config.all_secrets().internal_err()?) + } else { + None + }; + + Ok(ProviderConfigReadResponse { + fields: config_keys + .iter() + .map(|key| provider_config_field_value(config, key, secrets.as_ref())) .collect(), }) } + #[custom_method(ProviderConfigStatusRequest)] + async fn on_provider_config_status( + &self, + req: ProviderConfigStatusRequest, + ) -> Result { + Ok(ProviderConfigStatusResponse { + statuses: Self::provider_config_statuses(&req.provider_ids).await, + }) + } + + #[custom_method(ProviderConfigSaveRequest)] + async fn on_save_provider_config( + &self, + req: ProviderConfigSaveRequest, + ) -> Result { + let entry = crate::providers::get_from_registry(&req.provider_id) + .await + .invalid_params_err_ctx("Unknown provider")?; + let metadata = entry.metadata().clone(); + let config = Config::global(); + let mut config_updates = Vec::new(); + let mut secret_updates = Vec::new(); + + for field in &req.fields { + let Some(config_key) = metadata + .config_keys + .iter() + .find(|config_key| config_key.name == field.key) + else { + return Err(sacp::Error::invalid_params() + .data(format!("Unsupported provider config field: {}", field.key))); + }; + + let value = field.value.trim(); + if value.is_empty() { + return Err(sacp::Error::invalid_params().data(format!( + "Provider config field cannot be empty: {}", + field.key + ))); + } + + if config_key.secret { + secret_updates.push(( + config_key.name.clone(), + serde_json::Value::String(value.to_string()), + )); + } else { + config_updates.push((config_key.name.clone(), value.to_string())); + } + } + + for (key, value) in config_updates { + config + .set_param(&key, &value) + .internal_err_ctx("Failed to save provider config field")?; + } + config + .set_secret_values(&secret_updates) + .internal_err_ctx("Failed to save provider secret fields")?; + + let provider_ids = [req.provider_id.clone()]; + let status = Self::provider_config_status(req.provider_id.clone()).await; + let refresh = self.start_provider_inventory_refresh(&provider_ids).await?; + Ok(ProviderConfigChangeResponse { status, refresh }) + } + + #[custom_method(ProviderConfigDeleteRequest)] + async fn on_delete_provider_config( + &self, + req: ProviderConfigDeleteRequest, + ) -> Result { + let entry = crate::providers::get_from_registry(&req.provider_id) + .await + .invalid_params_err_ctx("Unknown provider")?; + let metadata = entry.metadata().clone(); + let config = Config::global(); + let mut secret_keys = Vec::new(); + + for config_key in &metadata.config_keys { + if config_key.secret { + secret_keys.push(config_key.name.clone()); + } else { + config + .delete(&config_key.name) + .internal_err_ctx("Failed to delete provider config field")?; + } + } + + config + .delete_secret_values(&secret_keys) + .internal_err_ctx("Failed to delete provider secret fields")?; + crate::providers::cleanup_provider(&req.provider_id) + .await + .internal_err_ctx("Failed to clean up provider state")?; + + let provider_ids = [req.provider_id.clone()]; + let status = Self::provider_config_status(req.provider_id.clone()).await; + let refresh = self.start_provider_inventory_refresh(&provider_ids).await?; + Ok(ProviderConfigChangeResponse { status, refresh }) + } + #[custom_method(ReadConfigRequest)] async fn on_read_config( &self, @@ -3118,6 +3481,7 @@ impl GooseAcpAgent { ) -> Result { let config = self.config()?; config.set_secret(&req.key, &req.value).internal_err()?; + Config::global().invalidate_secrets_cache(); Ok(EmptyResponse {}) } @@ -3128,6 +3492,7 @@ impl GooseAcpAgent { ) -> Result { let config = self.config()?; config.delete_secret(&req.key).internal_err()?; + Config::global().invalidate_secrets_cache(); Ok(EmptyResponse {}) } @@ -3832,6 +4197,7 @@ impl HandleDispatchFrom for GooseAcpHandler { let t_handler = std::time::Instant::now(); match config_id.as_ref() { "provider" => { + Config::global().invalidate_secrets_cache(); match agent.update_provider(&session_id.0, &value_id.0, None, None, None).await { Ok(_) => {} Err(e) => { responder.respond_with_error(e)?; return Ok(()); } @@ -3865,59 +4231,132 @@ impl HandleDispatchFrom for GooseAcpHandler { let provider_id = value_id.0.to_string(); agent .provider_inventory - .plan_refresh(std::slice::from_ref(&provider_id)) + .plan_refresh_jobs(std::slice::from_ref(&provider_id)) .await .ok() - .filter(|plan| plan.started.iter().any(|id| id == &provider_id)) + .and_then(|plan| { + plan.started + .into_iter() + .find(|job| job.provider_id == provider_id) + }) } else { None }; - if maybe_refresh.is_some() { + if let Some(refresh_job) = maybe_refresh { let agent_bg = agent.clone(); let cx_bg = cx.clone(); let session_id_bg = session_id.clone(); tokio::spawn(async move { - let refreshed = async { - let session_agent = - agent_bg.get_session_agent(&session_id_bg.0, None).await?; - let provider = session_agent - .provider() - .await - .map_err(|e| anyhow::anyhow!(e.to_string()))?; - let provider_name = provider.get_name().to_string(); - let models = provider - .fetch_recommended_models() - .await - .map_err(|e| anyhow::anyhow!(e.to_string()))?; - agent_bg - .provider_inventory - .store_refreshed_models(&provider_name, &models) - .await?; - agent_bg - .build_config_update(&session_id_bg) - .await - .map_err(|e| anyhow::anyhow!(e.to_string())) - } - .await; + let refresh_identity = refresh_job.identity; + let refresh_provider_id = refresh_job.provider_id; + let mut refresh_guard = + agent_bg.provider_inventory.refresh_guard(&refresh_identity); + let provider_result: Result> = + AssertUnwindSafe(async { + let session_agent = + agent_bg.get_session_agent(&session_id_bg.0, None).await?; + let provider = session_agent + .provider() + .await + .map_err(|e| anyhow::anyhow!(e.to_string()))?; + let provider_name = provider.get_name().to_string(); + if provider_name != refresh_provider_id { + return Err(anyhow::anyhow!( + "provider changed before inventory refresh completed" + )); + } + Ok(provider) + }) + .catch_unwind() + .await + .map_err(|_| { + anyhow::anyhow!("provider inventory refresh task panicked") + }) + .and_then(|result| result); - match refreshed { - Ok((fresh_notification, _)) => { - let _ = cx_bg.send_notification(fresh_notification); - } - Err(e) => { - if let Ok(session_agent) = - agent_bg.get_session_agent(&session_id_bg.0, None).await + let fetch_result = match provider_result { + Ok(provider) => { + match ensure_refresh_identity_current( + &refresh_provider_id, + &refresh_identity, + ) + .await + { + Ok(()) => match AssertUnwindSafe( + provider.fetch_recommended_models(), + ) + .catch_unwind() + .await { - if let Ok(provider) = session_agent.provider().await { - let provider_name = provider.get_name().to_string(); - let _ = agent_bg - .provider_inventory - .store_refresh_error(&provider_name, e.to_string()) - .await; + Ok(Ok(models)) => Ok(models), + Ok(Err(error)) => { + Err(anyhow::anyhow!(error.to_string())) } + Err(_) => Err(anyhow::anyhow!( + "provider inventory refresh task panicked" + )), + }, + Err(error) => Err(error), + } + } + Err(error) => Err(error), + }; + + match fetch_result { + Ok(models) => match agent_bg + .provider_inventory + .store_refreshed_models_for_identity( + &refresh_identity, + &models, + ) + .await + { + Ok(()) => { + refresh_guard.complete(); + match agent_bg.build_config_update(&session_id_bg).await + { + Ok((fresh_notification, _)) => { + let _ = cx_bg + .send_notification(fresh_notification); + } + Err(error) => warn!( + provider = %refresh_provider_id, + error = %error, + "failed to build config update after provider inventory refresh" + ), } } + Err(error) => warn!( + provider = %refresh_provider_id, + error = %error, + "failed to store refreshed provider inventory after config change" + ), + }, + Err(error) => { + let error_message = error.to_string(); + match agent_bg + .provider_inventory + .store_refresh_error_for_identity( + &refresh_identity, + error_message.clone(), + ) + .await + { + Ok(()) => refresh_guard.complete(), + Err(store_error) => warn!( + provider = %refresh_provider_id, + error = %store_error, + refresh_error = %error_message, + "failed to store provider inventory refresh error after config change" + ), + } + warn!( + provider = %refresh_provider_id, + error = %error_message, + "provider inventory refresh failed after config change" + ); } + } }); } diff --git a/crates/goose/src/config/base.rs b/crates/goose/src/config/base.rs index 34e6a6f9..5266c5dc 100644 --- a/crates/goose/src/config/base.rs +++ b/crates/goose/src/config/base.rs @@ -181,9 +181,10 @@ impl Default for Config { secrets_cache: Arc::new(Mutex::new(None)), }; - let secrets = if no_secrets_config - .get_param::("GOOSE_DISABLE_KEYRING") - .is_ok() + let secrets = if env::var("GOOSE_DISABLE_KEYRING").is_ok() + || no_secrets_config + .get_param::("GOOSE_DISABLE_KEYRING") + .is_ok_and(|v| keyring_disabled_value(&v)) { SecretStorage::File { path: config_dir.join("secrets.yaml"), @@ -281,6 +282,10 @@ fn parse_yaml_content(content: &str) -> Result { serde_yaml::from_str(content).map_err(|e| e.into()) } +fn keyring_disabled_value(value: &serde_yaml::Value) -> bool { + value.as_bool().unwrap_or(false) || value.as_str().is_some_and(|s| s == "true" || s == "1") +} + const EXTENSIONS_KEY: &str = "extensions"; pub fn merge_config_values(base: &mut Mapping, overlay: Mapping) { @@ -325,6 +330,19 @@ fn merge_extensions(base: &mut Mapping, overlay: &Mapping) { } } +/// Read the GOOSE_DISABLE_KEYRING flag from the config file. +/// +/// Called before Config is fully initialised, so we do a minimal raw read +/// rather than going through `get_param`. All errors are treated as `false` +/// (keyring stays enabled) so a missing/malformed file is never fatal here. +fn keyring_disabled_in_config(config_path: &Path) -> bool { + std::fs::read_to_string(config_path) + .ok() + .and_then(|s| parse_yaml_content(&s).ok()) + .and_then(|m| m.get("GOOSE_DISABLE_KEYRING").map(keyring_disabled_value)) + .unwrap_or(false) +} + impl Config { /// Get the global configuration instance. /// @@ -339,11 +357,25 @@ impl Config { /// This is primarily useful for testing or for applications that need /// to manage multiple configuration files. pub fn new>(config_path: P, service: &str) -> Result { - Ok(Config { - config_paths: vec![config_path.as_ref().to_path_buf()], - secrets: SecretStorage::Keyring { + let config_path = config_path.as_ref().to_path_buf(); + let secrets = if env::var("GOOSE_DISABLE_KEYRING").is_ok() + || keyring_disabled_in_config(&config_path) + { + let config_dir = config_path + .parent() + .map(Path::to_path_buf) + .unwrap_or_else(Paths::config_dir); + SecretStorage::File { + path: config_dir.join("secrets.yaml"), + } + } else { + SecretStorage::Keyring { service: service.to_string(), - }, + } + }; + Ok(Config { + config_paths: vec![config_path], + secrets, guard: Mutex::new(()), secrets_cache: Arc::new(Mutex::new(None)), }) @@ -767,6 +799,40 @@ impl Config { Ok(result) } + fn write_all_secrets(&self, values: &HashMap) -> Result<(), ConfigError> { + match &self.secrets { + SecretStorage::Keyring { service } => { + let json_value = serde_json::to_string(values)?; + match self.handle_keyring_operation( + |entry| entry.set_password(&json_value), + service, + Some(values), + ) { + Ok(_) => {} + Err(ConfigError::FallbackToFileStorage) => {} + Err(e) => return Err(e), + } + } + SecretStorage::File { path } => { + let yaml_value = serde_yaml::to_string(values)?; + write_secrets_file(path, &yaml_value)?; + } + } + + self.invalidate_secrets_cache(); + Ok(()) + } + + fn mutate_secrets( + &self, + mutate: impl FnOnce(&mut HashMap), + ) -> Result<(), ConfigError> { + let _guard = self.guard.lock().unwrap(); + let mut values = self.all_secrets()?; + mutate(&mut values); + self.write_all_secrets(&values) + } + /// Set a secret value in the system keyring. /// /// This will store the value in a single JSON object in the system keyring, @@ -785,34 +851,27 @@ impl Config { where V: Serialize, { - // Lock before reading to prevent race condition. - let _guard = self.guard.lock().unwrap(); + let value = serde_json::to_value(value)?; + self.mutate_secrets(|values| { + values.insert(key.to_string(), value); + }) + } - let mut values = self.all_secrets()?; - values.insert(key.to_string(), serde_json::to_value(value)?); + /// Set multiple secret values with one storage read and one storage write. + /// + /// This is intended for provider setup flows that save several fields at once. + /// It keeps keychain access batched while preserving the same storage format as + /// `set_secret`. + pub fn set_secret_values(&self, updates: &[(String, Value)]) -> Result<(), ConfigError> { + if updates.is_empty() { + return Ok(()); + } - match &self.secrets { - SecretStorage::Keyring { service } => { - let json_value = serde_json::to_string(&values)?; - match self.handle_keyring_operation( - |entry| entry.set_password(&json_value), - service, - Some(&values), - ) { - Ok(_) => {} - Err(ConfigError::FallbackToFileStorage) => {} - Err(e) => return Err(e), - } + self.mutate_secrets(|values| { + for (key, value) in updates { + values.insert(key.clone(), value.clone()); } - SecretStorage::File { path } => { - let yaml_value = serde_yaml::to_string(&values)?; - write_secrets_file(path, &yaml_value)?; - } - }; - - self.invalidate_secrets_cache(); - - Ok(()) + }) } /// Delete a secret from the system keyring. @@ -826,34 +885,22 @@ impl Config { /// - There is an error accessing the keyring /// - There is an error serializing the remaining values pub fn delete_secret(&self, key: &str) -> Result<(), ConfigError> { - // Lock before reading to prevent race condition. - let _guard = self.guard.lock().unwrap(); + self.mutate_secrets(|values| { + values.remove(key); + }) + } - let mut values = self.all_secrets()?; - values.remove(key); + /// Delete multiple secret values with one storage read and one storage write. + pub fn delete_secret_values(&self, keys: &[String]) -> Result<(), ConfigError> { + if keys.is_empty() { + return Ok(()); + } - match &self.secrets { - SecretStorage::Keyring { service } => { - let json_value = serde_json::to_string(&values)?; - match self.handle_keyring_operation( - |entry| entry.set_password(&json_value), - service, - Some(&values), - ) { - Ok(_) => {} - Err(ConfigError::FallbackToFileStorage) => {} - Err(e) => return Err(e), - } + self.mutate_secrets(|values| { + for key in keys { + values.remove(key); } - SecretStorage::File { path } => { - let yaml_value = serde_yaml::to_string(&values)?; - write_secrets_file(path, &yaml_value)?; - } - }; - - self.invalidate_secrets_cache(); - - Ok(()) + }) } /// Read secrets from a YAML file diff --git a/crates/goose/src/providers/inventory/mod.rs b/crates/goose/src/providers/inventory/mod.rs index 7db08a06..716f5988 100644 --- a/crates/goose/src/providers/inventory/mod.rs +++ b/crates/goose/src/providers/inventory/mod.rs @@ -9,8 +9,8 @@ use serde::{Deserialize, Serialize}; use sha2::{Digest, Sha256}; use sqlx::{Pool, Row, Sqlite, Transaction}; use std::collections::{BTreeMap, HashMap, HashSet}; -use std::sync::Arc; -use tokio::sync::RwLock; +use std::sync::{Arc, PoisonError, RwLock, RwLockReadGuard, RwLockWriteGuard}; +use tracing::warn; const STALE_AFTER_HOURS: i64 = 24; @@ -62,7 +62,7 @@ pub struct InventoryModel { pub recommended: bool, } -#[derive(Debug, Clone)] +#[derive(Debug, Clone, PartialEq, Eq)] pub struct InventoryIdentity { pub provider_id: String, pub provider_family: String, @@ -143,18 +143,94 @@ pub struct RefreshSkip { pub reason: RefreshSkipReason, } +#[derive(Debug, Clone)] +pub(crate) struct RefreshJob { + pub provider_id: String, + pub identity: InventoryIdentity, +} + #[derive(Debug, Clone, Default)] pub struct RefreshPlan { pub started: Vec, pub skipped: Vec, } +#[derive(Debug, Clone, Default)] +pub(crate) struct RefreshJobPlan { + pub started: Vec, + pub skipped: Vec, +} + +impl RefreshJobPlan { + pub(crate) fn into_public_plan(self) -> RefreshPlan { + RefreshPlan { + started: self + .started + .into_iter() + .map(|job| job.provider_id) + .collect(), + skipped: self.skipped, + } + } +} + #[derive(Clone)] pub struct ProviderInventoryService { storage: Arc, refreshing_keys: Arc>>, } +pub(crate) struct RefreshGuard { + inventory_key: String, + refreshing_keys: Arc>>, + completed: bool, +} + +impl RefreshGuard { + /// Mark the refresh as finished and remove its inventory key from the + /// refreshing-keys set. `RefreshGuard` is the single owner of refresh-key + /// removal; store methods do not clear keys themselves. + pub fn complete(&mut self) { + if self.completed { + return; + } + let mut refreshing_keys = self + .refreshing_keys + .write() + .unwrap_or_else(|poisoned| recover_poisoned_write(poisoned, "refreshing_keys")); + refreshing_keys.remove(&self.inventory_key); + self.completed = true; + } +} + +impl Drop for RefreshGuard { + fn drop(&mut self) { + self.complete(); + } +} + +fn recover_poisoned_read<'a, T>( + poisoned: PoisonError>, + lock_name: &str, +) -> RwLockReadGuard<'a, T> { + warn!( + lock = lock_name, + "recovering poisoned provider inventory read lock" + ); + poisoned.into_inner() +} + +fn recover_poisoned_write<'a, T>( + poisoned: PoisonError>, + lock_name: &str, +) -> RwLockWriteGuard<'a, T> { + warn!( + lock = lock_name, + "recovering poisoned provider inventory write lock" + ); + poisoned.into_inner() +} + #[derive(Debug, Clone)] struct InventorySnapshot { models: Vec, @@ -198,7 +274,7 @@ impl ProviderInventoryService { let refreshing = self .refreshing_keys .read() - .await + .unwrap_or_else(|poisoned| recover_poisoned_read(poisoned, "refreshing_keys")) .contains(&descriptor.identity.inventory_key); let models = inventory_models_from_snapshot( snapshot.as_ref(), @@ -241,8 +317,18 @@ impl ProviderInventoryService { } pub async fn plan_refresh(&self, provider_ids: &[String]) -> Result { + self.plan_refresh_jobs(provider_ids) + .await + .map(RefreshJobPlan::into_public_plan) + } + + pub(crate) async fn plan_refresh_jobs( + &self, + provider_ids: &[String], + ) -> Result { let ids = self.resolve_provider_ids(provider_ids).await; - let mut plan = RefreshPlan::default(); + let mut plan = RefreshJobPlan::default(); + let mut inserted_refreshing = Vec::new(); for provider_id in ids { let Some(descriptor) = self.describe_provider(&provider_id).await? else { @@ -269,8 +355,20 @@ impl ProviderInventoryService { continue; } - let mut refreshing_keys = self.refreshing_keys.write().await; - if refreshing_keys.contains(&descriptor.identity.inventory_key) { + let already_refreshing = { + let mut refreshing_keys = self + .refreshing_keys + .write() + .unwrap_or_else(|poisoned| recover_poisoned_write(poisoned, "refreshing_keys")); + if refreshing_keys.contains(&descriptor.identity.inventory_key) { + true + } else { + refreshing_keys.insert(descriptor.identity.inventory_key.clone()); + false + } + }; + + if already_refreshing { plan.skipped.push(RefreshSkip { provider_id: descriptor.provider_id, reason: RefreshSkipReason::AlreadyRefreshing, @@ -278,11 +376,16 @@ impl ProviderInventoryService { continue; } - refreshing_keys.insert(descriptor.identity.inventory_key.clone()); - drop(refreshing_keys); + inserted_refreshing.push(descriptor.identity.clone()); + if let Err(error) = self.mark_refresh_started(&descriptor.identity).await { + self.clear_refreshing_many(&inserted_refreshing); + return Err(error); + } - self.mark_refresh_started(&descriptor.identity).await?; - plan.started.push(descriptor.provider_id); + plan.started.push(RefreshJob { + provider_id: descriptor.provider_id, + identity: descriptor.identity, + }); } Ok(plan) @@ -294,8 +397,18 @@ impl ProviderInventoryService { model_ids: &[String], ) -> Result<()> { let descriptor = self.require_provider(provider_id).await?; - let models = - enrich_model_ids_with_canonical(&descriptor.identity.provider_family, model_ids); + self.store_refreshed_models_for_identity(&descriptor.identity, model_ids) + .await?; + self.clear_refreshing_many(std::slice::from_ref(&descriptor.identity)); + Ok(()) + } + + pub(crate) async fn store_refreshed_models_for_identity( + &self, + identity: &InventoryIdentity, + model_ids: &[String], + ) -> Result<()> { + let models = enrich_model_ids_with_canonical(&identity.provider_family, model_ids); let now = Utc::now(); let pool = self.storage.pool().await?; let mut tx = pool.begin().await?; @@ -320,16 +433,16 @@ impl ProviderInventoryService { updated_at = CURRENT_TIMESTAMP "#, ) - .bind(&descriptor.identity.inventory_key) - .bind(&descriptor.identity.provider_id) - .bind(&descriptor.identity.provider_family) + .bind(&identity.inventory_key) + .bind(&identity.provider_id) + .bind(&identity.provider_family) .bind(now.to_rfc3339()) .bind(now.to_rfc3339()) .execute(&mut *tx) .await?; sqlx::query("DELETE FROM provider_inventory_models WHERE inventory_key = ?") - .bind(&descriptor.identity.inventory_key) + .bind(&identity.inventory_key) .execute(&mut *tx) .await?; @@ -348,7 +461,7 @@ impl ProviderInventoryService { ) VALUES (?, ?, ?, ?, ?, ?, ?, ?) "#, ) - .bind(&descriptor.identity.inventory_key) + .bind(&identity.inventory_key) .bind(i64::try_from(ordinal)?) .bind(&model.id) .bind(&model.name) @@ -361,10 +474,6 @@ impl ProviderInventoryService { } tx.commit().await?; - self.refreshing_keys - .write() - .await - .remove(&descriptor.identity.inventory_key); Ok(()) } @@ -374,8 +483,19 @@ impl ProviderInventoryService { error: impl Into, ) -> Result<()> { let descriptor = self.require_provider(provider_id).await?; + self.store_refresh_error_for_identity(&descriptor.identity, error) + .await?; + self.clear_refreshing_many(std::slice::from_ref(&descriptor.identity)); + Ok(()) + } + + pub(crate) async fn store_refresh_error_for_identity( + &self, + identity: &InventoryIdentity, + error: impl Into, + ) -> Result<()> { let error = error.into(); - let existing = self.read_snapshot(&descriptor.identity).await?; + let existing = self.read_snapshot(identity).await?; sqlx::query( r#" @@ -397,22 +517,36 @@ impl ProviderInventoryService { updated_at = CURRENT_TIMESTAMP "#, ) - .bind(&descriptor.identity.inventory_key) - .bind(&descriptor.identity.provider_id) - .bind(&descriptor.identity.provider_family) + .bind(&identity.inventory_key) + .bind(&identity.provider_id) + .bind(&identity.provider_family) .bind(existing.and_then(|snapshot| snapshot.last_updated_at.map(|time| time.to_rfc3339()))) .bind(Utc::now().to_rfc3339()) .bind(error) .execute(self.storage.pool().await?) .await?; - self.refreshing_keys - .write() - .await - .remove(&descriptor.identity.inventory_key); Ok(()) } + fn clear_refreshing_many(&self, identities: &[InventoryIdentity]) { + let mut refreshing_keys = self + .refreshing_keys + .write() + .unwrap_or_else(|poisoned| recover_poisoned_write(poisoned, "refreshing_keys")); + for identity in identities { + refreshing_keys.remove(&identity.inventory_key); + } + } + + pub(crate) fn refresh_guard(&self, identity: &InventoryIdentity) -> RefreshGuard { + RefreshGuard { + inventory_key: identity.inventory_key.clone(), + refreshing_keys: Arc::clone(&self.refreshing_keys), + completed: false, + } + } + pub fn is_stale(entry: &ProviderInventoryEntry) -> bool { let Some(last_updated_at) = entry.last_updated_at else { return false; @@ -930,6 +1064,80 @@ pub async fn create_tables_in_tx(tx: &mut Transaction<'_, Sqlite>) -> Result<()> mod tests { use super::*; + fn test_identity(provider_id: &str, inventory_key: &str) -> InventoryIdentity { + InventoryIdentity { + provider_id: provider_id.to_string(), + provider_family: provider_id.to_string(), + inventory_key: inventory_key.to_string(), + } + } + + #[test] + fn refresh_guard_complete_clears_refreshing_key() { + let refreshing_keys = Arc::new(RwLock::new(HashSet::from(["key-a".to_string()]))); + let mut guard = RefreshGuard { + inventory_key: "key-a".to_string(), + refreshing_keys: Arc::clone(&refreshing_keys), + completed: false, + }; + + guard.complete(); + guard.complete(); + + assert!(!refreshing_keys.read().unwrap().contains("key-a")); + } + + #[tokio::test] + async fn clear_refreshing_many_removes_all_inserted_keys() { + let service = + ProviderInventoryService::new(Arc::new(SessionStorage::new(std::env::temp_dir()))); + let left = test_identity("openai", "key-a"); + let right = test_identity("anthropic", "key-b"); + { + let mut refreshing_keys = service.refreshing_keys.write().unwrap(); + refreshing_keys.insert(left.inventory_key.clone()); + refreshing_keys.insert(right.inventory_key.clone()); + } + + service.clear_refreshing_many(&[left, right]); + + assert!(service.refreshing_keys.read().unwrap().is_empty()); + } + + #[tokio::test] + async fn identity_store_writes_to_captured_inventory_key() { + let temp_dir = tempfile::tempdir().unwrap(); + let service = ProviderInventoryService::new(Arc::new(SessionStorage::new( + temp_dir.path().to_path_buf(), + ))); + let plan_time_identity = test_identity("openai", "plan-time-key"); + let current_identity = test_identity("openai", "current-key"); + let sentinel_model = "stark-plan-time-model".to_string(); + + service + .store_refreshed_models_for_identity( + &plan_time_identity, + std::slice::from_ref(&sentinel_model), + ) + .await + .unwrap(); + + let plan_time_snapshot = service + .read_snapshot(&plan_time_identity) + .await + .unwrap() + .unwrap(); + assert!(plan_time_snapshot + .models + .iter() + .any(|model| model.id == sentinel_model)); + assert!(service + .read_snapshot(¤t_identity) + .await + .unwrap() + .is_none()); + } + #[test] fn inventory_identity_hash_changes_with_secret_inputs() { let left = InventoryIdentityInput::new("openai", "openai") diff --git a/crates/goose/tests/acp_secret_cache_invalidation_test.rs b/crates/goose/tests/acp_secret_cache_invalidation_test.rs new file mode 100644 index 00000000..31271431 --- /dev/null +++ b/crates/goose/tests/acp_secret_cache_invalidation_test.rs @@ -0,0 +1,314 @@ +#[allow(dead_code)] +#[path = "acp_common_tests/mod.rs"] +mod common_tests; + +use common_tests::fixtures::server::AcpServerConnection; +use common_tests::fixtures::{run_test, send_custom, Connection, TestConnectionConfig}; +use goose::config::paths::Paths; +use goose::config::{Config, ConfigError}; +use goose::model::ModelConfig; +use goose::providers::base::{MessageStream, Provider}; +use goose::providers::errors::ProviderError; +use goose::providers::inventory::ProviderInventoryService; +use goose::session::session_manager::SessionStorage; +use goose_test_support::EnforceSessionId; +use serial_test::serial; +use std::sync::Arc; + +struct MockProvider { + name: String, + model_config: ModelConfig, +} + +#[async_trait::async_trait] +impl Provider for MockProvider { + fn get_name(&self) -> &str { + &self.name + } + + async fn stream( + &self, + _model_config: &ModelConfig, + _session_id: &str, + _system: &str, + _messages: &[goose::conversation::message::Message], + _tools: &[rmcp::model::Tool], + ) -> Result { + unimplemented!() + } + + fn get_model_config(&self) -> ModelConfig { + self.model_config.clone() + } + + async fn fetch_recommended_models(&self) -> Result, ProviderError> { + Ok(vec!["claude-3-5-haiku-latest".to_string()]) + } +} + +fn mock_provider_factory() -> goose::acp::server::AcpProviderFactory { + Arc::new(|provider_name, model_config, _extensions| { + Box::pin(async move { + Ok(Arc::new(MockProvider { + name: provider_name, + model_config, + }) as Arc) + }) + }) +} + +fn write_config(config_dir: &std::path::Path) { + std::fs::create_dir_all(config_dir).unwrap(); + std::fs::write( + config_dir.join(goose::config::base::CONFIG_YAML_NAME), + "GOOSE_MODEL: gpt-4o\nGOOSE_PROVIDER: openai\nGOOSE_DISABLE_KEYRING: true\n", + ) + .unwrap(); +} + +fn write_secrets(config_dir: &std::path::Path, secrets: &str) { + std::fs::write(config_dir.join("secrets.yaml"), secrets).unwrap(); +} + +#[test] +#[serial] +fn acp_secret_mutations_and_inventory_refresh_invalidate_global_secret_cache() { + let root = tempfile::tempdir().unwrap(); + let root_path = root.path().to_string_lossy().to_string(); + let _env = env_lock::lock_env([ + ("GOOSE_PATH_ROOT", Some(root_path.as_str())), + ("GOOSE_DISABLE_KEYRING", Some("1")), + ("ANTHROPIC_API_KEY", None), + ("OPENAI_API_KEY", None), + ("XAI_API_KEY", None), + ("XAI_HOST", None), + ]); + + let config_dir = Paths::config_dir(); + let data_dir = Paths::data_dir(); + write_config(&config_dir); + write_secrets(&config_dir, "OPENAI_API_KEY: stale-key\n"); + + run_test(async move { + assert_eq!( + Config::global() + .get_secret::("OPENAI_API_KEY") + .unwrap(), + "stale-key" + ); + + let openai = common_tests::fixtures::OpenAiFixture::new( + vec![], + Arc::new(EnforceSessionId::default()), + ) + .await; + let config = TestConnectionConfig { + data_root: config_dir.clone(), + provider_factory: Some(mock_provider_factory()), + ..Default::default() + }; + let conn = AcpServerConnection::new(config, openai).await; + + write_secrets(&config_dir, "OPENAI_API_KEY: fresh-key\n"); + send_custom( + conn.cx(), + "_goose/secret/upsert", + serde_json::json!({ + "key": "OPENAI_API_KEY", + "value": "fresh-key", + }), + ) + .await + .expect("secret upsert should succeed"); + + assert_eq!( + Config::global() + .get_secret::("OPENAI_API_KEY") + .unwrap(), + "fresh-key", + "ACP secret upsert should invalidate the global secrets cache" + ); + + write_secrets(&config_dir, "{}\n"); + send_custom( + conn.cx(), + "_goose/secret/remove", + serde_json::json!({ + "key": "OPENAI_API_KEY", + }), + ) + .await + .expect("secret remove should succeed"); + + assert!( + matches!( + Config::global().get_secret::("OPENAI_API_KEY"), + Err(ConfigError::NotFound(_)) + ), + "ACP secret remove should invalidate the global secrets cache" + ); + + let save_provider_config = send_custom( + conn.cx(), + "_goose/providers/config/save", + serde_json::json!({ + "providerId": "xai", + "fields": [ + { + "key": "XAI_API_KEY", + "value": "xai-provider-config-key", + }, + { + "key": "XAI_HOST", + "value": "https://api.x.ai/v1", + }, + ], + }), + ) + .await + .expect("provider config save should succeed"); + assert_eq!( + save_provider_config.get("status"), + Some(&serde_json::json!({ + "providerId": "xai", + "isConfigured": true, + })), + "provider config save should return the updated configured status" + ); + assert_eq!( + save_provider_config.get("refresh"), + Some(&serde_json::json!({ + "started": [], + "skipped": [ + { + "providerId": "xai", + "reason": "does_not_support_refresh", + }, + ], + })), + "provider config save should return the inventory refresh acknowledgement" + ); + assert_eq!( + Config::global() + .get_secret::("XAI_API_KEY") + .unwrap(), + "xai-provider-config-key", + "provider config save should invalidate the global secrets cache" + ); + + let read_provider_config = send_custom( + conn.cx(), + "_goose/providers/config/read", + serde_json::json!({ + "providerId": "xai", + }), + ) + .await + .expect("provider config read should succeed"); + let fields = read_provider_config + .get("fields") + .and_then(|fields| fields.as_array()) + .expect("provider config read should return fields"); + let api_key_field = fields + .iter() + .find(|field| field.get("key") == Some(&serde_json::json!("XAI_API_KEY"))) + .expect("provider config read should include the API key"); + assert_eq!(api_key_field.get("isSet"), Some(&serde_json::json!(true))); + assert_ne!( + api_key_field.get("value"), + Some(&serde_json::json!("xai-provider-config-key")), + "provider config read should mask secret values" + ); + + let delete_provider_config = send_custom( + conn.cx(), + "_goose/providers/config/delete", + serde_json::json!({ + "providerId": "xai", + }), + ) + .await + .expect("provider config delete should succeed"); + assert_eq!( + delete_provider_config.get("status"), + Some(&serde_json::json!({ + "providerId": "xai", + "isConfigured": false, + })), + "provider config delete should return the updated configured status" + ); + assert!( + matches!( + Config::global().get_secret::("XAI_API_KEY"), + Err(ConfigError::NotFound(_)) + ), + "provider config delete should invalidate the global secrets cache" + ); + + Config::global().invalidate_secrets_cache(); + assert!(Config::global() + .get_secret::("ANTHROPIC_API_KEY") + .is_err()); + + write_secrets(&config_dir, "ANTHROPIC_API_KEY: anthropic-key\n"); + + let refresh = send_custom( + conn.cx(), + "_goose/providers/inventory/refresh", + serde_json::json!({ + "providerIds": ["anthropic"], + }), + ) + .await + .expect("inventory refresh should succeed"); + + assert_eq!( + refresh.get("started"), + Some(&serde_json::json!(["anthropic"])), + "inventory refresh should invalidate the global secrets cache before planning" + ); + + write_secrets(&config_dir, "OPENAI_API_KEY: plan-time-key\n"); + Config::global().invalidate_secrets_cache(); + + let inventory = ProviderInventoryService::new(Arc::new(SessionStorage::new(data_dir))); + let plan = inventory + .plan_refresh(&["openai".to_string()]) + .await + .expect("plan refresh should start for configured OpenAI provider"); + assert_eq!(plan.started, vec!["openai".to_string()]); + + let entry_during_refresh = inventory + .entry_for_provider("openai") + .await + .expect("entry should load while refresh is in progress") + .expect("OpenAI inventory entry should exist"); + assert!( + entry_during_refresh.refreshing, + "plan refresh should mark the plan-time identity as refreshing" + ); + + let sentinel_model = "stark-plan-time-model".to_string(); + inventory + .store_refreshed_models("openai", std::slice::from_ref(&sentinel_model)) + .await + .expect("public store_refreshed_models compatibility wrapper should succeed"); + + let plan_time_entry = inventory + .entry_for_provider("openai") + .await + .expect("entry should load for plan-time credentials") + .expect("OpenAI inventory entry should exist for plan-time credentials"); + assert!( + !plan_time_entry.refreshing, + "store with captured identity should clear the plan-time refreshing key" + ); + assert!( + plan_time_entry + .models + .iter() + .any(|model| model.id == sentinel_model), + "models should be stored under the identity captured at plan time" + ); + }); +} diff --git a/documentation/docs/guides/config-files.md b/documentation/docs/guides/config-files.md index ef620613..023ae585 100644 --- a/documentation/docs/guides/config-files.md +++ b/documentation/docs/guides/config-files.md @@ -194,7 +194,7 @@ Settings are applied in the following order of precedence: ## Updating Configuration -Changes to config files require restarting goose to take effect. You can verify your current configuration using: +Direct edits to config files usually require restarting goose to take effect for existing sessions. Goose2 provider credential/config saves made through Settings use ACP/core to update storage and refresh provider inventory without restarting the app, but currently active chat sessions continue using the provider instance they started with. You can verify your current configuration using: ```bash goose info -v diff --git a/ui/goose2/AGENTS.md b/ui/goose2/AGENTS.md index da4382ad..481f6e3a 100644 --- a/ui/goose2/AGENTS.md +++ b/ui/goose2/AGENTS.md @@ -171,11 +171,14 @@ For a minimal frontend `api/` wrapper using the typed shape, see `ui/goose2/src/ ### When `invoke()` is still appropriate -Tauri commands (`invoke()` from `@tauri-apps/api/core`) are reserved for things that genuinely belong to the desktop shell, not to `goose` core. In practice that means: +Tauri commands (`invoke()` from `@tauri-apps/api/core`) are reserved for things that genuinely belong to the desktop shell, not to `goose` core. Provider config or secret mutations that affect the Goose runtime must flow through React → SDK → ACP → goose core so core can validate provider metadata, invalidate secret caches, refresh inventory, and apply provider changes consistently. In practice, Tauri is limited to: - `get_goose_serve_url` — bootstrapping the ACP connection. -- Secret storage owned by the OS keychain (e.g. `save_provider_field`, `delete_provider_config` — note dictation still uses these for writing API keys into the OS keychain, because that's a shell concern). +- Native auth subprocesses and desktop-shell side effects. - Window state, filesystem dialogs, and other Tauri-plugin-backed capabilities. +- Transitional provider cleanup such as `delete_provider_config` while local OAuth/cache side effects still live in the shell. `get_provider_config`, `check_all_provider_status`, and provider deletion duplicate provider/config knowledge in Tauri today and should move behind provider-scoped ACP methods. + +The long-term provider-config API should be provider-scoped rather than a frontend composition of generic config/secret writes. Prefer methods such as `_goose/providers/config/read`, `_goose/providers/config/status`, `_goose/providers/config/save`, and `_goose/providers/config/delete`; inventory refresh can be part of the result, but active provider reload belongs in the config mutation/apply path, not in `_goose/providers/inventory/refresh`. If the thing you're building is "get data from goose" or "tell goose to do something," it is **not** one of these cases. Add a custom ACP method instead. diff --git a/ui/goose2/src-tauri/Cargo.lock b/ui/goose2/src-tauri/Cargo.lock index 6fe4f563..f3ee8a4b 100644 --- a/ui/goose2/src-tauri/Cargo.lock +++ b/ui/goose2/src-tauri/Cargo.lock @@ -1152,17 +1152,6 @@ dependencies = [ "windows-sys 0.61.2", ] -[[package]] -name = "etcetera" -version = "0.8.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "136d1b5283a1ab77bd9257427ffd09d8667ced0570b6f938942bc7568ed5b943" -dependencies = [ - "cfg-if", - "home", - "windows-sys 0.48.0", -] - [[package]] name = "event-listener" version = "5.4.1" @@ -1670,7 +1659,6 @@ dependencies = [ "chrono", "dirs", "doctor", - "etcetera", "ignore", "keyring", "log", @@ -1809,15 +1797,6 @@ dependencies = [ "digest", ] -[[package]] -name = "home" -version = "0.5.12" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "cc627f471c528ff0c4a49e1d5e60450c8f6461dd6d10ba9dcd3a61d3dff7728d" -dependencies = [ - "windows-sys 0.61.2", -] - [[package]] name = "html5ever" version = "0.29.1" @@ -5829,15 +5808,6 @@ dependencies = [ "windows-targets 0.42.2", ] -[[package]] -name = "windows-sys" -version = "0.48.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "677d2418bec65e3338edb076e806bc1ec15693c5d0104683f2efe857f61056a9" -dependencies = [ - "windows-targets 0.48.5", -] - [[package]] name = "windows-sys" version = "0.52.0" @@ -5889,21 +5859,6 @@ dependencies = [ "windows_x86_64_msvc 0.42.2", ] -[[package]] -name = "windows-targets" -version = "0.48.5" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "9a2fa6e2155d7247be68c096456083145c183cbbbc2764150dda45a87197940c" -dependencies = [ - "windows_aarch64_gnullvm 0.48.5", - "windows_aarch64_msvc 0.48.5", - "windows_i686_gnu 0.48.5", - "windows_i686_msvc 0.48.5", - "windows_x86_64_gnu 0.48.5", - "windows_x86_64_gnullvm 0.48.5", - "windows_x86_64_msvc 0.48.5", -] - [[package]] name = "windows-targets" version = "0.52.6" @@ -5961,12 +5916,6 @@ version = "0.42.2" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "597a5118570b68bc08d8d59125332c54f1ba9d9adeedeef5b99b02ba2b0698f8" -[[package]] -name = "windows_aarch64_gnullvm" -version = "0.48.5" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "2b38e32f0abccf9987a4e3079dfb67dcd799fb61361e53e2882c3cbaf0d905d8" - [[package]] name = "windows_aarch64_gnullvm" version = "0.52.6" @@ -5985,12 +5934,6 @@ version = "0.42.2" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "e08e8864a60f06ef0d0ff4ba04124db8b0fb3be5776a5cd47641e942e58c4d43" -[[package]] -name = "windows_aarch64_msvc" -version = "0.48.5" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "dc35310971f3b2dbbf3f0690a219f40e2d9afcf64f9ab7cc1be722937c26b4bc" - [[package]] name = "windows_aarch64_msvc" version = "0.52.6" @@ -6009,12 +5952,6 @@ version = "0.42.2" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "c61d927d8da41da96a81f029489353e68739737d3beca43145c8afec9a31a84f" -[[package]] -name = "windows_i686_gnu" -version = "0.48.5" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "a75915e7def60c94dcef72200b9a8e58e5091744960da64ec734a6c6e9b3743e" - [[package]] name = "windows_i686_gnu" version = "0.52.6" @@ -6045,12 +5982,6 @@ version = "0.42.2" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "44d840b6ec649f480a41c8d80f9c65108b92d89345dd94027bfe06ac444d1060" -[[package]] -name = "windows_i686_msvc" -version = "0.48.5" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "8f55c233f70c4b27f66c523580f78f1004e8b5a8b659e05a4eb49d4166cca406" - [[package]] name = "windows_i686_msvc" version = "0.52.6" @@ -6069,12 +6000,6 @@ version = "0.42.2" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "8de912b8b8feb55c064867cf047dda097f92d51efad5b491dfb98f6bbb70cb36" -[[package]] -name = "windows_x86_64_gnu" -version = "0.48.5" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "53d40abd2583d23e4718fddf1ebec84dbff8381c07cae67ff7768bbf19c6718e" - [[package]] name = "windows_x86_64_gnu" version = "0.52.6" @@ -6093,12 +6018,6 @@ version = "0.42.2" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "26d41b46a36d453748aedef1486d5c7a85db22e56aff34643984ea85514e94a3" -[[package]] -name = "windows_x86_64_gnullvm" -version = "0.48.5" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "0b7b52767868a23d5bab768e390dc5f5c55825b6d30b86c844ff2dc7414044cc" - [[package]] name = "windows_x86_64_gnullvm" version = "0.52.6" @@ -6117,12 +6036,6 @@ version = "0.42.2" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "9aec5da331524158c6d1a4ac0ab1541149c0b9505fde06423b02f5ef0106b9f0" -[[package]] -name = "windows_x86_64_msvc" -version = "0.48.5" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "ed94fce61571a4006852b7389a063ab983c02eb1bb37b47f8272ce92d06d9538" - [[package]] name = "windows_x86_64_msvc" version = "0.52.6" diff --git a/ui/goose2/src-tauri/Cargo.toml b/ui/goose2/src-tauri/Cargo.toml index 12376ef6..233b8112 100644 --- a/ui/goose2/src-tauri/Cargo.toml +++ b/ui/goose2/src-tauri/Cargo.toml @@ -31,7 +31,6 @@ tokio = { version = "1.50.0", features = ["full"] } uuid = { version = "1", features = ["v4", "serde"] } chrono = { version = "0.4", features = ["serde"] } serde_yaml = "0.9" -etcetera = "0.8" doctor = { git = "https://github.com/block/builderbot", rev = "8e1c3ec145edc0df5f04b4427cfd758378036862" } ignore = "0.4.25" base64 = "0.22" diff --git a/ui/goose2/src-tauri/src/commands/credentials.rs b/ui/goose2/src-tauri/src/commands/credentials.rs deleted file mode 100644 index 94b06300..00000000 --- a/ui/goose2/src-tauri/src/commands/credentials.rs +++ /dev/null @@ -1,50 +0,0 @@ -use tauri::State; - -use crate::services::goose_config::{FieldValue, GooseConfig, ProviderStatus}; -use crate::services::provider_defs::find_config_key; - -#[tauri::command] -pub fn get_provider_config( - config: State<'_, GooseConfig>, - provider_id: String, -) -> Result, String> { - config.get_provider_field_values(&provider_id) -} - -#[tauri::command] -pub fn save_provider_field( - config: State<'_, GooseConfig>, - key: String, - value: String, -) -> Result<(), String> { - let config_key = - find_config_key(&key).ok_or_else(|| format!("Unknown provider config key '{key}'"))?; - let trimmed_value = value.trim(); - if trimmed_value.is_empty() { - return Err("Field value cannot be empty".to_string()); - } - - if config_key.is_secret { - config.set_secret(&key, trimmed_value) - } else { - config.set_param(&key, trimmed_value) - } -} - -#[tauri::command] -pub fn delete_provider_config( - config: State<'_, GooseConfig>, - provider_id: String, -) -> Result<(), String> { - config.delete_all_provider_fields(&provider_id) -} - -#[tauri::command] -pub fn check_all_provider_status(config: State<'_, GooseConfig>) -> Vec { - config.check_all_provider_status() -} - -#[tauri::command] -pub fn restart_app(app: tauri::AppHandle) { - app.restart(); -} diff --git a/ui/goose2/src-tauri/src/commands/mod.rs b/ui/goose2/src-tauri/src/commands/mod.rs index 51e933f6..7de2490a 100644 --- a/ui/goose2/src-tauri/src/commands/mod.rs +++ b/ui/goose2/src-tauri/src/commands/mod.rs @@ -1,7 +1,6 @@ pub mod acp; pub mod agent_setup; pub mod agents; -pub mod credentials; pub mod doctor; pub mod git; pub mod git_changes; diff --git a/ui/goose2/src-tauri/src/lib.rs b/ui/goose2/src-tauri/src/lib.rs index e82d11fa..fff92e60 100644 --- a/ui/goose2/src-tauri/src/lib.rs +++ b/ui/goose2/src-tauri/src/lib.rs @@ -2,7 +2,6 @@ mod commands; mod services; mod types; -use services::goose_config::GooseConfig; use services::personas::PersonaStore; use tauri_plugin_window_state::StateFlags; @@ -25,8 +24,7 @@ pub fn run() { .with_state_flags(StateFlags::all() & !StateFlags::VISIBLE) .build(), ) - .manage(PersonaStore::new()) - .manage(GooseConfig::new()); + .manage(PersonaStore::new()); #[cfg(feature = "app-test-driver")] let builder = builder.plugin(tauri_plugin_app_test_driver::init()); @@ -65,11 +63,6 @@ pub fn run() { commands::git::git_pull, commands::git::git_create_branch, commands::git::git_create_worktree, - commands::credentials::get_provider_config, - commands::credentials::save_provider_field, - commands::credentials::delete_provider_config, - commands::credentials::check_all_provider_status, - commands::credentials::restart_app, commands::model_setup::authenticate_model_provider, commands::agent_setup::check_agent_installed, commands::agent_setup::check_agent_auth, diff --git a/ui/goose2/src-tauri/src/services/goose_config.rs b/ui/goose2/src-tauri/src/services/goose_config.rs deleted file mode 100644 index b77b154d..00000000 --- a/ui/goose2/src-tauri/src/services/goose_config.rs +++ /dev/null @@ -1,452 +0,0 @@ -use std::collections::HashMap; -use std::path::PathBuf; -use std::sync::Mutex; - -use etcetera::{choose_app_strategy, AppStrategy, AppStrategyArgs}; -use serde::Serialize; -use serde_json::Value; - -use super::provider_defs::{find_provider_def, PROVIDER_CONFIG_DEFS}; - -const KEYRING_SERVICE: &str = "goose"; -const KEYRING_USERNAME: &str = "secrets"; -const CONFIG_YAML_NAME: &str = "config.yaml"; -const SECRETS_YAML_NAME: &str = "secrets.yaml"; -const SECRET_MASK_PREFIX_LEN: usize = 4; -const SECRET_MASK_SUFFIX_LEN: usize = 3; -const SECRET_MASK_FALLBACK: &str = "***"; - -#[derive(Clone, Debug, Serialize)] -#[serde(rename_all = "camelCase")] -pub struct ProviderStatus { - pub provider_id: String, - pub is_configured: bool, -} - -#[derive(Clone, Debug, Serialize)] -#[serde(rename_all = "camelCase")] -pub struct FieldValue { - pub key: String, - pub value: Option, - pub is_set: bool, - pub is_secret: bool, - pub required: bool, -} - -pub struct GooseConfig { - config_dir: PathBuf, - guard: Mutex<()>, -} - -impl GooseConfig { - pub fn new() -> Self { - let config_dir = Self::resolve_config_dir(); - log::info!("GooseConfig using config dir: {}", config_dir.display()); - Self { - config_dir, - guard: Mutex::new(()), - } - } - - fn resolve_config_dir() -> PathBuf { - if let Ok(root) = std::env::var("GOOSE_PATH_ROOT") { - return PathBuf::from(root).join("config"); - } - - let strategy = choose_app_strategy(AppStrategyArgs { - top_level_domain: "Block".to_string(), - author: "Block".to_string(), - app_name: "goose".to_string(), - }) - .expect("goose requires a home dir"); - - strategy.config_dir() - } - - fn read_config_map(&self) -> serde_yaml::Mapping { - let config_path = self.config_dir.join(CONFIG_YAML_NAME); - match std::fs::read_to_string(&config_path) { - Ok(contents) => { - serde_yaml::from_str::(&contents).unwrap_or_default() - } - Err(_) => serde_yaml::Mapping::new(), - } - } - - fn write_config_map(&self, config: &serde_yaml::Mapping) -> Result<(), String> { - let path = self.config_dir.join(CONFIG_YAML_NAME); - if let Some(parent) = path.parent() { - let _ = std::fs::create_dir_all(parent); - } - - let yaml = serde_yaml::to_string(config) - .map_err(|e| format!("Failed to serialize config: {e}"))?; - - std::fs::write(&path, yaml).map_err(|e| format!("Failed to write config file: {e}")) - } - - fn get_secret(&self, key: &str) -> Option { - let env_key = key.to_uppercase(); - if let Ok(value) = std::env::var(&env_key) { - return Some(value); - } - - let secrets = self.all_secrets(); - secrets - .get(key) - .and_then(|value| value.as_str().map(|secret| secret.to_string())) - } - - fn mask_secret_value(value: &str) -> String { - let prefix: String = value.chars().take(SECRET_MASK_PREFIX_LEN).collect(); - let suffix_chars: Vec = value.chars().rev().take(SECRET_MASK_SUFFIX_LEN).collect(); - let suffix: String = suffix_chars.into_iter().rev().collect(); - - if prefix.is_empty() - || suffix.is_empty() - || value.chars().count() <= SECRET_MASK_PREFIX_LEN + SECRET_MASK_SUFFIX_LEN - { - return SECRET_MASK_FALLBACK.to_string(); - } - - format!("{prefix}...{suffix}") - } - - fn has_param(&self, key: &str) -> bool { - self.get_param(key).is_some() - } - - fn has_secret(&self, key: &str) -> bool { - self.get_secret(key).is_some() - } - - pub fn get_param(&self, key: &str) -> Option { - let env_key = key.to_uppercase(); - if let Ok(value) = std::env::var(&env_key) { - return Some(value); - } - - let config = self.read_config_map(); - let yaml_key = serde_yaml::Value::String(key.to_string()); - config - .get(&yaml_key) - .and_then(|value| value.as_str().map(|param| param.to_string())) - } - - pub fn get_secret_masked(&self, key: &str) -> Option { - self.get_secret(key) - .map(|value| Self::mask_secret_value(&value)) - } - - pub fn set_param(&self, key: &str, value: &str) -> Result<(), String> { - let _guard = self.guard.lock().unwrap(); - - let mut config = self.read_config_map(); - config.insert( - serde_yaml::Value::String(key.to_string()), - serde_yaml::Value::String(value.to_string()), - ); - - self.write_config_map(&config) - } - - pub fn delete_param(&self, key: &str) -> Result { - let _guard = self.guard.lock().unwrap(); - - let mut config = self.read_config_map(); - let yaml_key = serde_yaml::Value::String(key.to_string()); - let removed = config.remove(&yaml_key).is_some(); - - if removed { - self.write_config_map(&config)?; - } - - Ok(removed) - } - - pub fn set_secret(&self, key: &str, value: &str) -> Result<(), String> { - let _guard = self.guard.lock().unwrap(); - - let mut secrets = self.all_secrets(); - secrets.insert(key.to_string(), Value::String(value.to_string())); - - if self.is_keyring_disabled() { - self.write_secrets_to_file(&secrets) - } else { - self.write_secrets_to_keyring(&secrets) - } - } - - pub fn delete_secret(&self, key: &str) -> Result { - let _guard = self.guard.lock().unwrap(); - - let mut secrets = self.all_secrets(); - let removed = secrets.remove(key).is_some(); - - if removed { - if self.is_keyring_disabled() { - self.write_secrets_to_file(&secrets)?; - } else { - self.write_secrets_to_keyring(&secrets)?; - } - } - - Ok(removed) - } - - fn has_oauth_cache(&self, cache_path: &str) -> bool { - let full_path = self.config_dir.join(cache_path); - if full_path.is_dir() { - std::fs::read_dir(&full_path) - .map(|mut entries| entries.any(|_| true)) - .unwrap_or(false) - } else { - full_path.exists() - } - } - - fn delete_oauth_cache(&self, cache_path: &str) -> Result { - let _guard = self.guard.lock().unwrap(); - let full_path = self.config_dir.join(cache_path); - - if full_path.is_dir() { - std::fs::remove_dir_all(&full_path) - .map_err(|e| format!("Failed to remove OAuth cache directory: {e}"))?; - return Ok(true); - } - - if full_path.exists() { - std::fs::remove_file(&full_path) - .map_err(|e| format!("Failed to remove OAuth cache file: {e}"))?; - return Ok(true); - } - - Ok(false) - } - - pub fn check_provider_status(&self, provider_id: &str) -> ProviderStatus { - if provider_id == "databricks" { - let has_host = self.has_param("DATABRICKS_HOST"); - let has_token = self.has_secret("DATABRICKS_TOKEN"); - let has_oauth = self.has_oauth_cache("databricks/oauth"); - - return ProviderStatus { - provider_id: provider_id.to_string(), - is_configured: has_host && (has_token || has_oauth), - }; - } - - let def = match find_provider_def(provider_id) { - Some(def) => def, - None => { - return ProviderStatus { - provider_id: provider_id.to_string(), - is_configured: false, - } - } - }; - - let has_oauth = def - .oauth_cache_path - .map(|p| self.has_oauth_cache(p)) - .unwrap_or(false); - - if has_oauth { - return ProviderStatus { - provider_id: provider_id.to_string(), - is_configured: true, - }; - } - - let all_required_present = def.keys.iter().all(|k| { - if !k.required { - return true; - } - if k.is_secret { - self.has_secret(k.name) - } else { - self.has_param(k.name) - } - }); - - let has_any_key = def.keys.iter().any(|k| { - if k.is_secret { - self.has_secret(k.name) - } else { - self.has_param(k.name) - } - }); - - let is_configured = if def.keys.is_empty() { - false - } else { - all_required_present && has_any_key - }; - - ProviderStatus { - provider_id: provider_id.to_string(), - is_configured, - } - } - - pub fn check_all_provider_status(&self) -> Vec { - PROVIDER_CONFIG_DEFS - .iter() - .map(|def| self.check_provider_status(def.id)) - .collect() - } - - pub fn get_provider_field_values(&self, provider_id: &str) -> Result, String> { - let def = find_provider_def(provider_id) - .ok_or_else(|| format!("Unknown provider '{provider_id}'"))?; - - Ok(def - .keys - .iter() - .map(|config_key| { - let value = if config_key.is_secret { - self.get_secret_masked(config_key.name) - } else { - self.get_param(config_key.name) - }; - - FieldValue { - key: config_key.name.to_string(), - is_set: value.is_some(), - value, - is_secret: config_key.is_secret, - required: config_key.required, - } - }) - .collect()) - } - - pub fn delete_all_provider_fields(&self, provider_id: &str) -> Result<(), String> { - let def = find_provider_def(provider_id) - .ok_or_else(|| format!("Unknown provider '{provider_id}'"))?; - - for config_key in def.keys { - if config_key.is_secret { - self.delete_secret(config_key.name)?; - } else { - self.delete_param(config_key.name)?; - } - } - - if let Some(oauth_cache_path) = def.oauth_cache_path { - self.delete_oauth_cache(oauth_cache_path)?; - } - - Ok(()) - } - - fn is_keyring_disabled(&self) -> bool { - if std::env::var("GOOSE_DISABLE_KEYRING").is_ok() { - return true; - } - - let config_path = self.config_dir.join(CONFIG_YAML_NAME); - if let Ok(contents) = std::fs::read_to_string(&config_path) { - if let Ok(yaml) = serde_yaml::from_str::(&contents) { - let key = serde_yaml::Value::String("GOOSE_DISABLE_KEYRING".to_string()); - if let Some(val) = yaml.get(&key) { - return val.as_bool().unwrap_or(false) - || val - .as_str() - .map(|s| s == "true" || s == "1") - .unwrap_or(false); - } - } - } - - false - } - - fn all_secrets(&self) -> HashMap { - if self.is_keyring_disabled() { - return self.read_secrets_from_file(); - } - - match self.read_secrets_from_keyring() { - Ok(secrets) => secrets, - Err(e) => { - log::warn!("Keyring read failed, falling back to secrets file: {e}"); - self.read_secrets_from_file() - } - } - } - - fn read_secrets_from_keyring(&self) -> Result, String> { - let entry = keyring::Entry::new(KEYRING_SERVICE, KEYRING_USERNAME) - .map_err(|e| format!("Failed to access keyring: {e}"))?; - - match entry.get_password() { - Ok(json_str) => serde_json::from_str(&json_str) - .map_err(|e| format!("Failed to parse keyring JSON: {e}")), - Err(keyring::Error::NoEntry) => Ok(HashMap::new()), - Err(e) => Err(format!("Failed to read keyring: {e}")), - } - } - - fn read_secrets_from_file(&self) -> HashMap { - let path = self.config_dir.join(SECRETS_YAML_NAME); - match std::fs::read_to_string(&path) { - Ok(contents) => serde_yaml::from_str::>(&contents) - .unwrap_or_default() - .into_iter() - .map(|(k, v)| (k, Value::String(v))) - .collect(), - Err(_) => HashMap::new(), - } - } - - fn write_secrets_to_keyring(&self, secrets: &HashMap) -> Result<(), String> { - let entry = keyring::Entry::new(KEYRING_SERVICE, KEYRING_USERNAME) - .map_err(|e| format!("Failed to access keyring: {e}"))?; - - let json_str = serde_json::to_string(secrets) - .map_err(|e| format!("Failed to serialize secrets: {e}"))?; - - entry - .set_password(&json_str) - .map_err(|e| format!("Failed to write to keyring: {e}")) - } - - fn write_secrets_to_file(&self, secrets: &HashMap) -> Result<(), String> { - let path = self.config_dir.join(SECRETS_YAML_NAME); - if let Some(parent) = path.parent() { - let _ = std::fs::create_dir_all(parent); - } - - let yaml_map: HashMap = secrets - .iter() - .filter_map(|(k, v)| v.as_str().map(|s| (k.clone(), s.to_string()))) - .collect(); - - let yaml = serde_yaml::to_string(&yaml_map) - .map_err(|e| format!("Failed to serialize secrets: {e}"))?; - - #[cfg(unix)] - { - use std::fs::OpenOptions; - use std::io::Write; - use std::os::unix::fs::OpenOptionsExt; - - let mut file = OpenOptions::new() - .write(true) - .create(true) - .truncate(true) - .mode(0o600) - .open(&path) - .map_err(|e| format!("Failed to open secrets file: {e}"))?; - - file.write_all(yaml.as_bytes()) - .map_err(|e| format!("Failed to write secrets file: {e}")) - } - - #[cfg(not(unix))] - { - std::fs::write(&path, yaml).map_err(|e| format!("Failed to write secrets file: {e}")) - } - } -} diff --git a/ui/goose2/src-tauri/src/services/mod.rs b/ui/goose2/src-tauri/src/services/mod.rs index 77759b3a..c0b447c6 100644 --- a/ui/goose2/src-tauri/src/services/mod.rs +++ b/ui/goose2/src-tauri/src/services/mod.rs @@ -1,4 +1,2 @@ pub mod acp; -pub mod goose_config; pub mod personas; -pub(crate) mod provider_defs; diff --git a/ui/goose2/src-tauri/src/services/provider_defs.rs b/ui/goose2/src-tauri/src/services/provider_defs.rs deleted file mode 100644 index 5eea0c0a..00000000 --- a/ui/goose2/src-tauri/src/services/provider_defs.rs +++ /dev/null @@ -1,150 +0,0 @@ -pub(crate) struct ConfigKey { - pub name: &'static str, - pub is_secret: bool, - pub required: bool, -} - -pub(crate) struct ProviderConfigDef { - pub id: &'static str, - pub keys: &'static [ConfigKey], - pub oauth_cache_path: Option<&'static str>, -} - -const fn key(name: &'static str, is_secret: bool, required: bool) -> ConfigKey { - ConfigKey { - name, - is_secret, - required, - } -} - -pub(crate) static PROVIDER_CONFIG_DEFS: &[ProviderConfigDef] = &[ - ProviderConfigDef { - id: "anthropic", - keys: &[key("ANTHROPIC_API_KEY", true, true)], - oauth_cache_path: None, - }, - ProviderConfigDef { - id: "openai", - keys: &[key("OPENAI_API_KEY", true, true)], - oauth_cache_path: None, - }, - ProviderConfigDef { - id: "google", - keys: &[key("GOOGLE_API_KEY", true, true)], - oauth_cache_path: None, - }, - ProviderConfigDef { - id: "openrouter", - keys: &[key("OPENROUTER_API_KEY", true, true)], - oauth_cache_path: None, - }, - ProviderConfigDef { - id: "xai", - keys: &[key("XAI_API_KEY", true, true)], - oauth_cache_path: None, - }, - ProviderConfigDef { - id: "nanogpt", - keys: &[key("NANOGPT_API_KEY", true, true)], - oauth_cache_path: None, - }, - ProviderConfigDef { - id: "venice", - keys: &[key("VENICE_API_KEY", true, true)], - oauth_cache_path: None, - }, - ProviderConfigDef { - id: "tetrate", - keys: &[key("TETRATE_API_KEY", true, true)], - oauth_cache_path: None, - }, - ProviderConfigDef { - id: "databricks", - keys: &[ - key("DATABRICKS_HOST", false, true), - key("DATABRICKS_TOKEN", true, false), - ], - oauth_cache_path: Some("databricks/oauth"), - }, - ProviderConfigDef { - id: "snowflake", - keys: &[ - key("SNOWFLAKE_HOST", false, true), - key("SNOWFLAKE_TOKEN", true, true), - ], - oauth_cache_path: None, - }, - ProviderConfigDef { - id: "litellm", - keys: &[ - key("LITELLM_HOST", false, true), - key("LITELLM_API_KEY", true, false), - ], - oauth_cache_path: None, - }, - ProviderConfigDef { - id: "azure", - keys: &[ - key("AZURE_OPENAI_ENDPOINT", false, true), - key("AZURE_OPENAI_DEPLOYMENT_NAME", false, true), - key("AZURE_OPENAI_API_KEY", true, false), - ], - oauth_cache_path: None, - }, - ProviderConfigDef { - id: "bedrock", - keys: &[key("AWS_REGION", false, false)], - oauth_cache_path: None, - }, - ProviderConfigDef { - id: "gcp_vertex_ai", - keys: &[ - key("GCP_PROJECT_ID", false, true), - key("GCP_LOCATION", false, true), - ], - oauth_cache_path: None, - }, - ProviderConfigDef { - id: "chatgpt_codex", - keys: &[key("CHATGPT_CODEX_TOKEN", true, true)], - oauth_cache_path: None, - }, - ProviderConfigDef { - id: "github_copilot", - keys: &[], - oauth_cache_path: Some("githubcopilot/info.json"), - }, - ProviderConfigDef { - id: "ollama", - keys: &[], - oauth_cache_path: None, - }, - ProviderConfigDef { - id: "local_inference", - keys: &[], - oauth_cache_path: None, - }, - // Dictation providers (voice input) - ProviderConfigDef { - id: "dictation_groq", - keys: &[key("GROQ_API_KEY", true, true)], - oauth_cache_path: None, - }, - ProviderConfigDef { - id: "dictation_elevenlabs", - keys: &[key("ELEVENLABS_API_KEY", true, true)], - oauth_cache_path: None, - }, -]; - -pub(crate) fn find_config_key(key_name: &str) -> Option<&'static ConfigKey> { - PROVIDER_CONFIG_DEFS - .iter() - .flat_map(|def| def.keys.iter()) - .find(|key| key.name == key_name) -} - -pub(crate) fn find_provider_def(provider_id: &str) -> Option<&'static ProviderConfigDef> { - PROVIDER_CONFIG_DEFS.iter().find(|d| d.id == provider_id) -} diff --git a/ui/goose2/src/app/hooks/useAppStartup.ts b/ui/goose2/src/app/hooks/useAppStartup.ts index 00a43542..ed4dd5fb 100644 --- a/ui/goose2/src/app/hooks/useAppStartup.ts +++ b/ui/goose2/src/app/hooks/useAppStartup.ts @@ -7,12 +7,6 @@ import { setNotificationHandler, getClient } from "@/shared/api/acpConnection"; import notificationHandler from "@/shared/api/acpNotificationHandler"; import { perfLog } from "@/shared/lib/perfLog"; -const INVENTORY_POLL_DELAYS_MS = [250, 500, 750, 1000, 1500, 2000]; - -function sleep(ms: number): Promise { - return new Promise((resolve) => window.setTimeout(resolve, ms)); -} - export function useAppStartup() { useEffect(() => { (async () => { @@ -101,27 +95,12 @@ export function useAppStartup() { return; } - const { getProviderInventory, refreshProviderInventory } = - await import("@/features/providers/api/inventory"); - const refresh = await refreshProviderInventory(configuredProviderIds); - if (refresh.started.length === 0) { - return; - } - - inventoryStore.mergeEntries( - await getProviderInventory(refresh.started), + const { syncProviderInventory } = await import( + "@/features/providers/api/inventorySync" ); - - for (const delayMs of INVENTORY_POLL_DELAYS_MS) { - await sleep(delayMs); - const refreshedEntries = await getProviderInventory( - refresh.started, - ); - inventoryStore.mergeEntries(refreshedEntries); - if (refreshedEntries.every((entry) => !entry.refreshing)) { - return; - } - } + await syncProviderInventory(configuredProviderIds, { + onEntries: (entries) => inventoryStore.mergeEntries(entries), + }); } catch (err) { console.error( "Failed to refresh provider inventory on startup:", diff --git a/ui/goose2/src/features/providers/api/credentials.test.ts b/ui/goose2/src/features/providers/api/credentials.test.ts new file mode 100644 index 00000000..8de5e332 --- /dev/null +++ b/ui/goose2/src/features/providers/api/credentials.test.ts @@ -0,0 +1,126 @@ +import { beforeEach, describe, expect, it, vi } from "vitest"; +import { + checkAllProviderStatus, + deleteProviderConfig, + getProviderConfig, + saveProviderConfig, +} from "./credentials"; + +const mocks = vi.hoisted(() => ({ + configRead: vi.fn(), + configSave: vi.fn(), + configDelete: vi.fn(), + configStatus: vi.fn(), + getClient: vi.fn(), +})); + +vi.mock("@/shared/api/acpConnection", () => ({ + getClient: () => mocks.getClient(), +})); + +describe("provider credential API", () => { + beforeEach(() => { + vi.clearAllMocks(); + mocks.getClient.mockResolvedValue({ + goose: { + GooseProvidersConfigRead: mocks.configRead, + GooseProvidersConfigSave: mocks.configSave, + GooseProvidersConfigDelete: mocks.configDelete, + GooseProvidersConfigStatus: mocks.configStatus, + }, + }); + }); + + it("reads provider config fields through the ACP provider config endpoint", async () => { + const fields = [ + { + key: "ANTHROPIC_API_KEY", + value: "sk-ant-********", + isSet: true, + isSecret: true, + required: true, + }, + ]; + mocks.configRead.mockResolvedValue({ fields }); + + await expect(getProviderConfig("anthropic")).resolves.toEqual(fields); + + expect(mocks.configRead).toHaveBeenCalledWith({ + providerId: "anthropic", + }); + }); + + it("saves provider config fields as one batch through ACP", async () => { + const fields = [ + { + key: "ANTHROPIC_API_KEY", + value: "sk-ant-test", + }, + { + key: "ANTHROPIC_HOST", + value: "https://api.anthropic.com", + }, + ]; + const response = { + status: { + providerId: "anthropic", + isConfigured: true, + }, + refresh: { + started: ["anthropic"], + skipped: [], + }, + }; + mocks.configSave.mockResolvedValue(response); + + await expect(saveProviderConfig("anthropic", fields)).resolves.toEqual( + response, + ); + + expect(mocks.configSave).toHaveBeenCalledWith({ + providerId: "anthropic", + fields, + }); + }); + + it("deletes provider config through ACP", async () => { + const response = { + status: { + providerId: "anthropic", + isConfigured: false, + }, + refresh: { + started: [], + skipped: [ + { + providerId: "anthropic", + reason: "not_configured", + }, + ], + }, + }; + mocks.configDelete.mockResolvedValue(response); + + await expect(deleteProviderConfig("anthropic")).resolves.toEqual(response); + + expect(mocks.configDelete).toHaveBeenCalledWith({ + providerId: "anthropic", + }); + }); + + it("checks provider status through ACP", async () => { + const statuses = [ + { + providerId: "anthropic", + isConfigured: true, + }, + ]; + mocks.configStatus.mockResolvedValue({ statuses }); + + await expect(checkAllProviderStatus()).resolves.toEqual(statuses); + + expect(mocks.configStatus).toHaveBeenCalledWith({ + providerIds: [], + }); + }); +}); diff --git a/ui/goose2/src/features/providers/api/credentials.ts b/ui/goose2/src/features/providers/api/credentials.ts index 2497fcf6..0d0024c1 100644 --- a/ui/goose2/src/features/providers/api/credentials.ts +++ b/ui/goose2/src/features/providers/api/credentials.ts @@ -1,32 +1,44 @@ -import { invoke } from "@tauri-apps/api/core"; +import type { + ProviderConfigChangeResponse, + ProviderConfigFieldUpdate, + ProviderConfigStatusDto, +} from "@aaif/goose-sdk"; import type { ProviderFieldValue } from "@/shared/types/providers"; +import { getClient } from "@/shared/api/acpConnection"; -export interface ProviderStatus { - providerId: string; - isConfigured: boolean; -} +export type ProviderStatus = ProviderConfigStatusDto; +export type ProviderFieldSaveInput = ProviderConfigFieldUpdate; export async function getProviderConfig( providerId: string, ): Promise { - return invoke("get_provider_config", { providerId }); + const client = await getClient(); + const response = await client.goose.GooseProvidersConfigRead({ providerId }); + return response.fields.map((field) => ({ + ...field, + value: field.value ?? null, + })); } -export async function saveProviderField( - key: string, - value: string, -): Promise { - return invoke("save_provider_field", { key, value }); +export async function saveProviderConfig( + providerId: string, + fields: ProviderFieldSaveInput[], +): Promise { + const client = await getClient(); + return client.goose.GooseProvidersConfigSave({ providerId, fields }); } -export async function deleteProviderConfig(providerId: string): Promise { - return invoke("delete_provider_config", { providerId }); +export async function deleteProviderConfig( + providerId: string, +): Promise { + const client = await getClient(); + return client.goose.GooseProvidersConfigDelete({ providerId }); } export async function checkAllProviderStatus(): Promise { - return invoke("check_all_provider_status"); -} - -export async function restartApp(): Promise { - return invoke("restart_app"); + const client = await getClient(); + const response = await client.goose.GooseProvidersConfigStatus({ + providerIds: [], + }); + return response.statuses; } diff --git a/ui/goose2/src/features/providers/api/inventorySync.test.ts b/ui/goose2/src/features/providers/api/inventorySync.test.ts new file mode 100644 index 00000000..67b955f2 --- /dev/null +++ b/ui/goose2/src/features/providers/api/inventorySync.test.ts @@ -0,0 +1,143 @@ +import type { ProviderInventoryEntryDto } from "@aaif/goose-sdk"; +import { beforeEach, describe, expect, it, vi } from "vitest"; +import { + INVENTORY_POLL_DELAYS_MS, + syncProviderInventory, +} from "./inventorySync"; + +function inventoryEntry( + providerId: string, + refreshing: boolean, + lastRefreshError: string | null = null, +): ProviderInventoryEntryDto { + return { + providerId, + providerName: providerId, + description: "", + defaultModel: "default-model", + configured: true, + providerType: "remote", + configKeys: [], + setupSteps: [], + supportsRefresh: true, + refreshing, + models: [], + lastUpdatedAt: null, + lastRefreshAttemptAt: null, + lastRefreshError, + stale: false, + modelSelectionHint: null, + }; +} + +describe("syncProviderInventory", () => { + const getInventory = vi.fn(); + const refreshInventory = vi.fn(); + const onEntries = vi.fn(); + const sleep = vi.fn(); + + beforeEach(() => { + vi.clearAllMocks(); + sleep.mockResolvedValue(undefined); + }); + + it("refreshes and polls with the startup delay schedule until entries settle", async () => { + refreshInventory.mockResolvedValue({ + started: ["anthropic"], + skipped: [], + }); + getInventory + .mockResolvedValueOnce([inventoryEntry("anthropic", true)]) + .mockResolvedValueOnce([inventoryEntry("anthropic", false)]); + + const result = await syncProviderInventory(["anthropic"], { + getInventory, + refreshInventory, + onEntries, + sleep, + }); + + expect(refreshInventory).toHaveBeenCalledWith(["anthropic"]); + expect(getInventory).toHaveBeenNthCalledWith(1, ["anthropic"]); + expect(getInventory).toHaveBeenNthCalledWith(2, ["anthropic"]); + expect(sleep).toHaveBeenCalledTimes(1); + expect(sleep).toHaveBeenCalledWith(INVENTORY_POLL_DELAYS_MS[0]); + expect(onEntries).toHaveBeenCalledTimes(2); + expect(result.settled).toBe(true); + expect(result.entries).toEqual([inventoryEntry("anthropic", false)]); + }); + + it("uses an initial refresh acknowledgement without starting another refresh", async () => { + const initialRefresh = { + started: ["anthropic"], + skipped: [], + }; + getInventory.mockResolvedValueOnce([inventoryEntry("anthropic", false)]); + + const result = await syncProviderInventory(["anthropic"], { + getInventory, + refreshInventory, + initialRefresh, + sleep, + }); + + expect(refreshInventory).not.toHaveBeenCalled(); + expect(getInventory).toHaveBeenCalledWith(["anthropic"]); + expect(result.refresh).toBe(initialRefresh); + expect(result.settled).toBe(true); + }); + + it("polls providers skipped because they were already refreshing", async () => { + refreshInventory.mockResolvedValue({ + started: [], + skipped: [ + { + providerId: "anthropic", + reason: "already_refreshing", + }, + ], + }); + getInventory + .mockResolvedValueOnce([inventoryEntry("anthropic", true)]) + .mockResolvedValueOnce([inventoryEntry("anthropic", false)]); + + const result = await syncProviderInventory(["anthropic"], { + getInventory, + refreshInventory, + sleep, + }); + + expect(result.polledProviderIds).toEqual(["anthropic"]); + expect(getInventory).toHaveBeenNthCalledWith(2, ["anthropic"]); + expect(result.settled).toBe(true); + }); + + it("merges skipped not-configured entries without forcing a warning state", async () => { + refreshInventory.mockResolvedValue({ + started: [], + skipped: [ + { + providerId: "anthropic", + reason: "not_configured", + }, + ], + }); + getInventory.mockResolvedValueOnce([ + { + ...inventoryEntry("anthropic", false), + configured: false, + }, + ]); + + const result = await syncProviderInventory(["anthropic"], { + getInventory, + refreshInventory, + sleep, + }); + + expect(result.settled).toBe(true); + expect(result.entries[0]?.configured).toBe(false); + expect(result.entries[0]?.lastRefreshError).toBeNull(); + expect(sleep).not.toHaveBeenCalled(); + }); +}); diff --git a/ui/goose2/src/features/providers/api/inventorySync.ts b/ui/goose2/src/features/providers/api/inventorySync.ts new file mode 100644 index 00000000..10e494f5 --- /dev/null +++ b/ui/goose2/src/features/providers/api/inventorySync.ts @@ -0,0 +1,115 @@ +import type { + ProviderInventoryEntryDto, + RefreshProviderInventoryResponse, +} from "@aaif/goose-sdk"; +import { getProviderInventory, refreshProviderInventory } from "./inventory"; + +export const INVENTORY_POLL_DELAYS_MS = [250, 500, 750, 1000, 1500, 2000]; + +type GetProviderInventory = typeof getProviderInventory; +type RefreshProviderInventory = typeof refreshProviderInventory; + +interface SyncProviderInventoryOptions { + getInventory?: GetProviderInventory; + refreshInventory?: RefreshProviderInventory; + initialRefresh?: RefreshProviderInventoryResponse; + onEntries?: (entries: ProviderInventoryEntryDto[]) => void; + sleep?: (ms: number) => Promise; +} + +export interface SyncProviderInventoryResult { + entries: ProviderInventoryEntryDto[]; + refresh: RefreshProviderInventoryResponse; + settled: boolean; + polledProviderIds: string[]; +} + +function defaultSleep(ms: number): Promise { + return new Promise((resolve) => window.setTimeout(resolve, ms)); +} + +function unique(values: string[]): string[] { + return [...new Set(values.filter(Boolean))]; +} + +function mergeEntries( + current: Map, + entries: ProviderInventoryEntryDto[], +) { + for (const entry of entries) { + current.set(entry.providerId, entry); + } +} + +function skippedProviderIds(refresh: RefreshProviderInventoryResponse) { + return (refresh.skipped ?? []).map((skip) => skip.providerId); +} + +function alreadyRefreshingProviderIds( + refresh: RefreshProviderInventoryResponse, +) { + return (refresh.skipped ?? []) + .filter((skip) => skip.reason === "already_refreshing") + .map((skip) => skip.providerId); +} + +export async function syncProviderInventory( + providerIds: string[], + { + getInventory = getProviderInventory, + refreshInventory = refreshProviderInventory, + initialRefresh, + onEntries, + sleep = defaultSleep, + }: SyncProviderInventoryOptions = {}, +): Promise { + const refresh = initialRefresh ?? (await refreshInventory(providerIds)); + const entriesByProviderId = new Map(); + const immediateProviderIds = unique([ + ...providerIds, + ...refresh.started, + ...skippedProviderIds(refresh), + ]); + const immediateEntries = await getInventory(immediateProviderIds); + mergeEntries(entriesByProviderId, immediateEntries); + onEntries?.(immediateEntries); + + const polledProviderIds = unique([ + ...providerIds, + ...refresh.started, + ...alreadyRefreshingProviderIds(refresh), + ]); + if ( + polledProviderIds.length === 0 || + immediateEntries.every((entry) => !entry.refreshing) + ) { + return { + entries: [...entriesByProviderId.values()], + refresh, + settled: true, + polledProviderIds, + }; + } + + for (const delayMs of INVENTORY_POLL_DELAYS_MS) { + await sleep(delayMs); + const entries = await getInventory(polledProviderIds); + mergeEntries(entriesByProviderId, entries); + onEntries?.(entries); + if (entries.every((entry) => !entry.refreshing)) { + return { + entries: [...entriesByProviderId.values()], + refresh, + settled: true, + polledProviderIds, + }; + } + } + + return { + entries: [...entriesByProviderId.values()], + refresh, + settled: false, + polledProviderIds, + }; +} diff --git a/ui/goose2/src/features/providers/hooks/useCredentials.test.tsx b/ui/goose2/src/features/providers/hooks/useCredentials.test.tsx new file mode 100644 index 00000000..b297ba6c --- /dev/null +++ b/ui/goose2/src/features/providers/hooks/useCredentials.test.tsx @@ -0,0 +1,248 @@ +import { act, renderHook, waitFor } from "@testing-library/react"; +import { beforeEach, describe, expect, it, vi } from "vitest"; +import { useCredentials } from "./useCredentials"; + +const mocks = vi.hoisted(() => ({ + checkAllProviderStatus: vi.fn(), + deleteProviderConfig: vi.fn(), + getProviderConfig: vi.fn(), + refreshProviderInventory: vi.fn(), + saveProviderConfig: vi.fn(), + syncProviderInventory: vi.fn(), +})); + +vi.mock("@/features/providers/api/credentials", () => ({ + checkAllProviderStatus: mocks.checkAllProviderStatus, + deleteProviderConfig: mocks.deleteProviderConfig, + getProviderConfig: mocks.getProviderConfig, + saveProviderConfig: mocks.saveProviderConfig, +})); + +vi.mock("@/features/providers/api/inventorySync", () => ({ + syncProviderInventory: mocks.syncProviderInventory, +})); + +vi.mock("@/features/providers/api/inventory", () => ({ + refreshProviderInventory: mocks.refreshProviderInventory, +})); + +describe("useCredentials", () => { + const saveResponse = { + status: { + providerId: "anthropic", + isConfigured: true, + }, + refresh: { + started: ["anthropic"], + skipped: [], + }, + }; + const deleteResponse = { + status: { + providerId: "anthropic", + isConfigured: false, + }, + refresh: { + started: [], + skipped: [ + { + providerId: "anthropic", + reason: "not_configured", + }, + ], + }, + }; + + beforeEach(() => { + vi.clearAllMocks(); + mocks.checkAllProviderStatus.mockResolvedValue([ + { + providerId: "anthropic", + isConfigured: true, + }, + ]); + mocks.saveProviderConfig.mockResolvedValue(saveResponse); + mocks.deleteProviderConfig.mockResolvedValue(deleteResponse); + mocks.refreshProviderInventory.mockResolvedValue({ + started: ["anthropic"], + skipped: [], + }); + mocks.syncProviderInventory.mockResolvedValue({ + entries: [], + refresh: { + started: ["anthropic"], + skipped: [], + }, + settled: true, + polledProviderIds: ["anthropic"], + }); + }); + + it("saves secret fields through the credential API and syncs inventory without requiring restart", async () => { + const { result } = renderHook(() => useCredentials()); + await waitFor(() => expect(result.current.loading).toBe(false)); + + await act(async () => { + await result.current.save("anthropic", [ + { + key: "ANTHROPIC_API_KEY", + value: "sk-ant-test", + isSecret: true, + }, + ]); + }); + + const fields = [ + { + key: "ANTHROPIC_API_KEY", + value: "sk-ant-test", + }, + ]; + + expect(mocks.saveProviderConfig).toHaveBeenCalledWith("anthropic", fields); + await waitFor(() => + expect(mocks.syncProviderInventory.mock.calls[0]?.[0]).toEqual([ + "anthropic", + ]), + ); + expect(mocks.syncProviderInventory.mock.calls[0]?.[1]).toEqual( + expect.objectContaining({ + initialRefresh: saveResponse.refresh, + }), + ); + expect(result.current).not.toHaveProperty("needsRestart"); + expect(result.current).not.toHaveProperty("restart"); + }); + + it("records refresh failure as a provider warning without rejecting the save", async () => { + mocks.syncProviderInventory.mockRejectedValueOnce( + new Error("model list failed"), + ); + const { result } = renderHook(() => useCredentials()); + await waitFor(() => expect(result.current.loading).toBe(false)); + + await act(async () => { + await result.current.save("anthropic", [ + { + key: "ANTHROPIC_API_KEY", + value: "sk-ant-test", + isSecret: true, + }, + ]); + }); + + expect(mocks.saveProviderConfig).toHaveBeenCalled(); + await waitFor(() => + expect(result.current.inventoryWarnings.get("anthropic")).toContain( + "model list failed", + ), + ); + }); + + it("suppresses stale refresh errors after deleting provider config", async () => { + mocks.syncProviderInventory.mockResolvedValueOnce({ + entries: [ + { + providerId: "anthropic", + lastRefreshError: "old refresh failure", + refreshing: false, + }, + ], + refresh: deleteResponse.refresh, + settled: true, + polledProviderIds: ["anthropic"], + }); + const { result } = renderHook(() => useCredentials()); + await waitFor(() => expect(result.current.loading).toBe(false)); + + await act(async () => { + await result.current.remove("anthropic"); + }); + + await waitFor(() => + expect(result.current.syncingProviderIds.has("anthropic")).toBe(false), + ); + expect(result.current.inventoryWarnings.has("anthropic")).toBe(false); + }); + + it("invalidates native OAuth secrets before refreshing provider status", async () => { + const refreshResponse = { + started: ["chatgpt_codex"], + skipped: [], + }; + mocks.checkAllProviderStatus + .mockResolvedValueOnce([ + { + providerId: "chatgpt_codex", + isConfigured: false, + }, + ]) + .mockResolvedValueOnce([ + { + providerId: "chatgpt_codex", + isConfigured: true, + }, + ]); + mocks.refreshProviderInventory.mockResolvedValueOnce(refreshResponse); + + const { result } = renderHook(() => useCredentials()); + await waitFor(() => expect(result.current.loading).toBe(false)); + expect(result.current.configuredIds.has("chatgpt_codex")).toBe(false); + + await act(async () => { + await result.current.completeNativeSetup("chatgpt_codex"); + }); + + expect(mocks.refreshProviderInventory).toHaveBeenCalledWith([ + "chatgpt_codex", + ]); + expect( + mocks.refreshProviderInventory.mock.invocationCallOrder[0], + ).toBeLessThan(mocks.checkAllProviderStatus.mock.invocationCallOrder[1]); + expect(mocks.syncProviderInventory).toHaveBeenCalledWith( + ["chatgpt_codex"], + expect.objectContaining({ + initialRefresh: refreshResponse, + }), + ); + expect(result.current.configuredIds.has("chatgpt_codex")).toBe(true); + }); + + it("refreshes native OAuth status when initial inventory refresh fails", async () => { + mocks.checkAllProviderStatus + .mockResolvedValueOnce([ + { + providerId: "chatgpt_codex", + isConfigured: false, + }, + ]) + .mockResolvedValueOnce([ + { + providerId: "chatgpt_codex", + isConfigured: true, + }, + ]); + mocks.refreshProviderInventory.mockRejectedValueOnce( + new Error("refresh unavailable"), + ); + + const { result } = renderHook(() => useCredentials()); + await waitFor(() => expect(result.current.loading).toBe(false)); + expect(result.current.configuredIds.has("chatgpt_codex")).toBe(false); + + await act(async () => { + await result.current.completeNativeSetup("chatgpt_codex"); + }); + + expect(mocks.refreshProviderInventory).toHaveBeenCalledWith([ + "chatgpt_codex", + ]); + expect(result.current.configuredIds.has("chatgpt_codex")).toBe(true); + expect(mocks.syncProviderInventory).toHaveBeenCalledWith( + ["chatgpt_codex"], + expect.objectContaining({ + initialRefresh: undefined, + }), + ); + }); +}); diff --git a/ui/goose2/src/features/providers/hooks/useCredentials.ts b/ui/goose2/src/features/providers/hooks/useCredentials.ts index 8b7c73b8..0f0a2f40 100644 --- a/ui/goose2/src/features/providers/hooks/useCredentials.ts +++ b/ui/goose2/src/features/providers/hooks/useCredentials.ts @@ -1,31 +1,81 @@ -import { useState, useEffect, useCallback, useMemo } from "react"; +import { useState, useEffect, useCallback, useMemo, useRef } from "react"; import { getProviderConfig, - saveProviderField, + saveProviderConfig, deleteProviderConfig, type ProviderStatus, checkAllProviderStatus, - restartApp, } from "@/features/providers/api/credentials"; +import { + syncProviderInventory, + type SyncProviderInventoryResult, +} from "@/features/providers/api/inventorySync"; +import { refreshProviderInventory } from "@/features/providers/api/inventory"; +import { useProviderInventoryStore } from "@/features/providers/stores/providerInventoryStore"; import type { ProviderFieldValue } from "@/shared/types/providers"; +export interface ProviderFieldSave { + key: string; + value: string; + isSecret: boolean; +} + interface UseCredentialsReturn { configuredIds: Set; loading: boolean; saving: boolean; - needsRestart: boolean; + savingProviderIds: Set; + syncingProviderIds: Set; + inventoryWarnings: Map; getConfig: (providerId: string) => Promise; - save: (key: string, value: string) => Promise; + save: (providerId: string, fields: ProviderFieldSave[]) => Promise; remove: (providerId: string) => Promise; - restart: () => Promise; - completeNativeSetup: () => Promise; + completeNativeSetup: (providerId: string) => Promise; +} + +function errorMessage(error: unknown): string { + return error instanceof Error ? error.message : String(error); +} + +function inventoryWarning( + providerId: string, + result: SyncProviderInventoryResult, +): string | null { + const entry = result.entries.find((item) => item.providerId === providerId); + const skipped = result.refresh.skipped?.find( + (item) => item.providerId === providerId, + ); + if (skipped?.reason === "not_configured") { + return null; + } + if (skipped?.reason === "unknown_provider") { + return "Provider inventory is unavailable."; + } + + if (entry?.lastRefreshError) { + return entry.lastRefreshError; + } + + if (!result.settled && entry?.refreshing) { + return "Model inventory is still refreshing."; + } + + return null; } export function useCredentials(): UseCredentialsReturn { const [statuses, setStatuses] = useState([]); const [loading, setLoading] = useState(true); - const [saving, setSaving] = useState(false); - const [needsRestart, setNeedsRestart] = useState(false); + const [savingProviderIds, setSavingProviderIds] = useState>( + () => new Set(), + ); + const [syncingProviderIds, setSyncingProviderIds] = useState>( + () => new Set(), + ); + const [inventoryWarnings, setInventoryWarnings] = useState< + Map + >(() => new Map()); + const syncRunIds = useRef(new Map()); const refreshStatuses = useCallback(async () => { const nextStatuses = await checkAllProviderStatus(); @@ -33,6 +83,16 @@ export function useCredentials(): UseCredentialsReturn { return nextStatuses; }, []); + const updateProviderStatus = useCallback((status: ProviderStatus) => { + setStatuses((current) => { + const next = current.filter( + (item) => item.providerId !== status.providerId, + ); + next.push(status); + return next; + }); + }, []); + useEffect(() => { refreshStatuses() .catch(() => {}) @@ -44,57 +104,158 @@ export function useCredentials(): UseCredentialsReturn { new Set(statuses.filter((s) => s.isConfigured).map((s) => s.providerId)), [statuses], ); + const saving = savingProviderIds.size > 0; const getConfig = useCallback(async (providerId: string) => { return getProviderConfig(providerId); }, []); + const setProviderSaving = useCallback( + (providerId: string, isSaving: boolean) => { + setSavingProviderIds((current) => { + const next = new Set(current); + if (isSaving) { + next.add(providerId); + } else { + next.delete(providerId); + } + return next; + }); + }, + [], + ); + + const setProviderSyncing = useCallback( + (providerId: string, isSyncing: boolean) => { + setSyncingProviderIds((current) => { + const next = new Set(current); + if (isSyncing) { + next.add(providerId); + } else { + next.delete(providerId); + } + return next; + }); + }, + [], + ); + + const setProviderInventoryWarning = useCallback( + (providerId: string, warning: string | null) => { + setInventoryWarnings((current) => { + const next = new Map(current); + if (warning) { + next.set(providerId, warning); + } else { + next.delete(providerId); + } + return next; + }); + }, + [], + ); + + const startInventorySync = useCallback( + ( + providerId: string, + initialRefresh?: SyncProviderInventoryResult["refresh"], + ) => { + const runId = (syncRunIds.current.get(providerId) ?? 0) + 1; + syncRunIds.current.set(providerId, runId); + setProviderSyncing(providerId, true); + setProviderInventoryWarning(providerId, null); + + void syncProviderInventory([providerId], { + initialRefresh, + onEntries: (entries) => { + if (syncRunIds.current.get(providerId) !== runId) { + return; + } + useProviderInventoryStore.getState().mergeEntries(entries); + }, + }) + .then((result) => { + if (syncRunIds.current.get(providerId) !== runId) { + return; + } + setProviderInventoryWarning( + providerId, + inventoryWarning(providerId, result), + ); + }) + .catch((error) => { + if (syncRunIds.current.get(providerId) !== runId) { + return; + } + setProviderInventoryWarning(providerId, errorMessage(error)); + }) + .finally(() => { + if (syncRunIds.current.get(providerId) !== runId) { + return; + } + setProviderSyncing(providerId, false); + }); + }, + [setProviderInventoryWarning, setProviderSyncing], + ); + const save = useCallback( - async (key: string, value: string) => { - setSaving(true); + async (providerId: string, fields: ProviderFieldSave[]) => { + setProviderSaving(providerId, true); try { - await saveProviderField(key, value); - await refreshStatuses(); - setNeedsRestart(true); + const result = await saveProviderConfig( + providerId, + fields.map(({ key, value }) => ({ key, value })), + ); + updateProviderStatus(result.status); + startInventorySync(providerId, result.refresh); } finally { - setSaving(false); + setProviderSaving(providerId, false); } }, - [refreshStatuses], + [setProviderSaving, startInventorySync, updateProviderStatus], ); const remove = useCallback( async (providerId: string) => { - setSaving(true); + setProviderSaving(providerId, true); try { - await deleteProviderConfig(providerId); - await refreshStatuses(); - setNeedsRestart(true); + const result = await deleteProviderConfig(providerId); + updateProviderStatus(result.status); + startInventorySync(providerId, result.refresh); } finally { - setSaving(false); + setProviderSaving(providerId, false); } }, - [refreshStatuses], + [setProviderSaving, startInventorySync, updateProviderStatus], ); - const restart = useCallback(async () => { - await restartApp(); - }, []); - - const completeNativeSetup = useCallback(async () => { - await refreshStatuses(); - setNeedsRestart(true); - }, [refreshStatuses]); + const completeNativeSetup = useCallback( + async (providerId: string) => { + // Native OAuth returns only after the subprocess writes credentials. + // Inventory refresh invalidates ACP's secret cache before status reads it. + let initialRefresh: SyncProviderInventoryResult["refresh"] | undefined; + try { + initialRefresh = await refreshProviderInventory([providerId]); + } catch (error) { + setProviderInventoryWarning(providerId, errorMessage(error)); + } + await refreshStatuses(); + startInventorySync(providerId, initialRefresh); + }, + [refreshStatuses, setProviderInventoryWarning, startInventorySync], + ); return { configuredIds, loading, saving, - needsRestart, + savingProviderIds, + syncingProviderIds, + inventoryWarnings, getConfig, save, remove, - restart, completeNativeSetup, }; } diff --git a/ui/goose2/src/features/settings/ui/AgentProviderCard.tsx b/ui/goose2/src/features/settings/ui/AgentProviderCard.tsx index ac206185..adf8dc3a 100644 --- a/ui/goose2/src/features/settings/ui/AgentProviderCard.tsx +++ b/ui/goose2/src/features/settings/ui/AgentProviderCard.tsx @@ -24,6 +24,7 @@ interface OutputLine { } const MAX_OUTPUT_LINES = 50; +const CHECKING_INDICATOR_DELAY_MS = 2000; interface AgentProviderCardProps { provider: ProviderDisplayInfo; @@ -38,6 +39,7 @@ export function AgentProviderCard({ provider }: AgentProviderCardProps) { const [setupPhase, setSetupPhase] = useState("idle"); const [setupOutput, setSetupOutput] = useState([]); const [setupError, setSetupError] = useState(null); + const [showCheckingIndicator, setShowCheckingIndicator] = useState(false); const [installStatus, setInstallStatus] = useState( hasBinary && !isBuiltIn ? "checking" : "installed", ); @@ -228,9 +230,6 @@ export function AgentProviderCard({ provider }: AgentProviderCardProps) { void handleConnect(); } - if (provider.showOnlyWhenInstalled && installStatus !== "installed") - return null; - const isReady = isBuiltIn || (installStatus === "installed" && !hasAuthCommand) || @@ -241,12 +240,34 @@ export function AgentProviderCard({ provider }: AgentProviderCardProps) { authStatus !== "checking" && authStatus !== "authenticated"; const needsInstall = installStatus === "missing" && hasInstallCommand; + const isChecking = + (installStatus === "checking" && hasBinary) || + (installStatus === "installed" && authStatus === "checking"); + + useEffect(() => { + if (!isChecking) { + setShowCheckingIndicator(false); + return; + } + + setShowCheckingIndicator(false); + const timeoutId = window.setTimeout(() => { + if (isMountedRef.current) { + setShowCheckingIndicator(true); + } + }, CHECKING_INDICATOR_DELAY_MS); + + return () => window.clearTimeout(timeoutId); + }, [isChecking]); + + if (provider.showOnlyWhenInstalled && installStatus !== "installed") + return null; function renderStatusIndicator() { if (isBuiltIn || isReady) { return (
- +
); } @@ -259,6 +280,26 @@ export function AgentProviderCard({ provider }: AgentProviderCardProps) { ); } + if ((isChecking && showCheckingIndicator) || isActive) { + return ( +
+
+ ); + } + if (needsAuth && !isActive) { return ( {renderSetupMessage(setupMessage)} + {error &&

{error}

} ); @@ -169,6 +214,8 @@ interface SetupFieldsPanelProps { fieldValueMap: Map; draftValues: Record; saving: boolean; + inventorySyncing: boolean; + inventoryWarning?: string | null; showSavedState: boolean; error: string; setupMethod: ProviderSetupMethod; @@ -185,6 +232,8 @@ export function SetupFieldsPanel({ fieldValueMap, draftValues, saving, + inventorySyncing, + inventoryWarning, showSavedState, error, setupMethod, @@ -257,6 +306,10 @@ export function SetupFieldsPanel({ {setupMethod === "cloud_credentials" && setupMessage ? renderSetupMessage(setupMessage) : null} + {error &&

{error}

} ); diff --git a/ui/goose2/src/features/settings/ui/ModelProviderRow.tsx b/ui/goose2/src/features/settings/ui/ModelProviderRow.tsx index 7a6751db..1982997f 100644 --- a/ui/goose2/src/features/settings/ui/ModelProviderRow.tsx +++ b/ui/goose2/src/features/settings/ui/ModelProviderRow.tsx @@ -34,24 +34,38 @@ import { getFieldSetupDescription, renderSetupMessage, } from "./modelProviderHelpers"; -import { ConnectedFieldsPanel, SetupFieldsPanel } from "./ModelProviderPanels"; +import { + ConnectedFieldsPanel, + InventorySyncMessage, + SetupFieldsPanel, +} from "./ModelProviderPanels"; + +interface ProviderFieldSaveInput { + key: string; + value: string; + isSecret: boolean; +} interface ModelProviderRowProps { provider: ProviderDisplayInfo; onGetConfig: (providerId: string) => Promise; - onSaveField: (key: string, value: string, isSecret: boolean) => Promise; + onSaveFields: (fields: ProviderFieldSaveInput[]) => Promise; onRemoveConfig?: () => Promise; - onCompleteNativeSetup: () => Promise; + onCompleteNativeSetup: (providerId: string) => Promise; saving?: boolean; + inventorySyncing?: boolean; + inventoryWarning?: string | null; } export function ModelProviderRow({ provider, onGetConfig, - onSaveField, + onSaveFields, onRemoveConfig, onCompleteNativeSetup, saving = false, + inventorySyncing = false, + inventoryWarning = null, }: ModelProviderRowProps) { const { t } = useTranslation("settings"); const [expanded, setExpanded] = useState(false); @@ -161,8 +175,10 @@ export function ModelProviderRow({ const unlisten = await onModelSetupOutput(provider.id, appendSetupOutput); try { + // The native connector exits after writing credentials; only then do we + // ask the credentials hook to refresh ACP inventory for this provider. await authenticateModelProvider(provider.id, provider.nativeConnectQuery); - await onCompleteNativeSetup(); + await onCompleteNativeSetup(provider.id); } catch (nextError) { setSetupError( nextError instanceof Error @@ -219,7 +235,9 @@ export function ModelProviderRow({ setError(""); try { shouldRestorePanelFocus.current = true; - await onSaveField(field.key, nextValue, field.secret); + await onSaveFields([ + { key: field.key, value: nextValue, isSecret: field.secret }, + ]); await loadConfig(); setEditingKey(null); setShowSavedState(true); @@ -269,10 +287,13 @@ export function ModelProviderRow({ setError(""); try { - for (const field of fieldsToSave) { - const nextValue = draftValues[field.key]?.trim() ?? ""; - await onSaveField(field.key, nextValue, field.secret); - } + await onSaveFields( + fieldsToSave.map((field) => ({ + key: field.key, + value: draftValues[field.key]?.trim() ?? "", + isSecret: field.secret, + })), + ); await loadConfig(); setShowSavedState(true); setPreserveSetupLayout(true); @@ -364,6 +385,10 @@ export function ModelProviderRow({ {t("providers.waitingForSignIn")} ) : null} + {setupOutput.length > 0 ? (
{setupOutput.map((line) => ( @@ -387,6 +412,8 @@ export function ModelProviderRow({ editingKey={editingKey} draftValues={draftValues} saving={saving} + inventorySyncing={inventorySyncing} + inventoryWarning={inventoryWarning} showSavedState={showSavedState} error={error} setupMessage={setupMessage} @@ -407,6 +434,8 @@ export function ModelProviderRow({ fieldValueMap={fieldValueMap} draftValues={draftValues} saving={saving} + inventorySyncing={inventorySyncing} + inventoryWarning={inventoryWarning} showSavedState={showSavedState} error={error} setupMethod={provider.setupMethod} @@ -426,6 +455,10 @@ export function ModelProviderRow({ className="focus-override mx-3 space-y-2 rounded-b-lg border-x border-b px-3 py-3 outline-none" > {renderSetupMessage(setupMessage)} +
); } @@ -452,6 +485,9 @@ export function ModelProviderRow({ {isConnected ? ( ) : null} + {inventorySyncing ? ( + + ) : null} {!isConnected && authenticating ? ( ) : null} diff --git a/ui/goose2/src/features/settings/ui/ProvidersSettings.tsx b/ui/goose2/src/features/settings/ui/ProvidersSettings.tsx index 38d34ca5..fcf0c8d7 100644 --- a/ui/goose2/src/features/settings/ui/ProvidersSettings.tsx +++ b/ui/goose2/src/features/settings/ui/ProvidersSettings.tsx @@ -2,8 +2,8 @@ import { useEffect, useMemo, useState } from "react"; import { useTranslation } from "react-i18next"; import { Button } from "@/shared/ui/button"; import { Separator } from "@/shared/ui/separator"; -import { Skeleton } from "@/shared/ui/skeleton"; -import { IconChevronDown, IconRefresh } from "@tabler/icons-react"; +import { Spinner } from "@/shared/ui/spinner"; +import { IconChevronDown } from "@tabler/icons-react"; import { getAgentProviders, getModelProviders, @@ -45,12 +45,12 @@ export function ProvidersSettings() { const { configuredIds, loading, - saving, - needsRestart, + savingProviderIds, + syncingProviderIds, + inventoryWarnings, getConfig, save, remove, - restart, completeNativeSetup, } = useCredentials(); @@ -112,23 +112,6 @@ export function ProvidersSettings() { const advancedModels = orderedModels.filter((m) => m.tier === "advanced"); const visibleModels = showAllModels ? orderedModels : promotedModels; - if (loading) { - return ( -
-
- - -
-
- - - - -
-
- ); - } - return (

@@ -138,16 +121,6 @@ export function ProvidersSettings() { {t("providers.description")}

- {needsRestart && ( -
-

{t("providers.restartMessage")}

- -
- )} -
@@ -171,9 +144,17 @@ export function ProvidersSettings() {
-

- {t("providers.models.title")} -

+
+

+ {t("providers.models.title")} +

+ {loading ? ( + + + {t("providers.models.checkingStatus")} + + ) : null} +

{t("providers.models.description")}

@@ -185,10 +166,12 @@ export function ProvidersSettings() { key={model.id} provider={model} onGetConfig={getConfig} - onSaveField={save} + onSaveFields={(fields) => save(model.id, fields)} onRemoveConfig={() => remove(model.id)} onCompleteNativeSetup={completeNativeSetup} - saving={saving} + saving={savingProviderIds.has(model.id)} + inventorySyncing={syncingProviderIds.has(model.id)} + inventoryWarning={inventoryWarnings.get(model.id)} /> ))}
diff --git a/ui/goose2/src/features/settings/ui/__tests__/AgentProviderCard.test.tsx b/ui/goose2/src/features/settings/ui/__tests__/AgentProviderCard.test.tsx index 60593465..a8750cd3 100644 --- a/ui/goose2/src/features/settings/ui/__tests__/AgentProviderCard.test.tsx +++ b/ui/goose2/src/features/settings/ui/__tests__/AgentProviderCard.test.tsx @@ -1,6 +1,6 @@ import { act, screen, waitFor } from "@testing-library/react"; import userEvent from "@testing-library/user-event"; -import { beforeEach, describe, expect, it, vi } from "vitest"; +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; import { renderWithProviders } from "@/test/render"; import { AgentProviderCard } from "../AgentProviderCard"; import type { ProviderDisplayInfo } from "@/shared/types/providers"; @@ -37,7 +37,12 @@ describe("AgentProviderCard", () => { vi.clearAllMocks(); }); - it("does not show sign in while auth status is checking", async () => { + afterEach(() => { + vi.useRealTimers(); + }); + + it("delays the checking indicator and does not show sign in while auth status is checking", async () => { + vi.useFakeTimers(); let resolveAuth!: (authenticated: boolean) => void; const authPromise = new Promise((resolve) => { resolveAuth = resolve; @@ -48,21 +53,53 @@ describe("AgentProviderCard", () => { renderWithProviders(); - expect(await screen.findByText("Checking...")).toBeInTheDocument(); + await act(async () => { + await Promise.resolve(); + await Promise.resolve(); + }); + expect(checkAgentAuth).toHaveBeenCalled(); + + expect(screen.queryByRole("status", { name: "Checking..." })).toBeNull(); + expect(screen.queryByText("Checking...")).not.toBeInTheDocument(); expect( screen.queryByRole("button", { name: /sign in/i }), ).not.toBeInTheDocument(); + act(() => { + vi.advanceTimersByTime(2000); + }); + + expect( + screen.getByRole("status", { name: "Checking..." }), + ).toBeInTheDocument(); + await act(async () => { resolveAuth(false); await authPromise; }); - await waitFor(() => { - expect( - screen.getByRole("button", { name: /sign in/i }), - ).toBeInTheDocument(); + expect( + screen.getByRole("button", { name: /sign in/i }), + ).toBeInTheDocument(); + }); + + it("does not show the checking indicator when auth resolves quickly", async () => { + vi.useFakeTimers(); + checkAgentInstalled.mockResolvedValue(true); + checkAgentAuth.mockResolvedValue(true); + + renderWithProviders(); + + await act(async () => { + await Promise.resolve(); + await Promise.resolve(); }); + + act(() => { + vi.advanceTimersByTime(2000); + }); + + expect(screen.queryByRole("status", { name: "Checking..." })).toBeNull(); }); it("checks installation by provider id after installing", async () => { diff --git a/ui/goose2/src/features/settings/ui/__tests__/ModelProviderRow.test.tsx b/ui/goose2/src/features/settings/ui/__tests__/ModelProviderRow.test.tsx new file mode 100644 index 00000000..bb6d4465 --- /dev/null +++ b/ui/goose2/src/features/settings/ui/__tests__/ModelProviderRow.test.tsx @@ -0,0 +1,113 @@ +import { render, screen, waitFor } from "@testing-library/react"; +import userEvent from "@testing-library/user-event"; +import type { ComponentType } from "react"; +import { beforeEach, describe, expect, it, vi } from "vitest"; +import { getModelProviders } from "@/features/providers/providerCatalog"; +import { ModelProviderRow } from "../ModelProviderRow"; + +const Row = ModelProviderRow as unknown as ComponentType< + Record +>; + +function modelProvider(id: string, status: "connected" | "not_configured") { + const provider = getModelProviders().find((entry) => entry.id === id); + if (!provider) { + throw new Error(`missing provider fixture: ${id}`); + } + return { + ...provider, + status, + }; +} + +describe("ModelProviderRow", () => { + const onGetConfig = vi.fn(); + const onSaveFields = vi.fn(); + const onRemoveConfig = vi.fn(); + const onCompleteNativeSetup = vi.fn(); + + beforeEach(() => { + vi.clearAllMocks(); + onGetConfig.mockResolvedValue([]); + onSaveFields.mockResolvedValue(undefined); + onRemoveConfig.mockResolvedValue(undefined); + onCompleteNativeSetup.mockResolvedValue(undefined); + }); + + it("saves all changed setup fields from one setup submit", async () => { + const user = userEvent.setup(); + + render( + , + ); + + await user.click(screen.getByRole("button", { name: /databricks/i })); + await user.type( + await screen.findByPlaceholderText(/cloud\.databricks\.com/i), + "https://dbc-test.cloud.databricks.com", + ); + await user.type( + screen.getByPlaceholderText(/paste your access token/i), + "databricks-token", + ); + await user.click(screen.getByRole("button", { name: /^save$/i })); + + await waitFor(() => expect(onSaveFields).toHaveBeenCalledTimes(1)); + expect(onSaveFields).toHaveBeenCalledWith([ + { + key: "DATABRICKS_HOST", + value: "https://dbc-test.cloud.databricks.com", + isSecret: false, + }, + { + key: "DATABRICKS_TOKEN", + value: "databricks-token", + isSecret: true, + }, + ]); + }); + + it("shows the connected row while model inventory is still loading", async () => { + const user = userEvent.setup(); + + render( + , + ); + + await user.click(screen.getByRole("button", { name: /anthropic/i })); + + expect(screen.getByText(/loading models/i)).toBeInTheDocument(); + }); + + it("shows a non-blocking inventory warning without replacing the connected state", async () => { + const user = userEvent.setup(); + + render( + , + ); + + await user.click(screen.getByRole("button", { name: /anthropic/i })); + + expect(screen.getByText(/model refresh failed/i)).toBeInTheDocument(); + }); +}); diff --git a/ui/goose2/src/features/settings/ui/__tests__/ProvidersSettings.test.tsx b/ui/goose2/src/features/settings/ui/__tests__/ProvidersSettings.test.tsx new file mode 100644 index 00000000..66407c25 --- /dev/null +++ b/ui/goose2/src/features/settings/ui/__tests__/ProvidersSettings.test.tsx @@ -0,0 +1,91 @@ +import { render, screen } from "@testing-library/react"; +import { beforeEach, describe, expect, it, vi } from "vitest"; +import { ProvidersSettings } from "../ProvidersSettings"; + +const mocks = vi.hoisted(() => ({ + useCredentials: vi.fn(), +})); + +vi.mock("@/features/providers/hooks/useCredentials", () => ({ + useCredentials: () => mocks.useCredentials(), +})); + +describe("ProvidersSettings", () => { + beforeEach(() => { + vi.clearAllMocks(); + mocks.useCredentials.mockReturnValue({ + configuredIds: new Set(), + loading: false, + saving: false, + savingProviderIds: new Set(), + syncingProviderIds: new Set(), + inventoryWarnings: new Map(), + getConfig: vi.fn(), + save: vi.fn(), + remove: vi.fn(), + completeNativeSetup: vi.fn(), + }); + }); + + it("does not show the restart banner for provider credential changes", () => { + render(); + + expect( + screen.queryByText(/restart to apply credential changes/i), + ).not.toBeInTheDocument(); + expect( + screen.queryByRole("button", { name: /restart now/i }), + ).not.toBeInTheDocument(); + }); + + it("renders the static provider catalog while credential status is loading", () => { + mocks.useCredentials.mockReturnValue({ + configuredIds: new Set(), + loading: true, + saving: false, + savingProviderIds: new Set(), + syncingProviderIds: new Set(), + inventoryWarnings: new Map(), + getConfig: vi.fn(), + save: vi.fn(), + remove: vi.fn(), + completeNativeSetup: vi.fn(), + }); + + render(); + + expect(screen.getByText("Providers")).toBeInTheDocument(); + expect(screen.getByText("Anthropic")).toBeInTheDocument(); + expect(screen.getByText("Checking provider status...")).toBeInTheDocument(); + }); + + it("matches main by ordering connected model providers first after status loads", () => { + mocks.useCredentials.mockReturnValue({ + configuredIds: new Set(["openai", "databricks"]), + loading: false, + saving: false, + savingProviderIds: new Set(), + syncingProviderIds: new Set(), + inventoryWarnings: new Map(), + getConfig: vi.fn(), + save: vi.fn(), + remove: vi.fn(), + completeNativeSetup: vi.fn(), + }); + + render(); + + const openai = screen.getByText("OpenAI"); + const databricks = screen.getByText("Databricks"); + const anthropic = screen.getByText("Anthropic"); + + expect( + openai.compareDocumentPosition(databricks) & + Node.DOCUMENT_POSITION_FOLLOWING, + ).toBeTruthy(); + expect( + databricks.compareDocumentPosition(anthropic) & + Node.DOCUMENT_POSITION_FOLLOWING, + ).toBeTruthy(); + }); +}); diff --git a/ui/goose2/src/shared/api/__tests__/dictation.test.ts b/ui/goose2/src/shared/api/__tests__/dictation.test.ts index ad6fb764..340edad8 100644 --- a/ui/goose2/src/shared/api/__tests__/dictation.test.ts +++ b/ui/goose2/src/shared/api/__tests__/dictation.test.ts @@ -2,11 +2,13 @@ import { describe, it, expect, vi, beforeEach } from "vitest"; import { cancelDictationLocalModelDownload, deleteDictationLocalModel, + deleteDictationProviderSecret, downloadDictationLocalModel, getDictationConfig, getDictationLocalModelDownloadProgress, listDictationLocalModels, saveDictationModelSelection, + saveDictationProviderSecret, transcribeDictation, } from "../dictation"; import { getClient } from "../acpConnection"; @@ -67,6 +69,23 @@ describe("dictation SDK wiring", () => { }); }); + it("saveDictationProviderSecret calls GooseSecretUpsert", async () => { + client.goose.GooseSecretUpsert = vi.fn().mockResolvedValue({}); + await saveDictationProviderSecret("groq", "gsk-test", "GROQ_API_KEY"); + expect(client.goose.GooseSecretUpsert).toHaveBeenCalledWith({ + key: "GROQ_API_KEY", + value: "gsk-test", + }); + }); + + it("deleteDictationProviderSecret calls GooseSecretRemove", async () => { + client.goose.GooseSecretRemove = vi.fn().mockResolvedValue({}); + await deleteDictationProviderSecret("groq", "GROQ_API_KEY"); + expect(client.goose.GooseSecretRemove).toHaveBeenCalledWith({ + key: "GROQ_API_KEY", + }); + }); + it("listDictationLocalModels returns the models array", async () => { client.goose.GooseDictationModelsList = vi.fn().mockResolvedValue({ models: [ diff --git a/ui/goose2/src/shared/api/dictation.ts b/ui/goose2/src/shared/api/dictation.ts index 4c3b42c6..b15628d0 100644 --- a/ui/goose2/src/shared/api/dictation.ts +++ b/ui/goose2/src/shared/api/dictation.ts @@ -1,4 +1,3 @@ -import { invoke } from "@tauri-apps/api/core"; import type { DictationDownloadProgress, DictationProvider, @@ -48,22 +47,19 @@ export async function saveDictationProviderSecret( if (!configKey) { throw new Error("No config key for this provider"); } - return invoke("save_provider_field", { key: configKey, value }); + const client = await getClient(); + await client.goose.GooseSecretUpsert({ key: configKey, value }); } export async function deleteDictationProviderSecret( - provider: DictationProvider, - _configKey?: string, + _provider: DictationProvider, + configKey?: string, ): Promise { - const providerIdMap: Record = { - groq: "dictation_groq", - elevenlabs: "dictation_elevenlabs", - }; - const providerId = providerIdMap[provider]; - if (!providerId) { + if (!configKey) { throw new Error("Cannot delete secrets for this provider"); } - return invoke("delete_provider_config", { providerId }); + const client = await getClient(); + await client.goose.GooseSecretRemove({ key: configKey }); } export async function listDictationLocalModels(): Promise< diff --git a/ui/goose2/src/shared/i18n/locales/en/settings.json b/ui/goose2/src/shared/i18n/locales/en/settings.json index 44d3e8e1..54573d43 100644 --- a/ui/goose2/src/shared/i18n/locales/en/settings.json +++ b/ui/goose2/src/shared/i18n/locales/en/settings.json @@ -220,9 +220,10 @@ "signInLabel": "Sign in to {{name}}", "status": { "checking": "Checking...", + "inProgress": "Setup in progress", "setupFailed": "Setup failed" }, - "title": "Agents" + "title": "Agent harnesses" }, "description": "Connect agents and AI models to use with Goose", "disconnect": "Disconnect", @@ -251,10 +252,11 @@ "oauthTerminal": "Run `goose configure` in your terminal to finish sign-in." } }, + "checkingStatus": "Checking provider status...", "title": "Models" }, - "restartButton": "Restart now", - "restartMessage": "Restart to apply credential changes.", + "loadingModels": "Loading models...", + "modelRefreshWarning": "Credentials saved. Model refresh needs attention: {{message}}", "saved": "Saved", "showFewer": "Show fewer", "showMore": "Show {{count}} more providers", diff --git a/ui/goose2/src/shared/i18n/locales/es/settings.json b/ui/goose2/src/shared/i18n/locales/es/settings.json index 747c0b3e..ebf86e89 100644 --- a/ui/goose2/src/shared/i18n/locales/es/settings.json +++ b/ui/goose2/src/shared/i18n/locales/es/settings.json @@ -220,9 +220,10 @@ "signInLabel": "Iniciar sesión en {{name}}", "status": { "checking": "Comprobando...", + "inProgress": "Configuración en curso", "setupFailed": "La configuración falló" }, - "title": "Agentes" + "title": "Arneses de agentes" }, "description": "Conecta agentes y modelos de IA para usar con Goose", "disconnect": "Desconectar", @@ -251,10 +252,11 @@ "oauthTerminal": "Ejecuta `goose configure` en tu terminal para terminar de iniciar sesión." } }, + "checkingStatus": "Comprobando estado del proveedor...", "title": "Modelos" }, - "restartButton": "Reiniciar ahora", - "restartMessage": "Reinicia para aplicar los cambios de credenciales.", + "loadingModels": "Cargando modelos...", + "modelRefreshWarning": "Credenciales guardadas. La actualización de modelos necesita atención: {{message}}", "saved": "Guardado", "showFewer": "Mostrar menos", "showMore": "Mostrar {{count}} proveedores más", diff --git a/ui/sdk/src/generated/client.gen.ts b/ui/sdk/src/generated/client.gen.ts index 45ba4c26..5cb1acd3 100644 --- a/ui/sdk/src/generated/client.gen.ts +++ b/ui/sdk/src/generated/client.gen.ts @@ -47,6 +47,13 @@ import type { ListProvidersResponse, ListSourcesRequest, ListSourcesResponse, + ProviderConfigChangeResponse, + ProviderConfigDeleteRequest, + ProviderConfigReadRequest, + ProviderConfigReadResponse, + ProviderConfigSaveRequest, + ProviderConfigStatusRequest, + ProviderConfigStatusResponse, ReadConfigRequest, ReadConfigResponse, ReadResourceRequest, @@ -83,6 +90,9 @@ import { zImportSourcesResponse, zListProvidersResponse, zListSourcesResponse, + zProviderConfigChangeResponse, + zProviderConfigReadResponse, + zProviderConfigStatusResponse, zReadConfigResponse, zReadResourceResponse, zRefreshProviderInventoryResponse, @@ -173,6 +183,52 @@ export class GooseExtClient { ) as RefreshProviderInventoryResponse; } + async GooseProvidersConfigRead( + params: ProviderConfigReadRequest, + ): Promise { + const raw = await this.conn.extMethod( + "_goose/providers/config/read", + params, + ); + return zProviderConfigReadResponse.parse(raw) as ProviderConfigReadResponse; + } + + async GooseProvidersConfigStatus( + params: ProviderConfigStatusRequest, + ): Promise { + const raw = await this.conn.extMethod( + "_goose/providers/config/status", + params, + ); + return zProviderConfigStatusResponse.parse( + raw, + ) as ProviderConfigStatusResponse; + } + + async GooseProvidersConfigSave( + params: ProviderConfigSaveRequest, + ): Promise { + const raw = await this.conn.extMethod( + "_goose/providers/config/save", + params, + ); + return zProviderConfigChangeResponse.parse( + raw, + ) as ProviderConfigChangeResponse; + } + + async GooseProvidersConfigDelete( + params: ProviderConfigDeleteRequest, + ): Promise { + const raw = await this.conn.extMethod( + "_goose/providers/config/delete", + params, + ); + return zProviderConfigChangeResponse.parse( + raw, + ) as ProviderConfigChangeResponse; + } + async GooseConfigRead( params: ReadConfigRequest, ): Promise { diff --git a/ui/sdk/src/generated/index.ts b/ui/sdk/src/generated/index.ts index 8220602d..90469747 100644 --- a/ui/sdk/src/generated/index.ts +++ b/ui/sdk/src/generated/index.ts @@ -1,6 +1,6 @@ // This file is auto-generated by @hey-api/openapi-ts -export type { AddConfigExtensionRequest, AddExtensionRequest, ArchiveSessionRequest, CheckSecretRequest, CheckSecretResponse, CreateSourceRequest, CreateSourceResponse, DeleteSessionRequest, DeleteSourceRequest, DictationConfigRequest, DictationConfigResponse, DictationDownloadProgress, DictationLocalModelStatus, DictationModelCancelRequest, DictationModelDeleteRequest, DictationModelDownloadProgressRequest, DictationModelDownloadProgressResponse, DictationModelDownloadRequest, DictationModelOption, DictationModelSelectRequest, DictationModelsListRequest, DictationModelsListResponse, DictationProviderStatusEntry, DictationTranscribeRequest, DictationTranscribeResponse, EmptyResponse, ExportSessionRequest, ExportSessionResponse, ExportSourceRequest, ExportSourceResponse, ExtRequest, ExtResponse, GetExtensionsRequest, GetExtensionsResponse, GetSessionExtensionsRequest, GetSessionExtensionsResponse, GetToolsRequest, GetToolsResponse, ImportSessionRequest, ImportSessionResponse, ImportSourcesRequest, ImportSourcesResponse, ListProvidersRequest, ListProvidersResponse, ListSourcesRequest, ListSourcesResponse, ProviderConfigKey, ProviderInventoryEntryDto, ProviderInventoryModelDto, ReadConfigRequest, ReadConfigResponse, ReadResourceRequest, ReadResourceResponse, RefreshProviderInventoryRequest, RefreshProviderInventoryResponse, RefreshProviderInventorySkipDto, RefreshProviderInventorySkipReasonDto, RemoveConfigExtensionRequest, RemoveConfigRequest, RemoveExtensionRequest, RemoveSecretRequest, RenameSessionRequest, SourceEntry, SourceType, ToggleConfigExtensionRequest, UnarchiveSessionRequest, UpdateSessionProjectRequest, UpdateSourceRequest, UpdateSourceResponse, UpdateWorkingDirRequest, UpsertConfigRequest, UpsertSecretRequest } from './types.gen.js'; +export type { AddConfigExtensionRequest, AddExtensionRequest, ArchiveSessionRequest, CheckSecretRequest, CheckSecretResponse, CreateSourceRequest, CreateSourceResponse, DeleteSessionRequest, DeleteSourceRequest, DictationConfigRequest, DictationConfigResponse, DictationDownloadProgress, DictationLocalModelStatus, DictationModelCancelRequest, DictationModelDeleteRequest, DictationModelDownloadProgressRequest, DictationModelDownloadProgressResponse, DictationModelDownloadRequest, DictationModelOption, DictationModelSelectRequest, DictationModelsListRequest, DictationModelsListResponse, DictationProviderStatusEntry, DictationTranscribeRequest, DictationTranscribeResponse, EmptyResponse, ExportSessionRequest, ExportSessionResponse, ExportSourceRequest, ExportSourceResponse, ExtRequest, ExtResponse, GetExtensionsRequest, GetExtensionsResponse, GetSessionExtensionsRequest, GetSessionExtensionsResponse, GetToolsRequest, GetToolsResponse, ImportSessionRequest, ImportSessionResponse, ImportSourcesRequest, ImportSourcesResponse, ListProvidersRequest, ListProvidersResponse, ListSourcesRequest, ListSourcesResponse, ProviderConfigChangeResponse, ProviderConfigDeleteRequest, ProviderConfigFieldUpdate, ProviderConfigFieldValueDto, ProviderConfigKey, ProviderConfigReadRequest, ProviderConfigReadResponse, ProviderConfigSaveRequest, ProviderConfigStatusDto, ProviderConfigStatusRequest, ProviderConfigStatusResponse, ProviderInventoryEntryDto, ProviderInventoryModelDto, ReadConfigRequest, ReadConfigResponse, ReadResourceRequest, ReadResourceResponse, RefreshProviderInventoryRequest, RefreshProviderInventoryResponse, RefreshProviderInventorySkipDto, RefreshProviderInventorySkipReasonDto, RemoveConfigExtensionRequest, RemoveConfigRequest, RemoveExtensionRequest, RemoveSecretRequest, RenameSessionRequest, SourceEntry, SourceType, ToggleConfigExtensionRequest, UnarchiveSessionRequest, UpdateSessionProjectRequest, UpdateSourceRequest, UpdateSourceResponse, UpdateWorkingDirRequest, UpsertConfigRequest, UpsertSecretRequest } from './types.gen.js'; export const GOOSE_EXT_METHODS = [ { @@ -68,6 +68,26 @@ export const GOOSE_EXT_METHODS = [ requestType: "RefreshProviderInventoryRequest", responseType: "RefreshProviderInventoryResponse", }, + { + method: "_goose/providers/config/read", + requestType: "ProviderConfigReadRequest", + responseType: "ProviderConfigReadResponse", + }, + { + method: "_goose/providers/config/status", + requestType: "ProviderConfigStatusRequest", + responseType: "ProviderConfigStatusResponse", + }, + { + method: "_goose/providers/config/save", + requestType: "ProviderConfigSaveRequest", + responseType: "ProviderConfigChangeResponse", + }, + { + method: "_goose/providers/config/delete", + requestType: "ProviderConfigDeleteRequest", + responseType: "ProviderConfigChangeResponse", + }, { method: "_goose/config/read", requestType: "ReadConfigRequest", diff --git a/ui/sdk/src/generated/types.gen.ts b/ui/sdk/src/generated/types.gen.ts index 222f30e9..6e8a0557 100644 --- a/ui/sdk/src/generated/types.gen.ts +++ b/ui/sdk/src/generated/types.gen.ts @@ -291,6 +291,66 @@ export type RefreshProviderInventorySkipDto = { export type RefreshProviderInventorySkipReasonDto = 'unknown_provider' | 'not_configured' | 'does_not_support_refresh' | 'already_refreshing'; +/** + * Read saved configuration field values for one provider. + */ +export type ProviderConfigReadRequest = { + providerId: string; +}; + +export type ProviderConfigReadResponse = { + fields: Array; +}; + +export type ProviderConfigFieldValueDto = { + key: string; + value?: string | null; + isSet: boolean; + isSecret: boolean; + required: boolean; +}; + +/** + * Return provider configured statuses. Empty provider_ids means all providers. + */ +export type ProviderConfigStatusRequest = { + providerIds?: Array; +}; + +export type ProviderConfigStatusResponse = { + statuses: Array; +}; + +export type ProviderConfigStatusDto = { + providerId: string; + isConfigured: boolean; +}; + +/** + * Save provider configuration fields and start an inventory refresh when supported. + */ +export type ProviderConfigSaveRequest = { + providerId: string; + fields: Array; +}; + +export type ProviderConfigFieldUpdate = { + key: string; + value: string; +}; + +export type ProviderConfigChangeResponse = { + status: ProviderConfigStatusDto; + refresh: RefreshProviderInventoryResponse; +}; + +/** + * Delete provider configuration fields and start an inventory refresh when supported. + */ +export type ProviderConfigDeleteRequest = { + providerId: string; +}; + /** * Read a single non-secret config value. */ @@ -667,14 +727,14 @@ export type DictationModelSelectRequest = { export type ExtRequest = { id: string; method: string; - params?: AddExtensionRequest | RemoveExtensionRequest | GetToolsRequest | ReadResourceRequest | UpdateWorkingDirRequest | DeleteSessionRequest | GetExtensionsRequest | AddConfigExtensionRequest | RemoveConfigExtensionRequest | ToggleConfigExtensionRequest | GetSessionExtensionsRequest | ListProvidersRequest | RefreshProviderInventoryRequest | ReadConfigRequest | UpsertConfigRequest | RemoveConfigRequest | CheckSecretRequest | UpsertSecretRequest | RemoveSecretRequest | ExportSessionRequest | ImportSessionRequest | UpdateSessionProjectRequest | RenameSessionRequest | ArchiveSessionRequest | UnarchiveSessionRequest | CreateSourceRequest | ListSourcesRequest | UpdateSourceRequest | DeleteSourceRequest | ExportSourceRequest | ImportSourcesRequest | DictationTranscribeRequest | DictationConfigRequest | DictationModelsListRequest | DictationModelDownloadRequest | DictationModelDownloadProgressRequest | DictationModelCancelRequest | DictationModelDeleteRequest | DictationModelSelectRequest | { + params?: AddExtensionRequest | RemoveExtensionRequest | GetToolsRequest | ReadResourceRequest | UpdateWorkingDirRequest | DeleteSessionRequest | GetExtensionsRequest | AddConfigExtensionRequest | RemoveConfigExtensionRequest | ToggleConfigExtensionRequest | GetSessionExtensionsRequest | ListProvidersRequest | RefreshProviderInventoryRequest | ProviderConfigReadRequest | ProviderConfigStatusRequest | ProviderConfigSaveRequest | ProviderConfigDeleteRequest | ReadConfigRequest | UpsertConfigRequest | RemoveConfigRequest | CheckSecretRequest | UpsertSecretRequest | RemoveSecretRequest | ExportSessionRequest | ImportSessionRequest | UpdateSessionProjectRequest | RenameSessionRequest | ArchiveSessionRequest | UnarchiveSessionRequest | CreateSourceRequest | ListSourcesRequest | UpdateSourceRequest | DeleteSourceRequest | ExportSourceRequest | ImportSourcesRequest | DictationTranscribeRequest | DictationConfigRequest | DictationModelsListRequest | DictationModelDownloadRequest | DictationModelDownloadProgressRequest | DictationModelCancelRequest | DictationModelDeleteRequest | DictationModelSelectRequest | { [key: string]: unknown; } | null; }; export type ExtResponse = { id: string; - result?: EmptyResponse | GetToolsResponse | ReadResourceResponse | GetExtensionsResponse | GetSessionExtensionsResponse | ListProvidersResponse | RefreshProviderInventoryResponse | ReadConfigResponse | CheckSecretResponse | ExportSessionResponse | ImportSessionResponse | CreateSourceResponse | ListSourcesResponse | UpdateSourceResponse | ExportSourceResponse | ImportSourcesResponse | DictationTranscribeResponse | DictationConfigResponse | DictationModelsListResponse | DictationModelDownloadProgressResponse | unknown; + result?: EmptyResponse | GetToolsResponse | ReadResourceResponse | GetExtensionsResponse | GetSessionExtensionsResponse | ListProvidersResponse | RefreshProviderInventoryResponse | ProviderConfigReadResponse | ProviderConfigStatusResponse | ProviderConfigChangeResponse | ReadConfigResponse | CheckSecretResponse | ExportSessionResponse | ImportSessionResponse | CreateSourceResponse | ListSourcesResponse | UpdateSourceResponse | ExportSourceResponse | ImportSourcesResponse | DictationTranscribeResponse | DictationConfigResponse | DictationModelsListResponse | DictationModelDownloadProgressResponse | unknown; } | { error: { code: number; diff --git a/ui/sdk/src/generated/zod.gen.ts b/ui/sdk/src/generated/zod.gen.ts index 3c7e1a34..f0f6e594 100644 --- a/ui/sdk/src/generated/zod.gen.ts +++ b/ui/sdk/src/generated/zod.gen.ts @@ -222,6 +222,69 @@ export const zRefreshProviderInventoryResponse = z.object({ skipped: z.array(zRefreshProviderInventorySkipDto).optional().default([]) }); +/** + * Read saved configuration field values for one provider. + */ +export const zProviderConfigReadRequest = z.object({ + providerId: z.string() +}); + +export const zProviderConfigFieldValueDto = z.object({ + key: z.string(), + value: z.union([ + z.string(), + z.null() + ]).optional().default(null), + isSet: z.boolean(), + isSecret: z.boolean(), + required: z.boolean() +}); + +export const zProviderConfigReadResponse = z.object({ + fields: z.array(zProviderConfigFieldValueDto) +}); + +/** + * Return provider configured statuses. Empty provider_ids means all providers. + */ +export const zProviderConfigStatusRequest = z.object({ + providerIds: z.array(z.string()).optional().default([]) +}); + +export const zProviderConfigStatusDto = z.object({ + providerId: z.string(), + isConfigured: z.boolean() +}); + +export const zProviderConfigStatusResponse = z.object({ + statuses: z.array(zProviderConfigStatusDto) +}); + +export const zProviderConfigFieldUpdate = z.object({ + key: z.string(), + value: z.string() +}); + +/** + * Save provider configuration fields and start an inventory refresh when supported. + */ +export const zProviderConfigSaveRequest = z.object({ + providerId: z.string(), + fields: z.array(zProviderConfigFieldUpdate) +}); + +export const zProviderConfigChangeResponse = z.object({ + status: zProviderConfigStatusDto, + refresh: zRefreshProviderInventoryResponse +}); + +/** + * Delete provider configuration fields and start an inventory refresh when supported. + */ +export const zProviderConfigDeleteRequest = z.object({ + providerId: z.string() +}); + /** * Read a single non-secret config value. */ @@ -628,6 +691,10 @@ export const zExtRequest = z.object({ zGetSessionExtensionsRequest, zListProvidersRequest, zRefreshProviderInventoryRequest, + zProviderConfigReadRequest, + zProviderConfigStatusRequest, + zProviderConfigSaveRequest, + zProviderConfigDeleteRequest, zReadConfigRequest, zUpsertConfigRequest, zRemoveConfigRequest, @@ -674,6 +741,9 @@ export const zExtResponse = z.union([ zGetSessionExtensionsResponse, zListProvidersResponse, zRefreshProviderInventoryResponse, + zProviderConfigReadResponse, + zProviderConfigStatusResponse, + zProviderConfigChangeResponse, zReadConfigResponse, zCheckSecretResponse, zExportSessionResponse,