Skip to content

Commit 8e2a918

Browse files
committed
fix: safeguards against orphaned copilot servers
Fixes zbirenbaum#667
1 parent 11f6d96 commit 8e2a918

File tree

3 files changed

+188
-0
lines changed

3 files changed

+188
-0
lines changed

lua/copilot/client/init.lua

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,13 +14,15 @@ local is_disabled = false
1414
---@field config vim.lsp.ClientConfig | nil
1515
---@field startup_error string | nil
1616
---@field initialized boolean
17+
---@field client_starting boolean
1718
local M = {
1819
augroup = nil,
1920
id = nil,
2021
capabilities = nil,
2122
config = nil,
2223
startup_error = nil,
2324
initialized = false,
25+
client_starting = false,
2426
}
2527

2628
---@param id integer
@@ -141,6 +143,11 @@ function M.ensure_client_started()
141143
return
142144
end
143145

146+
if M.client_starting then
147+
logger.trace("client startup already in progress, skipping duplicate")
148+
return
149+
end
150+
144151
if is_disabled then
145152
logger.debug("copilot is offline")
146153
return
@@ -155,9 +162,13 @@ function M.ensure_client_started()
155162
return
156163
end
157164

165+
M.client_starting = true
166+
158167
M.config.root_dir = utils.get_root_dir(config.root_dir)
159168
local client_id, err = vim.lsp.start(M.config, { attach = false })
160169

170+
M.client_starting = false
171+
161172
if not client_id then
162173
logger.error(string.format("error starting LSP client: %s", err))
163174
return
@@ -208,6 +219,11 @@ function M.setup()
208219
return
209220
end
210221

222+
-- Stop all existing copilot clients to prevent orphan processes
223+
for _, existing_client in ipairs(vim.lsp.get_clients({ name = "copilot" })) do
224+
existing_client:stop(true)
225+
end
226+
211227
is_disabled = false
212228
M.id = nil
213229

@@ -225,6 +241,17 @@ function M.setup()
225241
desc = "[copilot] (client) buf entered",
226242
})
227243

