[Bug] commit 25ab971 has introduced bug

[yingmanwumen]

This commit works fine with me:

commit 330ffb9630509cd0cd56815c6e667bc497ec82f4 (HEAD)
Author: Antoine Gaudreau Simard <antoineg.simard@gmail.com>
Date:   Thu Mar 20 20:21:50 2025 -0400

    Warning cleanup (#381)
    
    * refactor: resolve most Lua LSP warnings, leaving the unpack one for now as it seems to break the auth popup

While this introduced bug

commit 25ab971cb0badf3185d29f68a360c37b48d4e783 (HEAD)
Author: Antoine Gaudreau Simard <antoineg.simard@gmail.com>
Date:   Thu Mar 20 21:32:09 2025 -0400

    feat: add support for dynamic root_dir resolving  (#382)
    
    the configuration supports overriding the default function as root_dir, with a fallback to the current directory.
    fixes #371

Error message:

Error executing vim.schedule lua callback: ...local/share/nvim/lazy/copilot.lua/lua/copilot/client.lua:24: unexpectedly started multiple copilot servers
stack traceback:
	[C]: in function 'error'
	...local/share/nvim/lazy/copilot.lua/lua/copilot/client.lua:24: in function 'store_client_id'
	...local/share/nvim/lazy/copilot.lua/lua/copilot/client.lua:118: in function 'buf_attach'
	...local/share/nvim/lazy/copilot.lua/lua/copilot/client.lua:321: in function ''
	vim/_editor.lua: in function <vim/_editor.lua:0>

If I update to the latest commit(master branch), it comes with:

My config:

{
  "zbirenbaum/copilot.lua",
  event = "InsertEnter",
  cmd = "Copilot",
  config = function()
    require("copilot").setup({
      suggestion = { enabled = false },
      panel = { enabled = false },
    })
  end,
  -- enabled = false,
}

[yingmanwumen]

How it comes:

1. The original implementation used cwd as root_dir and it won't change during the lifetime of this plugin
2. The latest implementation recalculate root_dir every time invoking buf_attach
3. LSP Client use root_dir to decide if it should create a new lsp server instance (I guesss so)
4. The buf_attach is binded to event Filetype
5. The root_dir may be different during two buf_attach invocations
6. Due to it, error occurs

[yingmanwumen]

A recommanded fix may be:

M.clients = {}  -- root_dir -> client_id

function M.get()
  local root_dir = config.get_root_dir()
  local client_id = M.clients[root_dir]
  return client_id and vim.lsp.get_client_by_id(client_id)
end

local function store_client_id(root_dir, id)
  if M.clients[root_dir] and M.clients[root_dir] ~= id then
    if vim.lsp.get_client_by_id(M.clients[root_dir]) then
      error("unexpectedly started multiple copilot servers")
    end
  end
  M.clients[root_dir] = id
end

And all M.id should be replaced by M.clients, which would be a huge refactor...

I'd like to know your opinion...

[yingmanwumen]

Or we may keep one instance only. Everytime when there is a new instance to spawn, we may stop the old instance at first. This is much simpler than refactoring M.id

[AntoineGS]

Thanks I will look into it shortly 🙂🙂

[AntoineGS]

Fixed by #388, thanks for the pointer!