copilot: Fix Copilot fails to sign in on newer versions (#36195)
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::CheckStatus>(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.
This commit is contained in:
parent
1a169e0b16
commit
2acfa5e948
9 changed files with 83 additions and 84 deletions
|
@ -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::<SettingsStore>(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<Self>) {
|
||||
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::notification::DidChangeConfiguration>(
|
||||
&lsp::DidChangeConfigurationParams { settings },
|
||||
)
|
||||
.log_err();
|
||||
}
|
||||
}
|
||||
|
||||
#[cfg(any(test, feature = "test-support"))]
|
||||
pub fn fake(cx: &mut gpui::TestAppContext) -> (Entity<Self>, 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::CheckStatus>(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<Buffer>, cx: &App) -> Result<lsp::Url, ()> {
|
|||
}
|
||||
}
|
||||
|
||||
fn notify_did_change_config_to_server(
|
||||
server: &Arc<LanguageServer>,
|
||||
cx: &mut Context<Copilot>,
|
||||
) -> 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::notification::DidChangeConfiguration>(&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<dyn Fs>, 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<dyn Fs>, 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?;
|
||||
}
|
||||
|
||||
|
|
Loading…
Add table
Add a link
Reference in a new issue