feat: relax and improve validation for copilot_node_command and nodejs (#527)

Co-authored-by: Antoine Gaudreau Simard <[email protected]>

Author: timhugh

lua/copilot/client/init.lua

@@ -140,7 +140,11 @@ end
 function M.setup()
   logger.trace("setting up client")
   local node_command = config.copilot_node_command
-  lsp.setup(config.server, node_command)
+  if not lsp.setup(config.server, node_command) then
+    is_disabled = true
+    return
+  end
+
   M.config = require("copilot.client.config").prepare_client_config(config.server_opts_overrides, M)
 
   if not M.config then

lua/copilot/lsp/init.lua

@@ -54,20 +54,25 @@ end
 
 ---@param server_config ServerConfig
 ---@param copilot_node_command string
+---@return boolean
 function M.setup(server_config, copilot_node_command)
+  local result = true
+
   if not server_config then
     logger.error("server_config is required")
   end
 
   if server_config.type == "nodejs" then
-    M.nodejs.setup(copilot_node_command, server_config.custom_server_filepath)
+    result = M.nodejs.setup(copilot_node_command, server_config.custom_server_filepath)
   elseif server_config.type == "binary" then
     M.binary.setup(server_config.custom_server_filepath)
   else
     logger.error("invalid server_config.type")
+    result = false
   end
 
   M.config = server_config
+  return result
 end
 
 return M

lua/copilot/lsp/nodejs.lua

@@ -13,18 +13,24 @@ local M = {
 ---@return nil|string node_version_error
 function M.get_node_version()
   if not M.node_version then
-    local cmd = { M.node_command, "--version" }
-    local cmd_output_table = vim.fn.executable(M.node_command) == 1 and vim.fn.systemlist(cmd, nil, 0) or { "" }
-    local cmd_output = cmd_output_table[#cmd_output_table]
-    local cmd_exit_code = vim.v.shell_error
+    local version_cmd = vim.split(M.node_command, " ")
+    table.insert(version_cmd, "--version")
+
     local node_version_major = 0
     local node_version = ""
-
-    if cmd_output then
-      node_version = string.match(cmd_output, "^v(%S+)") or node_version
-      node_version_major = tonumber(string.match(node_version, "^(%d+)%.")) or node_version_major
-    else
-      cmd_output = "[no output]"
+    local cmd_exit_code = -1
+    local cmd_output = "[no output]"
+    local ok, process = pcall(vim.system, version_cmd)
+
+    if ok and process then
+      local result = process:wait()
+      cmd_output = result.stdout or cmd_output
+      cmd_exit_code = result.code
+
+      if cmd_output and cmd_output ~= "[no output]" then
+        node_version = string.match(cmd_output, "^v(%S+)") or node_version
+        node_version_major = tonumber(string.match(node_version, "^(%d+)%.")) or node_version_major
+      end
     end
 
     if node_version_major == 0 then
@@ -63,17 +69,6 @@ function M.validate_node_version()
   return true
 end
 
-function M.node_exists()
-  local node_exists = vim.fn.executable(M.node_command) == 1
-
-  if not node_exists then
-    logger.error("node.js is not installed or not in PATH")
-    return false
-  end
-
-  return true
-end
-
 ---@param server_path? string
 ---@return boolean
 function M.init_agent_path(server_path)
@@ -101,11 +96,10 @@ end
 
 ---@return table
 function M.get_execute_command()
-  return {
-    M.node_command,
-    M.server_path or M.get_server_path(),
-    "--stdio",
-  }
+  local cmd = vim.split(M.node_command, " ")
+  table.insert(cmd, M.server_path or M.get_server_path())
+  table.insert(cmd, "--stdio")
+  return cmd
 end
 
 ---@param node_command? string
@@ -114,7 +108,7 @@ end
 function M.setup(node_command, custom_server_path)
   M.node_command = node_command or "node"
 
-  if not M.node_exists() or not M.validate_node_version() or not M.init_agent_path(custom_server_path) then
+  if not M.validate_node_version() or not M.init_agent_path(custom_server_path) then
     return false
   end