From 2acfa5e948764cbe9ae5cbf9f95d6bf66ea904c2 Mon Sep 17 00:00:00 2001 From: smit Date: Thu, 14 Aug 2025 23:28:15 +0530 Subject: [PATCH] copilot: Fix Copilot fails to sign in on newer versions (#36195) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Follow-up for #36093 and https://github.com/zed-industries/zed/pull/36138 Since v1.355.0, `@github/copilot-language-server` has stopped responding to `CheckStatus` requests if a `DidChangeConfiguration` notification hasn’t been sent beforehand. This causes `CheckStatus` to remain in an await state until it times out, leaving the connection stuck for a long period before finally throwing a timeout error. ```rs let status = server .request::(request::CheckStatusParams { local_checks_only: false, }) .await .into_response() // bails here with ConnectionResult::Timeout .context("copilot: check status")?; ```` This PR fixes the issue by sending the `DidChangeConfiguration` notification before making the `CheckStatus` request. It’s just an ordering change i.e. no other LSP actions occur between these two calls. Previously, we only updated our internal connection status and UI in between. Release Notes: - Fixed an issue where GitHub Copilot could get stuck and fail to sign in. --- crates/copilot/src/copilot.rs | 95 +++++++++++++------------ crates/languages/src/css.rs | 5 +- crates/languages/src/json.rs | 5 +- crates/languages/src/python.rs | 5 +- crates/languages/src/tailwind.rs | 5 +- crates/languages/src/typescript.rs | 5 +- crates/languages/src/vtsls.rs | 8 +-- crates/languages/src/yaml.rs | 5 +- crates/node_runtime/src/node_runtime.rs | 34 +++++---- 9 files changed, 83 insertions(+), 84 deletions(-) diff --git a/crates/copilot/src/copilot.rs b/crates/copilot/src/copilot.rs index 166a582c70..dcebeae721 100644 --- a/crates/copilot/src/copilot.rs +++ b/crates/copilot/src/copilot.rs @@ -21,7 +21,7 @@ use language::{ point_from_lsp, point_to_lsp, }; use lsp::{LanguageServer, LanguageServerBinary, LanguageServerId, LanguageServerName}; -use node_runtime::{NodeRuntime, VersionCheck}; +use node_runtime::{NodeRuntime, VersionStrategy}; use parking_lot::Mutex; use project::DisableAiSettings; use request::StatusNotification; @@ -349,7 +349,11 @@ impl Copilot { this.start_copilot(true, false, cx); cx.observe_global::(move |this, cx| { this.start_copilot(true, false, cx); - this.send_configuration_update(cx); + if let Ok(server) = this.server.as_running() { + notify_did_change_config_to_server(&server.lsp, cx) + .context("copilot setting change: did change configuration") + .log_err(); + } }) .detach(); this @@ -438,43 +442,6 @@ impl Copilot { if env.is_empty() { None } else { Some(env) } } - fn send_configuration_update(&mut self, cx: &mut Context) { - let copilot_settings = all_language_settings(None, cx) - .edit_predictions - .copilot - .clone(); - - let settings = json!({ - "http": { - "proxy": copilot_settings.proxy, - "proxyStrictSSL": !copilot_settings.proxy_no_verify.unwrap_or(false) - }, - "github-enterprise": { - "uri": copilot_settings.enterprise_uri - } - }); - - if let Some(copilot_chat) = copilot_chat::CopilotChat::global(cx) { - copilot_chat.update(cx, |chat, cx| { - chat.set_configuration( - copilot_chat::CopilotChatConfiguration { - enterprise_uri: copilot_settings.enterprise_uri.clone(), - }, - cx, - ); - }); - } - - if let Ok(server) = self.server.as_running() { - server - .lsp - .notify::( - &lsp::DidChangeConfigurationParams { settings }, - ) - .log_err(); - } - } - #[cfg(any(test, feature = "test-support"))] pub fn fake(cx: &mut gpui::TestAppContext) -> (Entity, lsp::FakeLanguageServer) { use fs::FakeFs; @@ -573,6 +540,9 @@ impl Copilot { })? .await?; + this.update(cx, |_, cx| notify_did_change_config_to_server(&server, cx))? + .context("copilot: did change configuration")?; + let status = server .request::(request::CheckStatusParams { local_checks_only: false, @@ -598,8 +568,6 @@ impl Copilot { }); cx.emit(Event::CopilotLanguageServerStarted); this.update_sign_in_status(status, cx); - // Send configuration now that the LSP is fully started - this.send_configuration_update(cx); } Err(error) => { this.server = CopilotServer::Error(error.to_string().into()); @@ -1156,6 +1124,41 @@ fn uri_for_buffer(buffer: &Entity, cx: &App) -> Result { } } +fn notify_did_change_config_to_server( + server: &Arc, + cx: &mut Context, +) -> std::result::Result<(), anyhow::Error> { + let copilot_settings = all_language_settings(None, cx) + .edit_predictions + .copilot + .clone(); + + if let Some(copilot_chat) = copilot_chat::CopilotChat::global(cx) { + copilot_chat.update(cx, |chat, cx| { + chat.set_configuration( + copilot_chat::CopilotChatConfiguration { + enterprise_uri: copilot_settings.enterprise_uri.clone(), + }, + cx, + ); + }); + } + + let settings = json!({ + "http": { + "proxy": copilot_settings.proxy, + "proxyStrictSSL": !copilot_settings.proxy_no_verify.unwrap_or(false) + }, + "github-enterprise": { + "uri": copilot_settings.enterprise_uri + } + }); + + server.notify::(&lsp::DidChangeConfigurationParams { + settings, + }) +} + async fn clear_copilot_dir() { remove_matching(paths::copilot_dir(), |_| true).await } @@ -1169,8 +1172,9 @@ async fn get_copilot_lsp(fs: Arc, node_runtime: NodeRuntime) -> anyhow:: const SERVER_PATH: &str = "node_modules/@github/copilot-language-server/dist/language-server.js"; - // pinning it: https://github.com/zed-industries/zed/issues/36093 - const PINNED_VERSION: &str = "1.354"; + let latest_version = node_runtime + .npm_package_latest_version(PACKAGE_NAME) + .await?; let server_path = paths::copilot_dir().join(SERVER_PATH); fs.create_dir(paths::copilot_dir()).await?; @@ -1180,13 +1184,12 @@ async fn get_copilot_lsp(fs: Arc, node_runtime: NodeRuntime) -> anyhow:: PACKAGE_NAME, &server_path, paths::copilot_dir(), - &PINNED_VERSION, - VersionCheck::VersionMismatch, + VersionStrategy::Latest(&latest_version), ) .await; if should_install { node_runtime - .npm_install_packages(paths::copilot_dir(), &[(PACKAGE_NAME, &PINNED_VERSION)]) + .npm_install_packages(paths::copilot_dir(), &[(PACKAGE_NAME, &latest_version)]) .await?; } diff --git a/crates/languages/src/css.rs b/crates/languages/src/css.rs index 19329fcc6e..ffd9006c76 100644 --- a/crates/languages/src/css.rs +++ b/crates/languages/src/css.rs @@ -4,7 +4,7 @@ use futures::StreamExt; use gpui::AsyncApp; use language::{LanguageToolchainStore, LspAdapter, LspAdapterDelegate}; use lsp::{LanguageServerBinary, LanguageServerName}; -use node_runtime::NodeRuntime; +use node_runtime::{NodeRuntime, VersionStrategy}; use project::{Fs, lsp_store::language_server_settings}; use serde_json::json; use smol::fs; @@ -107,8 +107,7 @@ impl LspAdapter for CssLspAdapter { Self::PACKAGE_NAME, &server_path, &container_dir, - &version, - Default::default(), + VersionStrategy::Latest(version), ) .await; diff --git a/crates/languages/src/json.rs b/crates/languages/src/json.rs index 019b45d396..484631d01f 100644 --- a/crates/languages/src/json.rs +++ b/crates/languages/src/json.rs @@ -12,7 +12,7 @@ use language::{ LspAdapter, LspAdapterDelegate, }; use lsp::{LanguageServerBinary, LanguageServerName}; -use node_runtime::NodeRuntime; +use node_runtime::{NodeRuntime, VersionStrategy}; use project::{Fs, lsp_store::language_server_settings}; use serde_json::{Value, json}; use settings::{KeymapFile, SettingsJsonSchemaParams, SettingsStore}; @@ -344,8 +344,7 @@ impl LspAdapter for JsonLspAdapter { Self::PACKAGE_NAME, &server_path, &container_dir, - &version, - Default::default(), + VersionStrategy::Latest(version), ) .await; diff --git a/crates/languages/src/python.rs b/crates/languages/src/python.rs index 5513324487..40131089d1 100644 --- a/crates/languages/src/python.rs +++ b/crates/languages/src/python.rs @@ -13,7 +13,7 @@ use language::{LanguageName, ManifestName, ManifestProvider, ManifestQuery}; use language::{Toolchain, WorkspaceFoldersContent}; use lsp::LanguageServerBinary; use lsp::LanguageServerName; -use node_runtime::NodeRuntime; +use node_runtime::{NodeRuntime, VersionStrategy}; use pet_core::Configuration; use pet_core::os_environment::Environment; use pet_core::python_environment::PythonEnvironmentKind; @@ -205,8 +205,7 @@ impl LspAdapter for PythonLspAdapter { Self::SERVER_NAME.as_ref(), &server_path, &container_dir, - &version, - Default::default(), + VersionStrategy::Latest(version), ) .await; diff --git a/crates/languages/src/tailwind.rs b/crates/languages/src/tailwind.rs index 6f03eeda8d..0d647f07cf 100644 --- a/crates/languages/src/tailwind.rs +++ b/crates/languages/src/tailwind.rs @@ -5,7 +5,7 @@ use futures::StreamExt; use gpui::AsyncApp; use language::{LanguageName, LanguageToolchainStore, LspAdapter, LspAdapterDelegate}; use lsp::{LanguageServerBinary, LanguageServerName}; -use node_runtime::NodeRuntime; +use node_runtime::{NodeRuntime, VersionStrategy}; use project::{Fs, lsp_store::language_server_settings}; use serde_json::{Value, json}; use smol::fs; @@ -112,8 +112,7 @@ impl LspAdapter for TailwindLspAdapter { Self::PACKAGE_NAME, &server_path, &container_dir, - &version, - Default::default(), + VersionStrategy::Latest(version), ) .await; diff --git a/crates/languages/src/typescript.rs b/crates/languages/src/typescript.rs index a8ba880889..1877c86dc5 100644 --- a/crates/languages/src/typescript.rs +++ b/crates/languages/src/typescript.rs @@ -10,7 +10,7 @@ use language::{ LspAdapterDelegate, }; use lsp::{CodeActionKind, LanguageServerBinary, LanguageServerName}; -use node_runtime::NodeRuntime; +use node_runtime::{NodeRuntime, VersionStrategy}; use project::{Fs, lsp_store::language_server_settings}; use serde_json::{Value, json}; use smol::{fs, lock::RwLock, stream::StreamExt}; @@ -588,8 +588,7 @@ impl LspAdapter for TypeScriptLspAdapter { Self::PACKAGE_NAME, &server_path, &container_dir, - version.typescript_version.as_str(), - Default::default(), + VersionStrategy::Latest(version.typescript_version.as_str()), ) .await; diff --git a/crates/languages/src/vtsls.rs b/crates/languages/src/vtsls.rs index 73498fc579..90faf883ba 100644 --- a/crates/languages/src/vtsls.rs +++ b/crates/languages/src/vtsls.rs @@ -4,7 +4,7 @@ use collections::HashMap; use gpui::AsyncApp; use language::{LanguageName, LanguageToolchainStore, LspAdapter, LspAdapterDelegate}; use lsp::{CodeActionKind, LanguageServerBinary, LanguageServerName}; -use node_runtime::NodeRuntime; +use node_runtime::{NodeRuntime, VersionStrategy}; use project::{Fs, lsp_store::language_server_settings}; use serde_json::Value; use std::{ @@ -115,8 +115,7 @@ impl LspAdapter for VtslsLspAdapter { Self::PACKAGE_NAME, &server_path, &container_dir, - &latest_version.server_version, - Default::default(), + VersionStrategy::Latest(&latest_version.server_version), ) .await { @@ -129,8 +128,7 @@ impl LspAdapter for VtslsLspAdapter { Self::TYPESCRIPT_PACKAGE_NAME, &container_dir.join(Self::TYPESCRIPT_TSDK_PATH), &container_dir, - &latest_version.typescript_version, - Default::default(), + VersionStrategy::Latest(&latest_version.typescript_version), ) .await { diff --git a/crates/languages/src/yaml.rs b/crates/languages/src/yaml.rs index 28be2cc1a4..15a4d590bc 100644 --- a/crates/languages/src/yaml.rs +++ b/crates/languages/src/yaml.rs @@ -6,7 +6,7 @@ use language::{ LanguageToolchainStore, LspAdapter, LspAdapterDelegate, language_settings::AllLanguageSettings, }; use lsp::{LanguageServerBinary, LanguageServerName}; -use node_runtime::NodeRuntime; +use node_runtime::{NodeRuntime, VersionStrategy}; use project::{Fs, lsp_store::language_server_settings}; use serde_json::Value; use settings::{Settings, SettingsLocation}; @@ -108,8 +108,7 @@ impl LspAdapter for YamlLspAdapter { Self::PACKAGE_NAME, &server_path, &container_dir, - &version, - Default::default(), + VersionStrategy::Latest(version), ) .await; diff --git a/crates/node_runtime/src/node_runtime.rs b/crates/node_runtime/src/node_runtime.rs index 6fcc3a728a..f92c122e71 100644 --- a/crates/node_runtime/src/node_runtime.rs +++ b/crates/node_runtime/src/node_runtime.rs @@ -29,13 +29,11 @@ pub struct NodeBinaryOptions { pub use_paths: Option<(PathBuf, PathBuf)>, } -#[derive(Default)] -pub enum VersionCheck { - /// Check whether the installed and requested version have a mismatch - VersionMismatch, - /// Only check whether the currently installed version is older than the newest one - #[default] - OlderVersion, +pub enum VersionStrategy<'a> { + /// Install if current version doesn't match pinned version + Pin(&'a str), + /// Install if current version is older than latest version + Latest(&'a str), } #[derive(Clone)] @@ -295,8 +293,7 @@ impl NodeRuntime { package_name: &str, local_executable_path: &Path, local_package_directory: &Path, - latest_version: &str, - version_check: VersionCheck, + version_strategy: VersionStrategy<'_>, ) -> bool { // In the case of the local system not having the package installed, // or in the instances where we fail to parse package.json data, @@ -317,13 +314,20 @@ impl NodeRuntime { let Some(installed_version) = Version::parse(&installed_version).log_err() else { return true; }; - let Some(latest_version) = Version::parse(latest_version).log_err() else { - return true; - }; - match version_check { - VersionCheck::VersionMismatch => installed_version != latest_version, - VersionCheck::OlderVersion => installed_version < latest_version, + match version_strategy { + VersionStrategy::Pin(pinned_version) => { + let Some(pinned_version) = Version::parse(pinned_version).log_err() else { + return true; + }; + installed_version != pinned_version + } + VersionStrategy::Latest(latest_version) => { + let Some(latest_version) = Version::parse(latest_version).log_err() else { + return true; + }; + installed_version < latest_version + } } } }