From f4c083f1e59a40db68c6b9d0fdaa2a65599f21b8 Mon Sep 17 00:00:00 2001
From: Miles Johnson <milesj@users.noreply.github.com>
Date: Thu, 30 Nov 2023 16:02:15 -0800
Subject: [PATCH] fix: Use `proto run` when available for parent execution.
 (#313)

---
 crates/cli/src/commands/run.rs | 47 ++++++++++++++++++++++++----------
 1 file changed, 34 insertions(+), 13 deletions(-)

diff --git a/crates/cli/src/commands/run.rs b/crates/cli/src/commands/run.rs
index d413481af..ce7427124 100644
--- a/crates/cli/src/commands/run.rs
+++ b/crates/cli/src/commands/run.rs
@@ -122,7 +122,7 @@ fn create_command<I: IntoIterator<Item = A>, A: AsRef<OsStr>>(
     tool: &Tool,
     exe_config: &ExecutableConfig,
     args: I,
-) -> Command {
+) -> miette::Result<Command> {
     let exe_path = exe_config.exe_path.as_ref().unwrap();
     let args = args
         .into_iter()
@@ -130,18 +130,39 @@ fn create_command<I: IntoIterator<Item = A>, A: AsRef<OsStr>>(
         .collect::<Vec<_>>();
 
     let command = if let Some(parent_exe) = &exe_config.parent_exe_name {
-        // Force an .exe extension on Windows because Windows shims are very brittle.
+        #[allow(unused_mut)]
+        let mut parent_exe_path = parent_exe.to_owned();
+        let mut exe_args = vec![];
+
+        // Avoid using parent shims on Windows because Windows shims are very brittle.
         // For example, if we execute "npm serve", this becomes "node ~/npm.js serve",
-        // which results in "node" becoming "node.cmd" because of PATH resolution,
-        // and .cmd files do not handle signals/pipes correctly, especially when a child
-        // process. So forcing the parent to always use .exe seems like a good solution.
-        let parent_exe_path = if cfg!(windows) && !parent_exe.ends_with(".exe") {
-            format!("{parent_exe}.exe")
-        } else {
-            parent_exe.to_owned()
-        };
+        // which results in "node" becoming "node.cmd" because of `PATH` resolution,
+        // and .cmd files do not handle signals/pipes correctly.
+        #[cfg(windows)]
+        if !parent_exe.ends_with(".exe") {
+            use std::ffi::OsString;
+
+            let mut config = proto_core::ToolsConfig::load_upwards()?;
+            config.inherit_builtin_plugins();
+
+            // Attempt to use `proto run <tool>` first instead of a hard-coded .exe.
+            // This way we rely on proto's executable discovery functionality.
+            if config.plugins.contains_key(parent_exe)
+                && tool.proto.tools_dir.join(parent_exe).exists()
+            {
+                parent_exe_path = "proto.exe".to_owned();
+
+                exe_args.push(OsString::from("run"));
+                exe_args.push(OsString::from(parent_exe));
+                exe_args.push(OsString::from("--"));
+            }
+            // Otherwise use a hard-coded .exe and rely on OS path resolution.
+            else {
+                parent_exe_path = format!("{parent_exe}.exe");
+            }
+        }
 
-        let mut exe_args = vec![exe_path.as_os_str().to_os_string()];
+        exe_args.push(exe_path.as_os_str().to_os_string());
         exe_args.extend(args);
 
         debug!(bin = ?parent_exe_path, args = ?exe_args, "Running {}", tool.get_name());
@@ -154,7 +175,7 @@ fn create_command<I: IntoIterator<Item = A>, A: AsRef<OsStr>>(
     };
 
     // Convert std to tokio
-    Command::from(command)
+    Ok(Command::from(command))
 }
 
 #[system]
@@ -211,7 +232,7 @@ pub async fn run(args: ArgsRef<RunArgs>) -> SystemResult {
     })?;
 
     // Run the command
-    let status = create_command(&tool, &exe_config, &args.passthrough)
+    let status = create_command(&tool, &exe_config, &args.passthrough)?
         .env(
             format!("{}_VERSION", tool.get_env_var_prefix()),
             tool.get_resolved_version().to_string(),