fix(doctor): detect preexec plugin using env ATUIN_PREEXEC_BACKEND (#1856)

* refactor(doctor): update func names and desc to match current impl

* fix(doctor): use environment variable to detect plugin if possible

In this patch, if the plugin provides an environment variable, we use
the environment variable to test the existence of the plugin.  When an
environment variable is not available, we continue to use the mock
interactive session by "shell -ic command".  We also test
shell-specific plugins only in the corresponding shells.  An
additional test can be performed by a custom function for each plugin.

2 files changed, 101 insertions, 23 deletions

diff --git a/atuin/src/command/client/doctor.rs b/atuin/src/command/client/doctor.rs
index ef399e7d..5c144fed 100644
--- a/atuin/src/command/client/doctor.rs
+++ b/atuin/src/command/client/doctor.rs
@@ -1,5 +1,5 @@
 use std::process::Command;
-use std::{collections::HashMap, path::PathBuf};
+use std::{env, path::PathBuf, str::FromStr};
 
 use atuin_client::settings::Settings;
 use colored::Colorize;
@@ -19,12 +19,12 @@ struct ShellInfo {
 
 impl ShellInfo {
     // HACK ALERT!
-    // Many of the env vars we need to detect are not exported :(
-    // So, we're going to run `env` in a subshell and parse the output
-    // There's a chance this won't work, so it should not be fatal.
+    // Many of the shell vars we need to detect are not exported :(
+    // So, we're going to run a interactive session and directly check the
+    // variable.  There's a chance this won't work, so it should not be fatal.
     //
-    // Every shell we support handles `shell -c 'command'`
-    fn env_exists(shell: &str, var: &str) -> bool {
+    // Every shell we support handles `shell -ic 'command'`
+    fn shellvar_exists(shell: &str, var: &str) -> bool {
         let cmd = Command::new(shell)
             .args([
                 "-ic",
@@ -39,23 +39,96 @@ impl ShellInfo {
         cmd.contains("ATUIN_DOCTOR_ENV_FOUND")
     }
 
-    pub fn plugins(shell: &str) -> Vec<String> {
+    fn validate_plugin_blesh(
+        _shell: &str,
+        shell_process: &sysinfo::Process,
+        ble_session_id: &str,
+    ) -> Option<String> {
+        ble_session_id
+            .split('/')
+            .nth(1)
+            .and_then(|field| u32::from_str(field).ok())
+            .filter(|&blesh_pid| blesh_pid == shell_process.pid().as_u32())
+            .map(|_| "blesh".to_string())
+    }
+
+    pub fn plugins(shell: &str, shell_process: &sysinfo::Process) -> Vec<String> {
         // consider a different detection approach if there are plugins
-        // that don't set env vars
-
-        let map = HashMap::from([
-            ("ATUIN_SESSION", "atuin"),
-            ("BLE_ATTACHED", "blesh"),
-            ("bash_preexec_imported", "bash-preexec"),
-        ]);
-
-        map.into_iter()
-            .filter_map(|(env, plugin)| {
-                if ShellInfo::env_exists(shell, env) {
-                    return Some(plugin.to_string());
-                }
+        // that don't set shell vars
+
+        enum PluginShellType {
+            Any,
+            Bash,
+
+            // Note: these are currently unused
+            #[allow(dead_code)]
+            Zsh,
+            #[allow(dead_code)]
+            Fish,
+            #[allow(dead_code)]
+            Nushell,
+            #[allow(dead_code)]
+            Xonsh,
+        }
 
-                None
+        enum PluginProbeType {
+            EnvironmentVariable(&'static str),
+            InteractiveShellVariable(&'static str),
+        }
+
+        type PluginValidator = fn(&str, &sysinfo::Process, &str) -> Option<String>;
+
+        let plugin_list: [(
+            &str,
+            PluginShellType,
+            PluginProbeType,
+            Option<PluginValidator>,
+        ); 3] = [
+            (
+                "atuin",
+                PluginShellType::Any,
+                PluginProbeType::EnvironmentVariable("ATUIN_SESSION"),
+                None,
+            ),
+            (
+                "blesh",
+                PluginShellType::Bash,
+                PluginProbeType::EnvironmentVariable("BLE_SESSION_ID"),
+                Some(Self::validate_plugin_blesh),
+            ),
+            (
+                "bash-preexec",
+                PluginShellType::Bash,
+                PluginProbeType::InteractiveShellVariable("bash_preexec_imported"),
+                None,
+            ),
+        ];
+
+        plugin_list
+            .into_iter()
+            .filter(|(_, shell_type, _, _)| match shell_type {
+                PluginShellType::Any => true,
+                PluginShellType::Bash => shell.starts_with("bash") || shell == "sh",
+                PluginShellType::Zsh => shell.starts_with("zsh"),
+                PluginShellType::Fish => shell.starts_with("fish"),
+                PluginShellType::Nushell => shell.starts_with("nu"),
+                PluginShellType::Xonsh => shell.starts_with("xonsh"),
+            })
+            .filter_map(|(plugin, _, probe_type, validator)| -> Option<String> {
+                match probe_type {
+                    PluginProbeType::EnvironmentVariable(env) => {
+                        env::var(env).ok().filter(|value| !value.is_empty())
+                    }
+                    PluginProbeType::InteractiveShellVariable(shellvar) => {
+                        ShellInfo::shellvar_exists(shell, shellvar).then_some(String::default())
+                    }
+                }
+                .and_then(|value| {
+                    validator.map_or_else(
+                        || Some(plugin.to_string()),
+                        |validator| validator(shell, shell_process, &value),
+                    )
+                })
             })
             .collect()
     }
@@ -75,7 +148,7 @@ impl ShellInfo {
         let shell = shell.strip_prefix('-').unwrap_or(&shell);
         let name = shell.to_string();
 
-        let plugins = ShellInfo::plugins(name.as_str());
+        let plugins = ShellInfo::plugins(name.as_str(), parent);
 
         Self { name, plugins }
     }
@@ -188,7 +261,7 @@ fn checks(info: &DoctorDump) {
     println!(); // spacing
                 //
     let zfs_error = "[Filesystem] ZFS is known to have some issues with SQLite. Atuin uses SQLite heavily. If you are having poor performance, there are some workarounds here: https://github.com/atuinsh/atuin/issues/952".bold().red();
-    let bash_plugin_error = "[Shell] If you are using Bash, Atuin requires that either bash-preexec or ble.sh be installed. We cannot currently detect ble, so if you have it setup then ignore this! Read more here: https://docs.atuin.sh/guide/installation/#bash".bold().red();
+    let bash_plugin_error = "[Shell] If you are using Bash, Atuin requires that either bash-preexec or ble.sh be installed. An older ble.sh may not be detected. so ignore this if you have it set up! Read more here: https://docs.atuin.sh/guide/installation/#bash".bold().red();
 
     // ZFS: https://github.com/atuinsh/atuin/issues/952
     if info.system.disks.iter().any(|d| d.filesystem == "zfs") {
diff --git a/atuin/src/shell/atuin.bash b/atuin/src/shell/atuin.bash
index 38f58c59..ba4f2cc2 100644
--- a/atuin/src/shell/atuin.bash
+++ b/atuin/src/shell/atuin.bash
@@ -271,6 +271,11 @@ if [[ ${BLE_VERSION-} ]] && ((_ble_version >= 400)); then
     }
     ble/util/import/eval-after-load core-complete '
         ble/array#unshift _ble_complete_auto_source atuin-history'
+
+    # @env BLE_SESSION_ID: `atuin doctor` references the environment variable
+    # BLE_SESSION_ID.  We explicitly export the variable because it was not
+    # exported in older versions of ble.sh.
+    [[ ${BLE_SESSION_ID-} ]] && export BLE_SESSION_ID
 fi
 precmd_functions+=(__atuin_precmd)
 preexec_functions+=(__atuin_preexec)