Skip to content

Commit beef886

Browse files
committed
fix(api): provider endpoint mismatches preventing Copilot save, test, and remove
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). 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.
1 parent 21868af commit beef886

3 files changed

Lines changed: 112 additions & 1 deletion

File tree

src/api/providers.rs

Lines changed: 88 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -207,6 +207,14 @@ fn build_test_llm_config(provider: &str, credential: &str) -> crate::config::Llm
207207
let mut providers = HashMap::new();
208208
if let Some(provider_config) = crate::config::default_provider_config(provider, credential) {
209209
providers.insert(provider.to_string(), provider_config);
210+
} else if provider == "github-copilot" {
211+
// GitHub Copilot uses token exchange, so default_provider_config returns None.
212+
// For testing, add a provider entry with the PAT as the api_key —
213+
// LlmManager::get_copilot_token() will exchange it for a real Copilot token.
214+
providers.insert(
215+
provider.to_string(),
216+
crate::config::copilot_default_provider_config(credential),
217+
);
210218
}
211219

212220
crate::config::LlmConfig {
@@ -1591,6 +1599,66 @@ pub(super) async fn delete_provider(
15911599
StatusCode::INTERNAL_SERVER_ERROR
15921600
})?;
15931601

1602+
let doc: toml_edit::DocumentMut = content.parse().map_err(|error| {
1603+
tracing::error!(%error, "failed to parse config.toml for provider removal");
1604+
StatusCode::INTERNAL_SERVER_ERROR
1605+
})?;
1606+
1607+
// Check if the provider has a key in TOML
1608+
let has_toml_key = doc
1609+
.get("llm")
1610+
.and_then(|llm| llm.get(key_name))
1611+
.and_then(|val| val.as_str())
1612+
.is_some_and(|s| {
1613+
if let Some(alias) = s.strip_prefix("secret:") {
1614+
// Check if secret exists in secrets store
1615+
state
1616+
.secrets_store
1617+
.load()
1618+
.as_ref()
1619+
.as_ref()
1620+
.and_then(|store| store.get(alias).ok())
1621+
.is_some()
1622+
} else if let Some(var_name) = s.strip_prefix("env:") {
1623+
// Check if env var exists and is non-empty
1624+
std::env::var(var_name)
1625+
.ok()
1626+
.is_some_and(|v| !v.trim().is_empty())
1627+
} else {
1628+
// Direct value
1629+
!s.trim().is_empty()
1630+
}
1631+
});
1632+
1633+
// If no TOML key exists, check if configured via env var
1634+
if !has_toml_key {
1635+
let env_vars: Vec<String> = match provider.as_str() {
1636+
"ollama" => vec!["OLLAMA_BASE_URL".to_string(), "OLLAMA_API_KEY".to_string()],
1637+
_ => vec![format!(
1638+
"{}_API_KEY",
1639+
provider.to_uppercase().replace("-", "_")
1640+
)],
1641+
};
1642+
1643+
let configured_env_var = env_vars.iter().find(|name| {
1644+
std::env::var(name.as_str())
1645+
.ok()
1646+
.is_some_and(|v| !v.trim().is_empty())
1647+
});
1648+
1649+
if let Some(env_var) = configured_env_var {
1650+
return Ok(Json(ProviderUpdateResponse {
1651+
success: false,
1652+
message: format!(
1653+
"Provider '{}' is configured via the {} environment variable. \
1654+
To remove this provider, unset the environment variable and restart Spacebot.",
1655+
provider, env_var
1656+
),
1657+
}));
1658+
}
1659+
}
1660+
1661+
// Now remove from TOML
15941662
let mut doc: toml_edit::DocumentMut = content.parse().map_err(|error| {
15951663
tracing::error!(%error, "failed to parse config.toml for provider removal");
15961664
StatusCode::INTERNAL_SERVER_ERROR
@@ -1630,4 +1698,24 @@ mod tests {
16301698
assert_eq!(provider.base_url, "http://remote-ollama.local:11434");
16311699
assert_eq!(provider.api_key, "");
16321700
}
1701+
1702+
#[test]
1703+
fn build_test_llm_config_registers_github_copilot_provider() {
1704+
let config = build_test_llm_config("github-copilot", "ghp_test_pat_token");
1705+
let provider = config
1706+
.providers
1707+
.get("github-copilot")
1708+
.expect("github-copilot provider should be registered");
1709+
1710+
assert_eq!(
1711+
provider.base_url,
1712+
crate::config::GITHUB_COPILOT_DEFAULT_BASE_URL
1713+
);
1714+
assert_eq!(provider.api_key, "ghp_test_pat_token");
1715+
assert!(provider.use_bearer_auth);
1716+
assert_eq!(
1717+
config.github_copilot_key.as_deref(),
1718+
Some("ghp_test_pat_token")
1719+
);
1720+
}
16331721
}

src/config.rs

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,10 @@ pub use permissions::{
1818
DiscordPermissions, MattermostPermissions, SignalPermissions, SlackPermissions,
1919
TelegramPermissions, TwitchPermissions,
2020
};
21+
// Only used in tests; #[allow(unused_imports)] prevents warnings during non-test builds.
22+
#[allow(unused_imports)]
23+
pub(crate) use providers::GITHUB_COPILOT_DEFAULT_BASE_URL;
24+
pub(crate) use providers::copilot_default_provider_config;
2125
pub(crate) use providers::default_provider_config;
2226
pub use runtime::RuntimeConfig;
2327
pub use types::*;
@@ -1349,6 +1353,10 @@ id = "main"
13491353
"ollama",
13501354
"localhost:11434",
13511355
),
1356+
// Note: github_copilot_key is intentionally excluded — GitHub Copilot requires
1357+
// special token exchange handling and is registered via
1358+
// LlmManager::get_github_copilot_provider(), not via the standard shorthand
1359+
// provider registration mechanism used by other providers.
13521360
];
13531361