244+
vim.api.nvim_create_autocmd("VimLeavePre", {
245+
group = M.augroup,
246+
callback = function()
247+
local client = vim.lsp.get_client_by_id(M.id)
248+
if client then
249+
client:stop()
250+
end
251+
end,
252+
desc = "[copilot] (client) stop LSP client on exit",
253+
})
254+
228255
vim.api.nvim_create_autocmd("BufFilePost", {
229256
group = M.augroup,
230257
callback = function(args)

tests/files/saved_new.txt

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
123
2+
456
3+
7

tests/test_client_lifecycle.lua

Lines changed: 158 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,158 @@
1+
local child_helper = require("tests.child_helper")
2+
local child = child_helper.new_child_neovim("test_client_lifecycle")
3+
local u = require("tests.utils")
4+
local eq = MiniTest.expect.equality
5+
6+
local T = MiniTest.new_set({
7+
hooks = {
8+
pre_once = function() end,
9+
pre_case = function()
10+
child.run_pre_case(true)
11+
child.lua("c = require('copilot.client')")
12+
end,
13+
post_once = child.stop,
14+
},
15+
})
16+
17+
T["client lifecycle()"] = MiniTest.new_set()
18+
19+
-- Fix #1: ensure_client_started should have a startup guard to prevent duplicate spawns
20+
T["client lifecycle()"]["ensure_client_started sets starting guard during startup"] = function()
21+
child.configure_copilot()
22+
23+
-- After initialization, client_starting should be false (startup completed)
24+
local client_starting = child.lua("return c.client_starting")
25+
eq(client_starting, false)
26+
end
27+
28+
T["client lifecycle()"]["ensure_client_started with starting guard prevents duplicate calls"] = function()
29+
child.configure_copilot()
30+
31+
-- Simulate the guard being set (as if startup is in progress)
32+
child.lua("c.client_starting = true")
33+
34+
-- Store original client id
35+
local original_id = child.lua("return c.id")
36+
37+
-- Try to start another client - should be blocked by the guard
38+
child.lua([[
39+
c.id = nil
40+
c.ensure_client_started()
41+
]])
42+
43+
-- id should still be nil because the guard prevented a new start
44+
local new_id = child.lua("return c.id")
45+
eq(new_id, vim.NIL)
46+
end
47+
48+
-- Fix #2: setup() should stop existing client before resetting M.id
49+
T["client lifecycle()"]["setup stops existing client before resetting id"] = function()
50+
child.configure_copilot()
51+
52+
-- Verify client is running
53+
local id_before = child.lua("return c.id")
54+
assert(id_before ~= vim.NIL, "client should be running")
55+
56+
-- Track whether the old client was stopped
57+
child.lua([[
58+
_G.old_client_stopped = false
59+
local old_client = vim.lsp.get_client_by_id(c.id)
60+
if old_client then
61+
local original_stop = old_client.stop
62+
old_client.stop = function(self, ...)
63+
_G.old_client_stopped = true
64+
return original_stop(self, ...)
65+
end
66+
end
67+
]])
68+
69+
-- Call setup again (simulates :Copilot disable then :Copilot enable)
70+
child.lua("c.setup()")
71+
child.lua("vim.wait(500, function() return false end, 10)")
72+
73+
-- The old client should have been stopped
74+
local was_stopped = child.lua("return _G.old_client_stopped")
75+
eq(was_stopped, true)
76+
end
77+
78+
-- Fix #3: VimLeavePre should clean up the LSP client
79+
T["client lifecycle()"]["setup registers VimLeavePre autocmd"] = function()
80+
child.configure_copilot()
81+
82+
local has_autocmd = child.lua([[
83+
local autocmds = vim.api.nvim_get_autocmds({
84+
group = "copilot.client",
85+
event = "VimLeavePre",
86+
})
87+
return #autocmds > 0
88+
]])
89+
90+
eq(has_autocmd, true)
91+
end
92+
93+
T["client lifecycle()"]["VimLeavePre stops the LSP client"] = function()
94+
child.configure_copilot()
95+
96+
-- Verify client is running
97+
local id = child.lua("return c.id")
98+
assert(id ~= vim.NIL, "client should be running")
99+
100+
-- Track whether stop was called
101+
child.lua([[
102+
_G.client_stopped_on_leave = false
103+
local client_obj = vim.lsp.get_client_by_id(c.id)
104+
if client_obj then
105+
local original_stop = client_obj.stop
106+
client_obj.stop = function(self, ...)
107+
_G.client_stopped_on_leave = true
108+
return original_stop(self, ...)
109+
end
110+
end
111+
]])
112+
113+
-- Trigger VimLeavePre
114+
child.lua([[
115+
vim.api.nvim_exec_autocmds("VimLeavePre", { group = "copilot.client" })
116+
vim.wait(200, function() return false end, 10)
117+
]])
118+
119+
local was_stopped = child.lua("return _G.client_stopped_on_leave")
120+
eq(was_stopped, true)
121+
end
122+
123+
-- Regression: disable/enable cycle should not leak processes
124+
T["client lifecycle()"]["disable then enable does not leak client processes"] = function()
125+
child.configure_copilot()
126+
127+
local id_before = child.lua("return c.id")
128+
assert(id_before ~= vim.NIL, "client should be running")
129+
130+
-- Disable and re-enable
131+
child.lua([[
132+
require("copilot.command").disable()
133+
vim.wait(200, function() return false end, 10)
134+
require("copilot.command").enable()
135+
]])
136+
137+
-- Wait for the new client to initialize
138+
child.lua([[
139+
vim.wait(2000, function()
140+
return require("copilot.client").initialized
141+
end, 10)
142+
]])
143+
144+
-- There should be exactly 1 active (non-stopped) copilot LSP client
145+
local client_count = child.lua([[
146+
local count = 0
147+
for _, cl in ipairs(vim.lsp.get_clients({ name = "copilot" })) do
148+
if not cl:is_stopped() then
149+
count = count + 1
150+
end
151+
end
152+
return count
153+
]])
154+
155+
eq(client_count, 1)
156+
end
157+
158+
return T

0 commit comments

Comments
 (0)