supermaven_api: Ensure downloaded Supermaven binary has executable permissions set (#32576)

Closes #32068
Closes #15653

Not entirely sure that it fixes the latter issue, but I am fairly
certain given the comments in #32068 and the available logs in the
issue.

This PR fixes an issue where the Supermaven provider would not leave the
"Initializing" stage. This happened due to the downloaded binary missing
executable permissions. The change here ensures that freshly downloaded
binaries as well as existing binaries downloaded by Zed have executable
permissions set. I decided on also adding this for the latter since
existing downloads would continue to be broken and Supermaven does not
seem to change versions often given the logs provided by users.


While I was at it, I also added a `make_file_executable` to the util
crate mirroring the method of the `zed_extensions_api` and refactored
existing usages where possible to use that method instead. This makes
the code slightly more readable in my opinion, yet adds a method to
non-unix systems that practically does nothing. I can revert this should
that be preferred.


Release Notes:

- Fixed an issue where the Supermaven completion provider would not
leave the "Initializing" stage.

---------

Co-authored-by: Bennet Bo Fenner <bennetbo@gmx.de>

Cargo.lock

@@ -15479,6 +15479,7 @@ dependencies = [
  "serde",
  "serde_json",
  "smol",
+ "util",
  "workspace-hack",
 ]
 

crates/askpass/src/askpass.rs

@@ -13,11 +13,9 @@ use gpui::{AsyncApp, BackgroundExecutor, Task};
 #[cfg(unix)]
 use smol::fs;
 #[cfg(unix)]
-use smol::{fs::unix::PermissionsExt as _, net::unix::UnixListener};
+use smol::net::unix::UnixListener;
 #[cfg(unix)]
-use util::ResultExt as _;
-#[cfg(unix)]
-use util::get_shell_safe_zed_path;
+use util::{ResultExt as _, fs::make_file_executable, get_shell_safe_zed_path};
 
 #[derive(PartialEq, Eq)]
 pub enum AskPassResult {
@@ -122,7 +120,7 @@ impl AskPassSession {
             shebang = "#!/bin/sh",
         );
         fs::write(&askpass_script_path, askpass_script).await?;
-        fs::set_permissions(&askpass_script_path, std::fs::Permissions::from_mode(0o755)).await?;
+        make_file_executable(&askpass_script_path).await?;
 
         Ok(Self {
             script_path: askpass_script_path,

crates/extension_host/src/wasm_host/wit/since_v0_1_0.rs

@@ -16,8 +16,7 @@ use std::{
     path::{Path, PathBuf},
     sync::{Arc, OnceLock},
 };
-use util::archive::extract_zip;
-use util::maybe;
+use util::{archive::extract_zip, fs::make_file_executable, maybe};
 use wasmtime::component::{Linker, Resource};
 
 use super::latest;
@@ -558,22 +557,13 @@ impl ExtensionImports for WasmState {
     }
 
     async fn make_file_executable(&mut self, path: String) -> wasmtime::Result<Result<(), String>> {
-        #[allow(unused)]
         let path = self
             .host
             .writeable_path_from_extension(&self.manifest.id, Path::new(&path))?;
 
-        #[cfg(unix)]
-        {
-            use std::fs::{self, Permissions};
-            use std::os::unix::fs::PermissionsExt;
-
-            return fs::set_permissions(&path, Permissions::from_mode(0o755))
-                .with_context(|| format!("setting permissions for path {path:?}"))
-                .to_wasmtime_result();
-        }
-
-        #[cfg(not(unix))]
-        Ok(Ok(()))
+        make_file_executable(&path)
+            .await
+            .with_context(|| format!("setting permissions for path {path:?}"))
+            .to_wasmtime_result()
     }
 }

crates/extension_host/src/wasm_host/wit/since_v0_6_0.rs

@@ -30,7 +30,7 @@ use std::{
     sync::{Arc, OnceLock},
 };
 use task::{SpawnInTerminal, ZedDebugConfig};
-use util::{archive::extract_zip, maybe};
+use util::{archive::extract_zip, fs::make_file_executable, maybe};
 use wasmtime::component::{Linker, Resource};
 
 pub const MIN_VERSION: SemanticVersion = SemanticVersion::new(0, 6, 0);
@@ -1065,22 +1065,13 @@ impl ExtensionImports for WasmState {
     }
 
     async fn make_file_executable(&mut self, path: String) -> wasmtime::Result<Result<(), String>> {
-        #[allow(unused)]
         let path = self
             .host
             .writeable_path_from_extension(&self.manifest.id, Path::new(&path))?;
 
-        #[cfg(unix)]
-        {
-            use std::fs::{self, Permissions};
-            use std::os::unix::fs::PermissionsExt;
-
-            return fs::set_permissions(&path, Permissions::from_mode(0o755))
-                .with_context(|| format!("setting permissions for path {path:?}"))
-                .to_wasmtime_result();
-        }
-
-        #[cfg(not(unix))]
-        Ok(Ok(()))
+        make_file_executable(&path)
+            .await
+            .with_context(|| format!("setting permissions for path {path:?}"))
+            .to_wasmtime_result()
     }
 }

crates/language_extension/src/extension_lsp_adapter.rs

@@ -18,7 +18,7 @@ use language::{
 use lsp::{CodeActionKind, LanguageServerBinary, LanguageServerBinaryOptions, LanguageServerName};
 use serde::Serialize;
 use serde_json::Value;
-use util::{ResultExt, maybe};
+use util::{ResultExt, fs::make_file_executable, maybe};
 
 use crate::LanguageServerRegistryProxy;
 
@@ -146,14 +146,9 @@ impl LspAdapter for ExtensionLspAdapter {
             if ["toml", "zig"].contains(&self.extension.manifest().id.as_ref())
                 && path.starts_with(&self.extension.work_dir())
             {
-                #[cfg(not(windows))]
-                {
-                    use std::fs::{self, Permissions};
-                    use std::os::unix::fs::PermissionsExt;
-
-                    fs::set_permissions(&path, Permissions::from_mode(0o755))
-                        .context("failed to set file permissions")?;
-                }
+                make_file_executable(&path)
+                    .await
+                    .context("failed to set file permissions")?;
             }
 
             Ok(LanguageServerBinary {

crates/languages/src/rust.rs

@@ -25,7 +25,11 @@ use std::{
 use task::{TaskTemplate, TaskTemplates, TaskVariables, VariableName};
 use util::archive::extract_zip;
 use util::merge_json_value_into;
-use util::{ResultExt, fs::remove_matching, maybe};
+use util::{
+    ResultExt,
+    fs::{make_file_executable, remove_matching},
+    maybe,
+};
 
 use crate::language_settings::language_settings;
 
@@ -226,14 +230,7 @@ impl LspAdapter for RustLspAdapter {
             };
 
             // todo("windows")
-            #[cfg(not(windows))]
-            {
-                fs::set_permissions(
-                    &server_path,
-                    <fs::Permissions as fs::unix::PermissionsExt>::from_mode(0o755),
-                )
-                .await?;
-            }
+            make_file_executable(&server_path).await?;
         }
 
         Ok(LanguageServerBinary {

crates/supermaven_api/Cargo.toml

@@ -20,4 +20,5 @@ paths.workspace = true
 serde.workspace = true
 serde_json.workspace = true
 smol.workspace = true
+util.workspace = true
 workspace-hack.workspace = true

crates/supermaven_api/src/supermaven_api.rs

@@ -5,10 +5,11 @@ use http_client::{AsyncBody, HttpClient, Request as HttpRequest};
 use paths::supermaven_dir;
 use serde::{Deserialize, Serialize};
 use smol::fs::{self, File};
-use smol::stream::StreamExt;
 use std::path::{Path, PathBuf};
 use std::sync::Arc;
 
+use util::fs::{make_file_executable, remove_matching};
+
 #[derive(Serialize)]
 pub struct GetExternalUserRequest {
     pub id: String,
@@ -253,6 +254,11 @@ pub async fn get_supermaven_agent_path(client: Arc<dyn HttpClient>) -> Result<Pa
     let binary_path = version_path(download_info.version);
 
     if has_version(&binary_path).await {
+        // Due to an issue with the Supermaven binary not being made executable on
+        // earlier Zed versions and Supermaven releases not occurring that frequently,
+        // we ensure here that the found binary is actually executable.
+        make_file_executable(&binary_path).await?;
+
         return Ok(binary_path);
     }
 
@@ -271,13 +277,9 @@ pub async fn get_supermaven_agent_path(client: Arc<dyn HttpClient>) -> Result<Pa
         .await
         .with_context(|| format!("Unable to write binary to file at {:?}", binary_path))?;
 
-    let mut old_binary_paths = fs::read_dir(supermaven_dir()).await?;
-    while let Some(old_binary_path) = old_binary_paths.next().await {
-        let old_binary_path = old_binary_path?;
-        if old_binary_path.path() != binary_path {
-            fs::remove_file(old_binary_path.path()).await?;
-        }
-    }
+    make_file_executable(&binary_path).await?;
+
+    remove_matching(supermaven_dir(), |file| file != binary_path).await;
 
     Ok(binary_path)
 }

crates/util/src/fs.rs

@@ -91,3 +91,22 @@ pub async fn move_folder_files_to_folder<P: AsRef<Path>>(
 
     Ok(())
 }
+
+#[cfg(unix)]
+/// Set the permissions for the given path so that the file becomes executable.
+/// This is a noop for non-unix platforms.
+pub async fn make_file_executable(path: &PathBuf) -> std::io::Result<()> {
+    fs::set_permissions(
+        &path,
+        <fs::Permissions as fs::unix::PermissionsExt>::from_mode(0o755),
+    )
+    .await
+}
+
+#[cfg(not(unix))]
+#[allow(clippy::unused_async)]
+/// Set the permissions for the given path so that the file becomes executable.
+/// This is a noop for non-unix platforms.
+pub async fn make_file_executable(_path: &PathBuf) -> std::io::Result<()> {
+    Ok(())
+}