13541362
for (toml_key, toml_value, provider_name, url_substr) in cases {

src/config/providers.rs

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ pub(super) const NVIDIA_PROVIDER_BASE_URL: &str = "https://integrate.api.nvidia.
2626
pub(super) const FIREWORKS_PROVIDER_BASE_URL: &str = "https://api.fireworks.ai/inference";
2727
pub(crate) const GEMINI_PROVIDER_BASE_URL: &str =
2828
"https://generativelanguage.googleapis.com/v1beta/openai";
29-
pub(super) const GITHUB_COPILOT_DEFAULT_BASE_URL: &str = "https://api.individual.githubcopilot.com";
29+
pub(crate) const GITHUB_COPILOT_DEFAULT_BASE_URL: &str = "https://api.individual.githubcopilot.com";
3030

3131
/// App attribution headers sent with every OpenRouter API request.
3232
/// See <https://openrouter.ai/docs/app-attribution>.
@@ -286,6 +286,21 @@ pub(super) fn add_shorthand_provider(
286286
}
287287
}
288288

289+
/// Returns the default ProviderConfig for GitHub Copilot with the given API key.
290+
/// Used by API tests and other code that needs Copilot provider configs.
291+
pub fn copilot_default_provider_config(api_key: impl Into<String>) -> super::ProviderConfig {
292+
super::ProviderConfig {
293+
api_type: super::ApiType::OpenAiChatCompletions,
294+
base_url: GITHUB_COPILOT_DEFAULT_BASE_URL.to_string(),
295+
api_key: api_key.into(),
296+
name: Some("GitHub Copilot".to_string()),
297+
use_bearer_auth: true,
298+
extra_headers: vec![],
299+
api_version: None,
300+
deployment: None,
301+
}
302+
}
303+
289304
/// When `[defaults.routing]` is absent from the config file, pick routing
290305
/// defaults based on which provider the user actually has configured. This
291306
/// avoids the common pitfall where a user sets up OpenRouter (or another

0 commit comments

Comments
 (0)