mirror of
https://github.com/block/goose.git
synced 2026-07-17 12:56:20 +02:00
fix: clean up OAuth token cache on provider deletion (#7908)
Signed-off-by: Vincenzo Palazzo <vincenzopalazzodev@gmail.com> Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
This commit is contained in:
@@ -316,7 +316,7 @@ async fn handle_existing_config() -> anyhow::Result<()> {
|
||||
"remove" => remove_extension_dialog(),
|
||||
"settings" => configure_settings_dialog().await,
|
||||
"providers" => configure_provider_dialog().await.map(|_| ()),
|
||||
"custom_providers" => configure_custom_provider_dialog(),
|
||||
"custom_providers" => configure_custom_provider_dialog().await,
|
||||
_ => unreachable!(),
|
||||
}
|
||||
}
|
||||
@@ -2046,7 +2046,7 @@ fn add_provider() -> anyhow::Result<()> {
|
||||
Ok(())
|
||||
}
|
||||
|
||||
fn remove_provider() -> anyhow::Result<()> {
|
||||
async fn remove_provider() -> anyhow::Result<()> {
|
||||
let custom_providers_dir = goose::config::declarative_providers::custom_providers_dir();
|
||||
let custom_providers = if custom_providers_dir.exists() {
|
||||
goose::config::declarative_providers::load_custom_providers(&custom_providers_dir)?
|
||||
@@ -2069,12 +2069,17 @@ fn remove_provider() -> anyhow::Result<()> {
|
||||
.filter_mode()
|
||||
.interact()?;
|
||||
|
||||
// Clean up provider-specific cache files (e.g., OAuth tokens) before removing config
|
||||
if let Err(e) = goose::providers::cleanup_provider(selected_id).await {
|
||||
tracing::warn!("Failed to clean up provider cache: {}", e);
|
||||
}
|
||||
|
||||
remove_custom_provider(selected_id)?;
|
||||
cliclack::outro(format!("Removed custom provider: {}", selected_id))?;
|
||||
Ok(())
|
||||
}
|
||||
|
||||
pub fn configure_custom_provider_dialog() -> anyhow::Result<()> {
|
||||
pub async fn configure_custom_provider_dialog() -> anyhow::Result<()> {
|
||||
let action = cliclack::select("What would you like to do?")
|
||||
.item(
|
||||
"add",
|
||||
@@ -2090,7 +2095,7 @@ pub fn configure_custom_provider_dialog() -> anyhow::Result<()> {
|
||||
|
||||
match action {
|
||||
"add" => add_provider(),
|
||||
"remove" => remove_provider(),
|
||||
"remove" => remove_provider().await,
|
||||
_ => unreachable!(),
|
||||
}?;
|
||||
|
||||
|
||||
@@ -408,6 +408,7 @@ derive_utoipa!(Icon as IconSchema);
|
||||
super::routes::config_management::remove_custom_provider,
|
||||
super::routes::config_management::get_provider_catalog,
|
||||
super::routes::config_management::get_provider_catalog_template,
|
||||
super::routes::config_management::cleanup_provider_cache,
|
||||
super::routes::config_management::check_provider,
|
||||
super::routes::config_management::set_config_provider,
|
||||
super::routes::config_management::configure_provider_oauth,
|
||||
|
||||
@@ -710,6 +710,24 @@ pub async fn remove_custom_provider(Path(id): Path<String>) -> Result<Json<Strin
|
||||
Ok(Json(format!("Removed custom provider: {}", id)))
|
||||
}
|
||||
|
||||
#[utoipa::path(
|
||||
post,
|
||||
path = "/config/providers/{name}/cleanup",
|
||||
params(
|
||||
("name" = String, Path, description = "Provider name (e.g., githubcopilot)")
|
||||
),
|
||||
responses(
|
||||
(status = 200, description = "Provider cache cleaned up successfully", body = String),
|
||||
(status = 500, description = "Internal server error")
|
||||
)
|
||||
)]
|
||||
pub async fn cleanup_provider_cache(
|
||||
Path(name): Path<String>,
|
||||
) -> Result<Json<String>, ErrorResponse> {
|
||||
goose::providers::cleanup_provider(&name).await?;
|
||||
Ok(Json(format!("Cleaned up provider cache: {}", name)))
|
||||
}
|
||||
|
||||
#[utoipa::path(
|
||||
put,
|
||||
path = "/config/custom-providers/{id}",
|
||||
@@ -908,6 +926,10 @@ pub fn routes(state: Arc<AppState>) -> Router {
|
||||
"/config/provider-catalog/{id}",
|
||||
get(get_provider_catalog_template),
|
||||
)
|
||||
.route(
|
||||
"/config/providers/{name}/cleanup",
|
||||
post(cleanup_provider_cache),
|
||||
)
|
||||
.route("/config/detect-provider", post(detect_provider))
|
||||
.route("/config/slash_commands", get(get_slash_commands))
|
||||
.route(
|
||||
|
||||
@@ -135,6 +135,10 @@ pub struct DatabricksProvider {
|
||||
}
|
||||
|
||||
impl DatabricksProvider {
|
||||
pub async fn cleanup() -> Result<()> {
|
||||
super::oauth::cleanup_oauth_cache()
|
||||
}
|
||||
|
||||
pub async fn from_env(model: ModelConfig) -> Result<Self> {
|
||||
let config = crate::config::Config::global();
|
||||
|
||||
|
||||
@@ -118,6 +118,14 @@ impl DiskCache {
|
||||
tokio::fs::write(&self.cache_path, contents).await?;
|
||||
Ok(())
|
||||
}
|
||||
|
||||
async fn clear(&self) -> Result<()> {
|
||||
match tokio::fs::remove_file(&self.cache_path).await {
|
||||
Ok(()) => Ok(()),
|
||||
Err(e) if e.kind() == std::io::ErrorKind::NotFound => Ok(()),
|
||||
Err(e) => Err(e.into()),
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
#[derive(Debug, serde::Serialize)]
|
||||
@@ -134,6 +142,10 @@ pub struct GithubCopilotProvider {
|
||||
}
|
||||
|
||||
impl GithubCopilotProvider {
|
||||
pub async fn cleanup() -> Result<()> {
|
||||
DiskCache::new().clear().await
|
||||
}
|
||||
|
||||
fn payload_contains_image(payload: &Value) -> bool {
|
||||
payload
|
||||
.get("messages")
|
||||
|
||||
@@ -76,6 +76,16 @@ async fn init_registry() -> RwLock<ProviderRegistry> {
|
||||
registry.register::<VeniceProvider>(false);
|
||||
registry.register::<XaiProvider>(false);
|
||||
});
|
||||
// Register cleanup functions for providers with cached state
|
||||
registry.set_cleanup(
|
||||
"github_copilot",
|
||||
Arc::new(|| Box::pin(GithubCopilotProvider::cleanup())),
|
||||
);
|
||||
registry.set_cleanup(
|
||||
"databricks",
|
||||
Arc::new(|| Box::pin(DatabricksProvider::cleanup())),
|
||||
);
|
||||
|
||||
if let Err(e) = load_custom_providers_into_registry(&mut registry) {
|
||||
tracing::warn!("Failed to load custom providers: {}", e);
|
||||
}
|
||||
@@ -146,6 +156,20 @@ pub async fn create_with_default_model(
|
||||
.await
|
||||
}
|
||||
|
||||
pub async fn cleanup_provider(name: &str) -> Result<()> {
|
||||
let cleanup_fn = {
|
||||
let registry = get_registry().await.read().unwrap();
|
||||
registry
|
||||
.entries
|
||||
.get(name)
|
||||
.and_then(|entry| entry.cleanup.clone())
|
||||
};
|
||||
if let Some(cleanup) = cleanup_fn {
|
||||
return cleanup().await;
|
||||
}
|
||||
Ok(())
|
||||
}
|
||||
|
||||
pub async fn create_with_named_model(
|
||||
provider_name: &str,
|
||||
model_name: &str,
|
||||
|
||||
@@ -48,6 +48,7 @@ pub mod venice;
|
||||
pub mod xai;
|
||||
|
||||
pub use init::{
|
||||
create, create_with_default_model, create_with_named_model, providers, refresh_custom_providers,
|
||||
cleanup_provider, create, create_with_default_model, create_with_named_model, providers,
|
||||
refresh_custom_providers,
|
||||
};
|
||||
pub use retry::{retry_operation, RetryConfig};
|
||||
|
||||
@@ -41,6 +41,15 @@ fn get_base_path() -> PathBuf {
|
||||
Paths::in_config_dir("databricks/oauth")
|
||||
}
|
||||
|
||||
pub fn cleanup_oauth_cache() -> Result<()> {
|
||||
let base_path = get_base_path();
|
||||
match fs::remove_dir_all(&base_path) {
|
||||
Ok(()) => Ok(()),
|
||||
Err(e) if e.kind() == std::io::ErrorKind::NotFound => Ok(()),
|
||||
Err(e) => Err(e.into()),
|
||||
}
|
||||
}
|
||||
|
||||
impl TokenCache {
|
||||
fn new(host: &str, client_id: &str, scopes: &[String]) -> Self {
|
||||
let mut hasher = sha2::Sha256::new();
|
||||
|
||||
@@ -12,10 +12,13 @@ pub type ProviderConstructor = Arc<
|
||||
+ Sync,
|
||||
>;
|
||||
|
||||
pub type ProviderCleanup = Arc<dyn Fn() -> BoxFuture<'static, Result<()>> + Send + Sync>;
|
||||
|
||||
#[derive(Clone)]
|
||||
pub struct ProviderEntry {
|
||||
metadata: ProviderMetadata,
|
||||
pub(crate) constructor: ProviderConstructor,
|
||||
pub(crate) cleanup: Option<ProviderCleanup>,
|
||||
provider_type: ProviderType,
|
||||
}
|
||||
|
||||
@@ -61,6 +64,7 @@ impl ProviderRegistry {
|
||||
Ok(Arc::new(provider) as Arc<dyn Provider>)
|
||||
})
|
||||
}),
|
||||
cleanup: None,
|
||||
provider_type: if preferred {
|
||||
ProviderType::Preferred
|
||||
} else {
|
||||
@@ -153,11 +157,18 @@ impl ProviderRegistry {
|
||||
Ok(Arc::new(provider) as Arc<dyn Provider>)
|
||||
})
|
||||
}),
|
||||
cleanup: None,
|
||||
provider_type,
|
||||
},
|
||||
);
|
||||
}
|
||||
|
||||
pub fn set_cleanup(&mut self, name: &str, cleanup: ProviderCleanup) {
|
||||
if let Some(entry) = self.entries.get_mut(name) {
|
||||
entry.cleanup = Some(cleanup);
|
||||
}
|
||||
}
|
||||
|
||||
pub fn with_providers<F>(mut self, setup: F) -> Self
|
||||
where
|
||||
F: FnOnce(&mut Self),
|
||||
|
||||
@@ -1383,6 +1383,40 @@
|
||||
}
|
||||
}
|
||||
},
|
||||
"/config/providers/{name}/cleanup": {
|
||||
"post": {
|
||||
"tags": [
|
||||
"super::routes::config_management"
|
||||
],
|
||||
"operationId": "cleanup_provider_cache",
|
||||
"parameters": [
|
||||
{
|
||||
"name": "name",
|
||||
"in": "path",
|
||||
"description": "Provider name (e.g., githubcopilot)",
|
||||
"required": true,
|
||||
"schema": {
|
||||
"type": "string"
|
||||
}
|
||||
}
|
||||
],
|
||||
"responses": {
|
||||
"200": {
|
||||
"description": "Provider cache cleaned up successfully",
|
||||
"content": {
|
||||
"text/plain": {
|
||||
"schema": {
|
||||
"type": "string"
|
||||
}
|
||||
}
|
||||
}
|
||||
},
|
||||
"500": {
|
||||
"description": "Internal server error"
|
||||
}
|
||||
}
|
||||
}
|
||||
},
|
||||
"/config/providers/{name}/models": {
|
||||
"get": {
|
||||
"tags": [
|
||||
|
||||
File diff suppressed because one or more lines are too long
File diff suppressed because one or more lines are too long
@@ -2655,6 +2655,34 @@ export type ProvidersResponses = {
|
||||
|
||||
export type ProvidersResponse2 = ProvidersResponses[keyof ProvidersResponses];
|
||||
|
||||
export type CleanupProviderCacheData = {
|
||||
body?: never;
|
||||
path: {
|
||||
/**
|
||||
* Provider name (e.g., githubcopilot)
|
||||
*/
|
||||
name: string;
|
||||
};
|
||||
query?: never;
|
||||
url: '/config/providers/{name}/cleanup';
|
||||
};
|
||||
|
||||
export type CleanupProviderCacheErrors = {
|
||||
/**
|
||||
* Internal server error
|
||||
*/
|
||||
500: unknown;
|
||||
};
|
||||
|
||||
export type CleanupProviderCacheResponses = {
|
||||
/**
|
||||
* Provider cache cleaned up successfully
|
||||
*/
|
||||
200: string;
|
||||
};
|
||||
|
||||
export type CleanupProviderCacheResponse = CleanupProviderCacheResponses[keyof CleanupProviderCacheResponses];
|
||||
|
||||
export type GetProviderModelsData = {
|
||||
body?: never;
|
||||
path: {
|
||||
|
||||
@@ -17,7 +17,12 @@ import { providerConfigSubmitHandler } from './subcomponents/handlers/DefaultSub
|
||||
import { useConfig } from '../../../ConfigContext';
|
||||
import { useModelAndProvider } from '../../../ModelAndProviderContext';
|
||||
import { AlertTriangle, LogIn } from 'lucide-react';
|
||||
import { ProviderDetails, removeCustomProvider, configureProviderOauth } from '../../../../api';
|
||||
import {
|
||||
ProviderDetails,
|
||||
removeCustomProvider,
|
||||
configureProviderOauth,
|
||||
cleanupProviderCache,
|
||||
} from '../../../../api';
|
||||
import { Button } from '../../../../components/ui/button';
|
||||
import { errorMessage } from '../../../../utils/conversionUtils';
|
||||
|
||||
@@ -153,6 +158,13 @@ export default function ProviderConfigurationModal({
|
||||
return;
|
||||
}
|
||||
|
||||
// Clean up provider-specific cache files (e.g., OAuth tokens) before removing config
|
||||
try {
|
||||
await cleanupProviderCache({ path: { name: provider.name } });
|
||||
} catch {
|
||||
// Cleanup is best-effort — proceed with deletion even if it fails
|
||||
}
|
||||
|
||||
const isCustomProvider = provider.provider_type === 'Custom';
|
||||
|
||||
if (isCustomProvider) {
|
||||
|
||||
Reference in New Issue
Block a user