From ee04701d0639afb59f0946ef41e646a62ff19b2d Mon Sep 17 00:00:00 2001 From: bhagwan Date: Sun, 29 Mar 2026 13:54:54 -0400 Subject: [PATCH 1/2] fix(api): provider endpoint mismatches preventing Copilot save, test, and remove MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fix #415 Three endpoint mismatches between frontend and backend caused the GitHub Copilot provider's save, test, and remove buttons to fail with 405 Method Not Allowed. These affected all providers for save. - change update_provider annotation from post to put to match frontend - fix test button URL from /providers/test to /providers/test-model - add github-copilot entry in build_test_llm_config since default_provider_config returns None for providers that require token exchange - widen GITHUB_COPILOT_DEFAULT_BASE_URL visibility to pub(crate) - add unit test for build_test_llm_config with github-copilot fix(api): Copilot provider shows as available after remove when env var is set get_providers fell back to the GITHUB_COPILOT_API_KEY env var when the TOML key was absent, so the provider stayed visible in settings after a remove — the env var can't be unset from a running process. Only check the TOML key for Copilot status in the config-exists path. The env var fallback remains for the no-config-file case (fresh install). --- src/api/providers.rs | 48 ++++++++++++++++++++++++++++++++++++++++- src/config.rs | 1 + src/config/providers.rs | 2 +- 3 files changed, 49 insertions(+), 2 deletions(-) diff --git a/src/api/providers.rs b/src/api/providers.rs index 894f8c2e9..d53ada352 100644 --- a/src/api/providers.rs +++ b/src/api/providers.rs @@ -207,6 +207,23 @@ fn build_test_llm_config(provider: &str, credential: &str) -> crate::config::Llm let mut providers = HashMap::new(); if let Some(provider_config) = crate::config::default_provider_config(provider, credential) { providers.insert(provider.to_string(), provider_config); + } else if provider == "github-copilot" { + // GitHub Copilot uses token exchange, so default_provider_config returns None. + // For testing, add a provider entry with the PAT as the api_key — + // LlmManager::get_copilot_token() will exchange it for a real Copilot token. + providers.insert( + provider.to_string(), + crate::config::ProviderConfig { + api_type: crate::config::ApiType::OpenAiChatCompletions, + base_url: crate::config::GITHUB_COPILOT_DEFAULT_BASE_URL.to_string(), + api_key: credential.to_string(), + name: Some("GitHub Copilot".to_string()), + use_bearer_auth: true, + extra_headers: vec![], + api_version: None, + deployment: None, + }, + ); } crate::config::LlmConfig { @@ -447,6 +464,15 @@ pub(super) async fn get_providers( env_set(env_var) }; + // Copilot: only check TOML key, not env var. The env var GITHUB_COPILOT_API_KEY + // is a fallback for config loading but shouldn't keep the provider visible in + // the settings UI after a remove — the env var can't be unset from the process. + let copilot_configured = doc + .get("llm") + .and_then(|llm| llm.get("github_copilot_key")) + .and_then(|val| val.as_str()) + .is_some_and(|s| resolve_value(s).is_some()); + ( has_value("anthropic_key", "ANTHROPIC_API_KEY"), has_value("openai_key", "OPENAI_API_KEY"), @@ -470,7 +496,7 @@ pub(super) async fn get_providers( has_value("minimax_cn_key", "MINIMAX_CN_API_KEY"), has_value("moonshot_key", "MOONSHOT_API_KEY"), has_value("zai_coding_plan_key", "ZAI_CODING_PLAN_API_KEY"), - has_value("github_copilot_key", "GITHUB_COPILOT_API_KEY"), + copilot_configured, doc.get("llm") .and_then(|llm| llm.get("provider")) .and_then(|provider| provider.get("azure")) @@ -1607,4 +1633,24 @@ mod tests { assert_eq!(provider.base_url, "http://remote-ollama.local:11434"); assert_eq!(provider.api_key, ""); } + + #[test] + fn build_test_llm_config_registers_github_copilot_provider() { + let config = build_test_llm_config("github-copilot", "ghp_test_pat_token"); + let provider = config + .providers + .get("github-copilot") + .expect("github-copilot provider should be registered"); + + assert_eq!( + provider.base_url, + crate::config::GITHUB_COPILOT_DEFAULT_BASE_URL + ); + assert_eq!(provider.api_key, "ghp_test_pat_token"); + assert!(provider.use_bearer_auth); + assert_eq!( + config.github_copilot_key.as_deref(), + Some("ghp_test_pat_token") + ); + } } diff --git a/src/config.rs b/src/config.rs index eb0891a9a..480f13b0c 100644 --- a/src/config.rs +++ b/src/config.rs @@ -18,6 +18,7 @@ pub use permissions::{ DiscordPermissions, MattermostPermissions, SignalPermissions, SlackPermissions, TelegramPermissions, TwitchPermissions, }; +pub(crate) use providers::GITHUB_COPILOT_DEFAULT_BASE_URL; pub(crate) use providers::default_provider_config; pub use runtime::RuntimeConfig; pub use types::*; diff --git a/src/config/providers.rs b/src/config/providers.rs index 575b9cb2b..46dbf8a7e 100644 --- a/src/config/providers.rs +++ b/src/config/providers.rs @@ -26,7 +26,7 @@ pub(super) const NVIDIA_PROVIDER_BASE_URL: &str = "https://integrate.api.nvidia. pub(super) const FIREWORKS_PROVIDER_BASE_URL: &str = "https://api.fireworks.ai/inference"; pub(crate) const GEMINI_PROVIDER_BASE_URL: &str = "https://generativelanguage.googleapis.com/v1beta/openai"; -pub(super) const GITHUB_COPILOT_DEFAULT_BASE_URL: &str = "https://api.individual.githubcopilot.com"; +pub(crate) const GITHUB_COPILOT_DEFAULT_BASE_URL: &str = "https://api.individual.githubcopilot.com"; /// App attribution headers sent with every OpenRouter API request. /// See . From e8a76df1025ffe836e5a18bb01e5b5e06ed1f0de Mon Sep 17 00:00:00 2001 From: bhagwan Date: Thu, 2 Apr 2026 10:50:14 -0400 Subject: [PATCH 2/2] fix(api): make Copilot provider visibility consistent with other providers - Change Copilot from TOML-only check to has_value() pattern, consistent with all other providers. Now env var GITHUB_COPILOT_API_KEY works like other provider env vars in get_providers(). - Add helpful error message in delete_provider() when attempting to remove a provider that is only configured via environment variable. The message explains which env var to unset and that a restart is required. For Ollama, checks both OLLAMA_BASE_URL and OLLAMA_API_KEY env vars. - Add comment in test explaining why github_copilot_key is excluded from all_shorthand_keys_register_providers_via_toml test (uses token exchange, not standard shorthand registration). - Extract copilot_default_provider_config() helper to reduce duplication in Copilot provider construction. --- src/api/providers.rs | 81 +++++++++++++++++++++++++++++++---------- src/config.rs | 7 ++++ src/config/providers.rs | 15 ++++++++ 3 files changed, 83 insertions(+), 20 deletions(-) diff --git a/src/api/providers.rs b/src/api/providers.rs index d53ada352..c90437694 100644 --- a/src/api/providers.rs +++ b/src/api/providers.rs @@ -213,16 +213,7 @@ fn build_test_llm_config(provider: &str, credential: &str) -> crate::config::Llm // LlmManager::get_copilot_token() will exchange it for a real Copilot token. providers.insert( provider.to_string(), - crate::config::ProviderConfig { - api_type: crate::config::ApiType::OpenAiChatCompletions, - base_url: crate::config::GITHUB_COPILOT_DEFAULT_BASE_URL.to_string(), - api_key: credential.to_string(), - name: Some("GitHub Copilot".to_string()), - use_bearer_auth: true, - extra_headers: vec![], - api_version: None, - deployment: None, - }, + crate::config::copilot_default_provider_config(credential), ); } @@ -464,15 +455,6 @@ pub(super) async fn get_providers( env_set(env_var) }; - // Copilot: only check TOML key, not env var. The env var GITHUB_COPILOT_API_KEY - // is a fallback for config loading but shouldn't keep the provider visible in - // the settings UI after a remove — the env var can't be unset from the process. - let copilot_configured = doc - .get("llm") - .and_then(|llm| llm.get("github_copilot_key")) - .and_then(|val| val.as_str()) - .is_some_and(|s| resolve_value(s).is_some()); - ( has_value("anthropic_key", "ANTHROPIC_API_KEY"), has_value("openai_key", "OPENAI_API_KEY"), @@ -496,7 +478,7 @@ pub(super) async fn get_providers( has_value("minimax_cn_key", "MINIMAX_CN_API_KEY"), has_value("moonshot_key", "MOONSHOT_API_KEY"), has_value("zai_coding_plan_key", "ZAI_CODING_PLAN_API_KEY"), - copilot_configured, + has_value("github_copilot_key", "GITHUB_COPILOT_API_KEY"), doc.get("llm") .and_then(|llm| llm.get("provider")) .and_then(|provider| provider.get("azure")) @@ -1598,6 +1580,65 @@ pub(super) async fn delete_provider( .await .map_err(|_| StatusCode::INTERNAL_SERVER_ERROR)?; + let doc: toml_edit::DocumentMut = content + .parse() + .map_err(|_| StatusCode::INTERNAL_SERVER_ERROR)?; + + // Check if the provider has a key in TOML + let has_toml_key = doc + .get("llm") + .and_then(|llm| llm.get(key_name)) + .and_then(|val| val.as_str()) + .is_some_and(|s| { + if let Some(alias) = s.strip_prefix("secret:") { + // Check if secret exists in secrets store + state + .secrets_store + .load() + .as_ref() + .as_ref() + .and_then(|store| store.get(alias).ok()) + .is_some() + } else if let Some(var_name) = s.strip_prefix("env:") { + // Check if env var exists and is non-empty + std::env::var(var_name) + .ok() + .is_some_and(|v| !v.trim().is_empty()) + } else { + // Direct value + !s.trim().is_empty() + } + }); + + // If no TOML key exists, check if configured via env var + if !has_toml_key { + let env_vars: Vec = match provider.as_str() { + "ollama" => vec!["OLLAMA_BASE_URL".to_string(), "OLLAMA_API_KEY".to_string()], + _ => vec![format!( + "{}_API_KEY", + provider.to_uppercase().replace("-", "_") + )], + }; + + let configured_env_var = env_vars.iter().find(|name| { + std::env::var(name.as_str()) + .ok() + .is_some_and(|v| !v.trim().is_empty()) + }); + + if let Some(env_var) = configured_env_var { + return Ok(Json(ProviderUpdateResponse { + success: false, + message: format!( + "Provider '{}' is configured via the {} environment variable. \ + To remove this provider, unset the environment variable and restart Spacebot.", + provider, env_var + ), + })); + } + } + + // Now remove from TOML let mut doc: toml_edit::DocumentMut = content .parse() .map_err(|_| StatusCode::INTERNAL_SERVER_ERROR)?; diff --git a/src/config.rs b/src/config.rs index 480f13b0c..a4d529d0c 100644 --- a/src/config.rs +++ b/src/config.rs @@ -18,7 +18,10 @@ pub use permissions::{ DiscordPermissions, MattermostPermissions, SignalPermissions, SlackPermissions, TelegramPermissions, TwitchPermissions, }; +// Only used in tests; #[allow(unused_imports)] prevents warnings during non-test builds. +#[allow(unused_imports)] pub(crate) use providers::GITHUB_COPILOT_DEFAULT_BASE_URL; +pub(crate) use providers::copilot_default_provider_config; pub(crate) use providers::default_provider_config; pub use runtime::RuntimeConfig; pub use types::*; @@ -1307,6 +1310,10 @@ maintenance_merge_similarity_threshold = 1.1 "ollama", "localhost:11434", ), + // Note: github_copilot_key is intentionally excluded — GitHub Copilot requires + // special token exchange handling and is registered via + // LlmManager::get_github_copilot_provider(), not via the standard shorthand + // provider registration mechanism used by other providers. ]; for (toml_key, toml_value, provider_name, url_substr) in cases { diff --git a/src/config/providers.rs b/src/config/providers.rs index 46dbf8a7e..fc4d7191a 100644 --- a/src/config/providers.rs +++ b/src/config/providers.rs @@ -286,6 +286,21 @@ pub(super) fn add_shorthand_provider( } } +/// Returns the default ProviderConfig for GitHub Copilot with the given API key. +/// Used by API tests and other code that needs Copilot provider configs. +pub fn copilot_default_provider_config(api_key: impl Into) -> super::ProviderConfig { + super::ProviderConfig { + api_type: super::ApiType::OpenAiChatCompletions, + base_url: GITHUB_COPILOT_DEFAULT_BASE_URL.to_string(), + api_key: api_key.into(), + name: Some("GitHub Copilot".to_string()), + use_bearer_auth: true, + extra_headers: vec![], + api_version: None, + deployment: None, + } +} + /// When `[defaults.routing]` is absent from the config file, pick routing /// defaults based on which provider the user actually has configured. This /// avoids the common pitfall where a user sets up OpenRouter (or another