From 80d02d28df8d1ef0499eea2ca69b896fae4dea62 Mon Sep 17 00:00:00 2001 From: jinzhongjia Date: Fri, 2 Jan 2026 19:50:59 +0800 Subject: [PATCH 01/13] feat(server): share process across multiple instances Add a lock file mechanism to coordinate a single server process among multiple Neovim clients. Track ownership and active clients in opencode-server.lock to ensure the process persists until the last instance closes. Additionally, implement URI encoding for API queries. --- lua/opencode/api_client.lua | 28 +-- lua/opencode/opencode_server.lua | 312 +++++++++++++++++++++++++++- lua/opencode/server_job.lua | 6 + tests/unit/api_client_spec.lua | 15 +- tests/unit/opencode_server_spec.lua | 272 ++++++++++++++++++++++++ 5 files changed, 605 insertions(+), 28 deletions(-) diff --git a/lua/opencode/api_client.lua b/lua/opencode/api_client.lua index 4bc05083..3bb04c5a 100644 --- a/lua/opencode/api_client.lua +++ b/lua/opencode/api_client.lua @@ -58,18 +58,20 @@ function OpencodeApiClient:_call(endpoint, method, body, query) end local url = self.base_url .. endpoint - if query then - local params = {} + query = query or {} + if query.directory == nil then + query.directory = vim.fn.getcwd() + end - for k, v in pairs(query) do - if v ~= nil then - table.insert(params, k .. '=' .. tostring(v)) - end + local params = {} + for k, v in pairs(query) do + if v ~= nil then + table.insert(params, k .. '=' .. vim.uri_encode(tostring(v))) end + end - if #params > 0 then - url = url .. '?' .. table.concat(params, '&') - end + if #params > 0 then + url = url .. '?' .. table.concat(params, '&') end return server_job.call_api(url, method, body) @@ -394,13 +396,11 @@ end --- Subscribe to events (streaming) --- @param directory string|nil Directory path --- @param on_event fun(event: table) Event callback ---- @return table The streaming job handle +--- @return table streaming job handle function OpencodeApiClient:subscribe_to_events(directory, on_event) self:_ensure_base_url() - local url = self.base_url .. '/event' - if directory then - url = url .. '?directory=' .. directory - end + directory = directory or vim.fn.getcwd() + local url = self.base_url .. '/event?directory=' .. vim.uri_encode(directory) return server_job.stream_api(url, 'GET', nil, function(chunk) -- strip data: prefix if present diff --git a/lua/opencode/opencode_server.lua b/lua/opencode/opencode_server.lua index 99384a7d..93ed10b1 100644 --- a/lua/opencode/opencode_server.lua +++ b/lua/opencode/opencode_server.lua @@ -3,18 +3,235 @@ local safe_call = util.safe_call local Promise = require('opencode.promise') --- @class OpencodeServer ---- @field job any The vim.system job handle +--- @field job any|nil The vim.system job handle (nil if using shared server) --- @field url string|nil The server URL once ready --- @field handle any Compatibility property for job.stop interface --- @field spawn_promise Promise --- @field shutdown_promise Promise +--- @field is_owner boolean Whether this instance owns the server process +--- @field pid number This instance's PID local OpencodeServer = {} OpencodeServer.__index = OpencodeServer ---- Create a new ServerJob instance +local current_pid = vim.fn.getpid() + +local FLOCK_TIMEOUT_MS = 5000 +local FLOCK_RETRY_INTERVAL_MS = 50 + +--- @return string +local function get_lock_file_path() + local tmpdir = vim.fn.stdpath('cache') + return vim.fs.joinpath(tmpdir --[[@as string]], 'opencode-server.lock') +end + +--- @return string +local function get_flock_path() + return get_lock_file_path() .. '.flock' +end + +--- @return boolean acquired +local function acquire_file_lock() + local flock_path = get_flock_path() + local start_time = vim.uv.now() + + while (vim.uv.now() - start_time) < FLOCK_TIMEOUT_MS do + local fd = vim.uv.fs_open(flock_path, 'wx', 384) + if fd then + vim.uv.fs_write(fd, tostring(current_pid)) + vim.uv.fs_close(fd) + return true + end + + local stat = vim.uv.fs_stat(flock_path) + if stat then + local age_ms = (vim.uv.now() - stat.mtime.sec * 1000) + if age_ms > FLOCK_TIMEOUT_MS then + os.remove(flock_path) + end + end + + vim.wait(FLOCK_RETRY_INTERVAL_MS, function() + return false + end) + end + + return false +end + +local function release_file_lock() + local flock_path = get_flock_path() + os.remove(flock_path) +end + +--- @param fn function +--- @return any +local function with_file_lock(fn) + if not acquire_file_lock() then + return nil + end + local ok, result = pcall(fn) + release_file_lock() + if not ok then + error(result) + end + return result +end + +--- @param pid number +--- @return boolean +local function is_pid_alive(pid) + if not pid or pid <= 0 then + return false + end + local ok, result = pcall(vim.uv.kill, pid, 0) + return ok and result == 0 +end + +--- @class LockFileData +--- @field url string +--- @field owner number +--- @field clients number[] +--- @return LockFileData|nil +local function read_lock_file() + local lock_path = get_lock_file_path() + local f = io.open(lock_path, 'r') + if not f then + return nil + end + + local content = f:read('*a') + f:close() + + if not content or content == '' then + return nil + end + + local url = content:match('url=([^\n]+)') + local owner_str = content:match('owner=([^\n]+)') + local clients_str = content:match('clients=([^\n]*)') + + if not url then + return nil + end + + local owner = tonumber(owner_str) or 0 + local clients = {} + + if clients_str and clients_str ~= '' then + for pid_str in clients_str:gmatch('([^,]+)') do + local pid = tonumber(pid_str) + if pid then + table.insert(clients, pid) + end + end + end + + return { url = url, owner = owner, clients = clients } +end + +--- @param data LockFileData +local function write_lock_file(data) + local lock_path = get_lock_file_path() + local f = io.open(lock_path, 'w') + if not f then + return + end + + local clients_str = table.concat( + vim.tbl_map(function(pid) + return tostring(pid) + end, data.clients), + ',' + ) + + f:write(string.format('url=%s\nowner=%d\nclients=%s\n', data.url, data.owner, clients_str)) + f:close() +end + +local function remove_lock_file() + local lock_path = get_lock_file_path() + os.remove(lock_path) +end + +--- @param data LockFileData +--- @return LockFileData +local function cleanup_dead_pids(data) + local alive_clients = vim.tbl_filter(function(pid) + return is_pid_alive(pid) + end, data.clients) + + data.clients = alive_clients + + if not is_pid_alive(data.owner) and #alive_clients > 0 then + data.owner = alive_clients[1] + end + + return data +end + +--- @param url string +local function register_as_owner(url) + with_file_lock(function() + write_lock_file({ + url = url, + owner = current_pid, + clients = { current_pid }, + }) + end) +end + +--- @return boolean success +local function register_client() + local result = with_file_lock(function() + local data = read_lock_file() + if not data then + return false + end + + data = cleanup_dead_pids(data) + + local already_registered = vim.tbl_contains(data.clients, current_pid) + if not already_registered then + table.insert(data.clients, current_pid) + end + + write_lock_file(data) + return true + end) + return result or false +end + +--- @return boolean should_kill_server +local function unregister_client() + local result = with_file_lock(function() + local data = read_lock_file() + if not data then + return false + end + + data = cleanup_dead_pids(data) + + data.clients = vim.tbl_filter(function(pid) + return pid ~= current_pid + end, data.clients) + + if #data.clients == 0 then + remove_lock_file() + return true + end + + if data.owner == current_pid then + data.owner = data.clients[1] + end + + write_lock_file(data) + return false + end) + return result or false +end + --- @return OpencodeServer function OpencodeServer.new() - --- before quitting vim ensure we close opencode server vim.api.nvim_create_autocmd('VimLeavePre', { group = vim.api.nvim_create_augroup('OpencodeVimLeavePre', { clear = true }), callback = function() @@ -28,27 +245,100 @@ function OpencodeServer.new() job = nil, url = nil, handle = nil, + is_owner = false, + pid = current_pid, spawn_promise = Promise.new(), shutdown_promise = Promise.new(), }, OpencodeServer) end +--- @return string|nil url Returns the server URL if an existing server is available +function OpencodeServer.try_existing_server() + local server_url = with_file_lock(function() + local data = read_lock_file() + if not data then + return nil + end + + data = cleanup_dead_pids(data) + + if #data.clients == 0 then + remove_lock_file() + return nil + end + + write_lock_file(data) + return data.url + end) + + if not server_url then + return nil + end + + local curl = require('opencode.curl') + local result_received = false + local is_healthy = false + + curl.request({ + url = server_url .. '/global/health', + method = 'GET', + timeout = 2000, + callback = function(response) + result_received = true + is_healthy = response and response.status >= 200 and response.status < 300 + end, + on_error = function() + result_received = true + is_healthy = false + end, + }) + + vim.wait(3000, function() + return result_received + end, 50) + + if is_healthy then + return server_url + end + + with_file_lock(function() + remove_lock_file() + end) + return nil +end + +--- @param url string +--- @return OpencodeServer +function OpencodeServer.from_existing(url) + local server = OpencodeServer.new() + server.url = url + server.is_owner = false + register_client() + server.spawn_promise:resolve(server --[[@as any]]) + return server +end + function OpencodeServer:is_running() - return self.job and self.job.pid ~= nil + if self.is_owner then + return self.job and self.job.pid ~= nil + end + return self.url ~= nil end ---- Clean up this server job --- @return Promise function OpencodeServer:shutdown() - if self.job and self.job.pid then + local should_kill = unregister_client() + + if should_kill and self.job and self.job.pid then pcall(function() self.job:kill('sigterm') end) end + self.job = nil self.url = nil self.handle = nil - self.shutdown_promise:resolve(true) + self.shutdown_promise:resolve(true --[[@as boolean]]) return self.shutdown_promise end @@ -58,12 +348,13 @@ end --- @field on_error fun(err: any) --- @field on_exit fun(exit_opts: vim.SystemCompleted ) ---- Spawn the opencode server for this ServerJob instance. --- @param opts? OpencodeServerSpawnOpts --- @return Promise function OpencodeServer:spawn(opts) opts = opts or {} + self.is_owner = true + self.job = vim.system({ 'opencode', 'serve', @@ -78,7 +369,8 @@ function OpencodeServer:spawn(opts) local url = data:match('opencode server listening on ([^%s]+)') if url then self.url = url - self.spawn_promise:resolve(self) + register_as_owner(url) + self.spawn_promise:resolve(self --[[@as any]]) safe_call(opts.on_ready, self.job, url) end end @@ -94,7 +386,7 @@ function OpencodeServer:spawn(opts) self.url = nil self.handle = nil safe_call(opts.on_exit, exit_opts) - self.shutdown_promise:resolve(true) + self.shutdown_promise:resolve(true --[[@as boolean]]) end) self.handle = self.job and self.job.pid diff --git a/lua/opencode/server_job.lua b/lua/opencode/server_job.lua index 789ab19b..124e05e5 100644 --- a/lua/opencode/server_job.lua +++ b/lua/opencode/server_job.lua @@ -139,6 +139,12 @@ function M.ensure_server() return promise:resolve(state.opencode_server) end + local existing_url = opencode_server.try_existing_server() + if existing_url then + state.opencode_server = opencode_server.from_existing(existing_url) + return promise:resolve(state.opencode_server) + end + state.opencode_server = opencode_server.new() state.opencode_server:spawn({ diff --git a/tests/unit/api_client_spec.lua b/tests/unit/api_client_spec.lua index ce7445d1..c04f4ed1 100644 --- a/tests/unit/api_client_spec.lua +++ b/tests/unit/api_client_spec.lua @@ -61,8 +61,14 @@ describe('api_client', function() it('should construct URLs correctly with query parameters', function() local server_job = require('opencode.server_job') local original_call_api = server_job.call_api + local original_getcwd = vim.fn.getcwd local captured_calls = {} + -- Mock vim.fn.getcwd to return predictable value + vim.fn.getcwd = function() + return '/mock/cwd' + end + server_job.call_api = function(url, method, body) table.insert(captured_calls, { url = url, method = method, body = body }) local promise = require('opencode.promise').new() @@ -72,12 +78,12 @@ describe('api_client', function() local client = api_client.new('http://localhost:8080') - -- Test without query params + -- Test without query params - should auto-add directory from cwd client:list_projects() - assert.are.equal('http://localhost:8080/project', captured_calls[1].url) + assert.are.equal('http://localhost:8080/project?directory=/mock/cwd', captured_calls[1].url) assert.are.equal('GET', captured_calls[1].method) - -- Test with query params + -- Test with explicit directory - should use provided value client:list_projects('/some/directory') assert.are.equal('http://localhost:8080/project?directory=/some/directory', captured_calls[2].url) @@ -93,7 +99,8 @@ describe('api_client', function() assert.is_not_nil(actual_url:find('model=claude%-3')) -- Escape the dash assert.is_not_nil(actual_url:find('directory=/some/dir')) - -- Restore original function + -- Restore original functions server_job.call_api = original_call_api + vim.fn.getcwd = original_getcwd end) end) diff --git a/tests/unit/opencode_server_spec.lua b/tests/unit/opencode_server_spec.lua index 566855d7..0f751a9c 100644 --- a/tests/unit/opencode_server_spec.lua +++ b/tests/unit/opencode_server_spec.lua @@ -1,13 +1,74 @@ local OpencodeServer = require('opencode.opencode_server') local assert = require('luassert') +--- Helper to get lock file path (mirrors the module's internal function) +local function get_lock_file_path() + local tmpdir = vim.fn.stdpath('cache') + return vim.fs.joinpath(tmpdir --[[@as string]], 'opencode-server.lock') +end + +--- Helper to write a lock file directly for testing +local function write_test_lock_file(url, owner, clients) + local lock_path = get_lock_file_path() + local f = io.open(lock_path, 'w') + if f then + local clients_str = table.concat( + vim.tbl_map(function(pid) + return tostring(pid) + end, clients), + ',' + ) + f:write(string.format('url=%s\nowner=%d\nclients=%s\n', url, owner, clients_str)) + f:close() + end +end + +--- Helper to read lock file directly for testing +local function read_test_lock_file() + local lock_path = get_lock_file_path() + local f = io.open(lock_path, 'r') + if not f then + return nil + end + local content = f:read('*a') + f:close() + if not content or content == '' then + return nil + end + local url = content:match('url=([^\n]+)') + local owner_str = content:match('owner=([^\n]+)') + local clients_str = content:match('clients=([^\n]*)') + local owner = tonumber(owner_str) or 0 + local clients = {} + if clients_str and clients_str ~= '' then + for pid_str in clients_str:gmatch('([^,]+)') do + local pid = tonumber(pid_str) + if pid then + table.insert(clients, pid) + end + end + end + return { url = url, owner = owner, clients = clients } +end + +local function remove_test_lock_file() + local lock_path = get_lock_file_path() + os.remove(lock_path) + os.remove(lock_path .. '.flock') +end + describe('opencode.opencode_server', function() local original_system + local original_uv_kill before_each(function() original_system = vim.system + original_uv_kill = vim.uv.kill + remove_test_lock_file() end) after_each(function() vim.system = original_system + vim.uv.kill = original_uv_kill + remove_test_lock_file() end) -- Tests for server lifecycle behavior @@ -162,4 +223,215 @@ describe('opencode.opencode_server', function() assert.is_nil(server.url) assert.is_nil(server.handle) end) + + describe('lock file operations', function() + it('try_existing_server returns nil when no lock file exists', function() + local url = OpencodeServer.try_existing_server() + assert.is_nil(url) + end) + + it('try_existing_server returns nil when lock file has no alive clients', function() + write_test_lock_file('http://localhost:8888', 99999, { 99999 }) + + vim.uv.kill = function(pid, sig) + if sig == 0 then + error('ESRCH: no such process') + end + end + + local url = OpencodeServer.try_existing_server() + assert.is_nil(url) + + local lock_data = read_test_lock_file() + assert.is_nil(lock_data) + end) + + it('from_existing creates server with url and registers as client', function() + local current_pid = vim.fn.getpid() + write_test_lock_file('http://localhost:9999', 12345, { 12345 }) + + vim.uv.kill = function(pid, sig) + if sig == 0 then + return 0 + end + end + + local server = OpencodeServer.from_existing('http://localhost:9999') + + assert.equals('http://localhost:9999', server.url) + assert.is_false(server.is_owner) + + local lock_data = read_test_lock_file() + assert.is_not_nil(lock_data) + assert.equals(12345, lock_data.owner) + assert.is_true(vim.tbl_contains(lock_data.clients, current_pid)) + end) + + it('spawn registers as owner in lock file', function() + local current_pid = vim.fn.getpid() + + vim.system = function(cmd, opts) + vim.schedule(function() + opts.stdout(nil, 'opencode server listening on http://127.0.0.1:5555') + end) + return { pid = current_pid, kill = function() end } + end + + local server = OpencodeServer.new() + local resolved = false + + server:spawn({ + cwd = '.', + on_ready = function() + resolved = true + end, + on_error = function() end, + on_exit = function() end, + }) + + vim.wait(100, function() + return resolved + end) + + assert.is_true(server.is_owner) + + local lock_data = read_test_lock_file() + assert.is_not_nil(lock_data) + assert.equals('http://127.0.0.1:5555', lock_data.url) + assert.equals(current_pid, lock_data.owner) + assert.is_true(vim.tbl_contains(lock_data.clients, current_pid)) + end) + + it('shutdown removes client from lock file', function() + local current_pid = vim.fn.getpid() + write_test_lock_file('http://localhost:7777', current_pid, { current_pid, 99998 }) + + vim.uv.kill = function(pid, sig) + if sig == 0 then + return 0 + end + end + + local server = OpencodeServer.new() + server.url = 'http://localhost:7777' + server.is_owner = true + server.job = { pid = current_pid, kill = function() end } + + server:shutdown() + + local lock_data = read_test_lock_file() + assert.is_not_nil(lock_data) + assert.is_false(vim.tbl_contains(lock_data.clients, current_pid)) + assert.equals(99998, lock_data.owner) + end) + + it('shutdown removes lock file when last client exits', function() + local current_pid = vim.fn.getpid() + write_test_lock_file('http://localhost:6666', current_pid, { current_pid }) + + vim.uv.kill = function(pid, sig) + if sig == 0 then + return 0 + end + end + + local server = OpencodeServer.new() + server.url = 'http://localhost:6666' + server.is_owner = true + server.job = { pid = current_pid, kill = function() end } + + server:shutdown() + + local lock_data = read_test_lock_file() + assert.is_nil(lock_data) + end) + + it('ownership transfers to next client when owner exits', function() + local current_pid = vim.fn.getpid() + local other_pid_1 = 88881 + local other_pid_2 = 88882 + write_test_lock_file('http://localhost:4444', current_pid, { current_pid, other_pid_1, other_pid_2 }) + + vim.uv.kill = function(pid, sig) + if sig == 0 then + return 0 + end + end + + local server = OpencodeServer.new() + server.url = 'http://localhost:4444' + server.is_owner = true + server.job = { pid = current_pid, kill = function() end } + + server:shutdown() + + local lock_data = read_test_lock_file() + assert.is_not_nil(lock_data) + assert.equals(other_pid_1, lock_data.owner) + assert.equals(2, #lock_data.clients) + assert.is_false(vim.tbl_contains(lock_data.clients, current_pid)) + assert.is_true(vim.tbl_contains(lock_data.clients, other_pid_1)) + assert.is_true(vim.tbl_contains(lock_data.clients, other_pid_2)) + end) + + it('cleanup_dead_pids removes dead processes and promotes new owner', function() + local current_pid = vim.fn.getpid() + local dead_pid = 99997 + local alive_pid = 99996 + + write_test_lock_file('http://localhost:3333', dead_pid, { dead_pid, alive_pid, current_pid }) + + vim.uv.kill = function(pid, sig) + if sig == 0 then + if pid == dead_pid then + error('ESRCH: no such process') + end + return 0 + end + end + + local server = OpencodeServer.from_existing('http://localhost:3333') + + local lock_data = read_test_lock_file() + assert.is_not_nil(lock_data) + assert.is_false(vim.tbl_contains(lock_data.clients, dead_pid)) + assert.equals(alive_pid, lock_data.owner) + end) + + it('flock file is created and removed during operations', function() + local flock_path = get_lock_file_path() .. '.flock' + + vim.uv.kill = function(pid, sig) + if sig == 0 then + return 0 + end + end + + vim.system = function(cmd, opts) + vim.schedule(function() + opts.stdout(nil, 'opencode server listening on http://127.0.0.1:1111') + end) + return { pid = vim.fn.getpid(), kill = function() end } + end + + local server = OpencodeServer.new() + local resolved = false + + server:spawn({ + cwd = '.', + on_ready = function() + resolved = true + end, + on_error = function() end, + on_exit = function() end, + }) + + vim.wait(100, function() + return resolved + end) + + local flock_exists_after = vim.uv.fs_stat(flock_path) ~= nil + assert.is_false(flock_exists_after) + end) + end) end) From 421dbcf68ef0cd0562293626f751080ce2105afc Mon Sep 17 00:00:00 2001 From: jinzhongjia Date: Fri, 2 Jan 2026 20:04:33 +0800 Subject: [PATCH 02/13] test(server_job): mock try_existing_server in unit tests --- tests/unit/server_job_spec.lua | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/tests/unit/server_job_spec.lua b/tests/unit/server_job_spec.lua index bf7b82c5..1a0d35fa 100644 --- a/tests/unit/server_job_spec.lua +++ b/tests/unit/server_job_spec.lua @@ -7,15 +7,18 @@ describe('server_job', function() local original_curl_request local opencode_server = require('opencode.opencode_server') local original_new + local original_try_existing_server before_each(function() original_curl_request = curl.request original_new = opencode_server.new + original_try_existing_server = opencode_server.try_existing_server end) after_each(function() curl.request = original_curl_request opencode_server.new = original_new + opencode_server.try_existing_server = original_try_existing_server end) it('exposes expected public functions', function() @@ -97,6 +100,9 @@ describe('server_job', function() opencode_server.new = function() return fake end + opencode_server.try_existing_server = function() + return nil + end local first = server_job.ensure_server():wait() assert.same(fake, first._value or first) -- ensure_server returns resolved promise value From 2d164f83d2f3e5af1a4b9aaa3635bb7fd537f639 Mon Sep 17 00:00:00 2001 From: jinzhongjia Date: Fri, 2 Jan 2026 20:04:52 +0800 Subject: [PATCH 03/13] test(util): add year-aware matching for date formats Add logic to tests/unit/util_spec.lua to handle year transitions in date formatting. The assertions now check the year of the timestamp to determine if the year should be present in the output string. --- tests/unit/util_spec.lua | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/tests/unit/util_spec.lua b/tests/unit/util_spec.lua index 61a9e488..b7dbb77f 100644 --- a/tests/unit/util_spec.lua +++ b/tests/unit/util_spec.lua @@ -148,7 +148,14 @@ describe('util.format_time', function() it('formats last week with same month date', function() local result = util.format_time(last_week) - assert.matches('^%d%d? %a%a%a %d%d?:%d%d [AP]M$', result) + local last_week_year = tonumber(os.date('%Y', last_week)) + if last_week_year == today.year then + -- Same year: no year in output + assert.matches('^%d%d? %a%a%a %d%d?:%d%d [AP]M$', result) + else + -- Different year (e.g., early January looking back to December): year included + assert.matches('^%d%d? %a%a%a %d%d%d%d %d%d?:%d%d [AP]M$', result) + end end) it('formats future date with full date', function() @@ -170,15 +177,16 @@ describe('util.format_time', function() end) it('handles large millisecond timestamps correctly', function() - local ms_timestamp = 1762350000000 -- ~November 2025 in milliseconds + local ms_timestamp = 1762350000000 local result = util.format_time(ms_timestamp) assert.is_not_nil(result) assert.is_string(result) local is_time_only = result:match('^%d%d?:%d%d [AP]M$') - local is_full_date = result:match('^%d%d? %a%a%a %d%d?:%d%d [AP]M$') - assert.is_true(is_time_only ~= nil or is_full_date ~= nil) + local is_date_same_year = result:match('^%d%d? %a%a%a %d%d?:%d%d [AP]M$') + local is_date_diff_year = result:match('^%d%d? %a%a%a %d%d%d%d %d%d?:%d%d [AP]M$') + assert.is_true(is_time_only ~= nil or is_date_same_year ~= nil or is_date_diff_year ~= nil) end) it('does not convert regular second timestamps', function() From 4470cd4cf522d4bf99070646e09eea0139512823 Mon Sep 17 00:00:00 2001 From: jinzhongjia Date: Fri, 2 Jan 2026 20:19:45 +0800 Subject: [PATCH 04/13] refactor(server): replace owner logic with server pid Replace the `owner` field in the lock file with `server_pid` to explicitly track the background process. Update `unregister_client` to return this PID and remove client promotion logic. --- lua/opencode/opencode_server.lua | 55 +++++++++++++++-------------- tests/unit/opencode_server_spec.lua | 46 ++++++++++++++---------- 2 files changed, 55 insertions(+), 46 deletions(-) diff --git a/lua/opencode/opencode_server.lua b/lua/opencode/opencode_server.lua index 93ed10b1..ac96ca21 100644 --- a/lua/opencode/opencode_server.lua +++ b/lua/opencode/opencode_server.lua @@ -89,8 +89,8 @@ end --- @class LockFileData --- @field url string ---- @field owner number --- @field clients number[] +--- @field server_pid number|nil --- @return LockFileData|nil local function read_lock_file() local lock_path = get_lock_file_path() @@ -107,14 +107,14 @@ local function read_lock_file() end local url = content:match('url=([^\n]+)') - local owner_str = content:match('owner=([^\n]+)') local clients_str = content:match('clients=([^\n]*)') + local server_pid_str = content:match('server_pid=([^\n]+)') if not url then return nil end - local owner = tonumber(owner_str) or 0 + local server_pid = tonumber(server_pid_str) local clients = {} if clients_str and clients_str ~= '' then @@ -126,7 +126,7 @@ local function read_lock_file() end end - return { url = url, owner = owner, clients = clients } + return { url = url, clients = clients, server_pid = server_pid } end --- @param data LockFileData @@ -144,7 +144,10 @@ local function write_lock_file(data) ',' ) - f:write(string.format('url=%s\nowner=%d\nclients=%s\n', data.url, data.owner, clients_str)) + f:write(string.format('url=%s\nclients=%s\n', data.url, clients_str)) + if data.server_pid then + f:write(string.format('server_pid=%d\n', data.server_pid)) + end f:close() end @@ -162,20 +165,17 @@ local function cleanup_dead_pids(data) data.clients = alive_clients - if not is_pid_alive(data.owner) and #alive_clients > 0 then - data.owner = alive_clients[1] - end - return data end --- @param url string -local function register_as_owner(url) +--- @param server_pid number +local function create_lock_file(url, server_pid) with_file_lock(function() write_lock_file({ url = url, - owner = current_pid, clients = { current_pid }, + server_pid = server_pid, }) end) end @@ -201,12 +201,12 @@ local function register_client() return result or false end ---- @return boolean should_kill_server +--- @return number|nil server_pid_to_kill local function unregister_client() local result = with_file_lock(function() local data = read_lock_file() if not data then - return false + return nil end data = cleanup_dead_pids(data) @@ -216,18 +216,15 @@ local function unregister_client() end, data.clients) if #data.clients == 0 then + local server_pid = data.server_pid remove_lock_file() - return true - end - - if data.owner == current_pid then - data.owner = data.clients[1] + return server_pid end write_lock_file(data) - return false + return nil end) - return result or false + return result end --- @return OpencodeServer @@ -327,12 +324,16 @@ end --- @return Promise function OpencodeServer:shutdown() - local should_kill = unregister_client() - - if should_kill and self.job and self.job.pid then - pcall(function() - self.job:kill('sigterm') - end) + local server_pid_to_kill = unregister_client() + + if server_pid_to_kill then + if self.job and self.job.pid then + pcall(function() + self.job:kill('sigterm') + end) + else + pcall(vim.uv.kill, server_pid_to_kill, 'sigterm') + end end self.job = nil @@ -369,7 +370,7 @@ function OpencodeServer:spawn(opts) local url = data:match('opencode server listening on ([^%s]+)') if url then self.url = url - register_as_owner(url) + create_lock_file(url, self.job.pid) self.spawn_promise:resolve(self --[[@as any]]) safe_call(opts.on_ready, self.job, url) end diff --git a/tests/unit/opencode_server_spec.lua b/tests/unit/opencode_server_spec.lua index 0f751a9c..dc326334 100644 --- a/tests/unit/opencode_server_spec.lua +++ b/tests/unit/opencode_server_spec.lua @@ -8,7 +8,10 @@ local function get_lock_file_path() end --- Helper to write a lock file directly for testing -local function write_test_lock_file(url, owner, clients) +--- @param url string +--- @param clients number[] +--- @param server_pid number|nil +local function write_test_lock_file(url, clients, server_pid) local lock_path = get_lock_file_path() local f = io.open(lock_path, 'w') if f then @@ -18,7 +21,10 @@ local function write_test_lock_file(url, owner, clients) end, clients), ',' ) - f:write(string.format('url=%s\nowner=%d\nclients=%s\n', url, owner, clients_str)) + f:write(string.format('url=%s\nclients=%s\n', url, clients_str)) + if server_pid then + f:write(string.format('server_pid=%d\n', server_pid)) + end f:close() end end @@ -36,9 +42,8 @@ local function read_test_lock_file() return nil end local url = content:match('url=([^\n]+)') - local owner_str = content:match('owner=([^\n]+)') local clients_str = content:match('clients=([^\n]*)') - local owner = tonumber(owner_str) or 0 + local server_pid_str = content:match('server_pid=([^\n]+)') local clients = {} if clients_str and clients_str ~= '' then for pid_str in clients_str:gmatch('([^,]+)') do @@ -48,7 +53,8 @@ local function read_test_lock_file() end end end - return { url = url, owner = owner, clients = clients } + local server_pid = tonumber(server_pid_str) + return { url = url, clients = clients, server_pid = server_pid } end local function remove_test_lock_file() @@ -231,7 +237,7 @@ describe('opencode.opencode_server', function() end) it('try_existing_server returns nil when lock file has no alive clients', function() - write_test_lock_file('http://localhost:8888', 99999, { 99999 }) + write_test_lock_file('http://localhost:8888', { 99999 }, 99999) vim.uv.kill = function(pid, sig) if sig == 0 then @@ -248,7 +254,7 @@ describe('opencode.opencode_server', function() it('from_existing creates server with url and registers as client', function() local current_pid = vim.fn.getpid() - write_test_lock_file('http://localhost:9999', 12345, { 12345 }) + write_test_lock_file('http://localhost:9999', { 12345 }, 54321) vim.uv.kill = function(pid, sig) if sig == 0 then @@ -263,11 +269,11 @@ describe('opencode.opencode_server', function() local lock_data = read_test_lock_file() assert.is_not_nil(lock_data) - assert.equals(12345, lock_data.owner) assert.is_true(vim.tbl_contains(lock_data.clients, current_pid)) + assert.equals(54321, lock_data.server_pid) end) - it('spawn registers as owner in lock file', function() + it('spawn creates lock file with server_pid', function() local current_pid = vim.fn.getpid() vim.system = function(cmd, opts) @@ -298,13 +304,13 @@ describe('opencode.opencode_server', function() local lock_data = read_test_lock_file() assert.is_not_nil(lock_data) assert.equals('http://127.0.0.1:5555', lock_data.url) - assert.equals(current_pid, lock_data.owner) assert.is_true(vim.tbl_contains(lock_data.clients, current_pid)) + assert.equals(current_pid, lock_data.server_pid) end) it('shutdown removes client from lock file', function() local current_pid = vim.fn.getpid() - write_test_lock_file('http://localhost:7777', current_pid, { current_pid, 99998 }) + write_test_lock_file('http://localhost:7777', { current_pid, 99998 }, 54321) vim.uv.kill = function(pid, sig) if sig == 0 then @@ -322,12 +328,12 @@ describe('opencode.opencode_server', function() local lock_data = read_test_lock_file() assert.is_not_nil(lock_data) assert.is_false(vim.tbl_contains(lock_data.clients, current_pid)) - assert.equals(99998, lock_data.owner) + assert.is_true(vim.tbl_contains(lock_data.clients, 99998)) end) it('shutdown removes lock file when last client exits', function() local current_pid = vim.fn.getpid() - write_test_lock_file('http://localhost:6666', current_pid, { current_pid }) + write_test_lock_file('http://localhost:6666', { current_pid }, 54321) vim.uv.kill = function(pid, sig) if sig == 0 then @@ -346,11 +352,11 @@ describe('opencode.opencode_server', function() assert.is_nil(lock_data) end) - it('ownership transfers to next client when owner exits', function() + it('shutdown keeps remaining clients when one client exits', function() local current_pid = vim.fn.getpid() local other_pid_1 = 88881 local other_pid_2 = 88882 - write_test_lock_file('http://localhost:4444', current_pid, { current_pid, other_pid_1, other_pid_2 }) + write_test_lock_file('http://localhost:4444', { current_pid, other_pid_1, other_pid_2 }, 54321) vim.uv.kill = function(pid, sig) if sig == 0 then @@ -367,19 +373,19 @@ describe('opencode.opencode_server', function() local lock_data = read_test_lock_file() assert.is_not_nil(lock_data) - assert.equals(other_pid_1, lock_data.owner) assert.equals(2, #lock_data.clients) assert.is_false(vim.tbl_contains(lock_data.clients, current_pid)) assert.is_true(vim.tbl_contains(lock_data.clients, other_pid_1)) assert.is_true(vim.tbl_contains(lock_data.clients, other_pid_2)) + assert.equals(54321, lock_data.server_pid) end) - it('cleanup_dead_pids removes dead processes and promotes new owner', function() + it('cleanup_dead_pids removes dead processes from clients list', function() local current_pid = vim.fn.getpid() local dead_pid = 99997 local alive_pid = 99996 - write_test_lock_file('http://localhost:3333', dead_pid, { dead_pid, alive_pid, current_pid }) + write_test_lock_file('http://localhost:3333', { dead_pid, alive_pid, current_pid }, 54321) vim.uv.kill = function(pid, sig) if sig == 0 then @@ -395,7 +401,9 @@ describe('opencode.opencode_server', function() local lock_data = read_test_lock_file() assert.is_not_nil(lock_data) assert.is_false(vim.tbl_contains(lock_data.clients, dead_pid)) - assert.equals(alive_pid, lock_data.owner) + assert.is_true(vim.tbl_contains(lock_data.clients, alive_pid)) + assert.is_true(vim.tbl_contains(lock_data.clients, current_pid)) + assert.equals(54321, lock_data.server_pid) end) it('flock file is created and removed during operations', function() From f948664afb5e4f2b8ca3f8143d632ef78401b23d Mon Sep 17 00:00:00 2001 From: jinzhongjia Date: Fri, 2 Jan 2026 20:27:39 +0800 Subject: [PATCH 05/13] fix(ci): detect test failures by output content, not exit code nvim returns non-zero exit codes when error messages are printed, even for expected errors in error handling tests. Changed test runner to check for actual '[31mFail.*||' patterns in output instead. --- run_tests.sh | 249 ++++++++++++++++++++++++++------------------------- 1 file changed, 125 insertions(+), 124 deletions(-) diff --git a/run_tests.sh b/run_tests.sh index e9a86902..a11a8592 100755 --- a/run_tests.sh +++ b/run_tests.sh @@ -15,58 +15,63 @@ FILTER="" TEST_TYPE="all" print_usage() { - echo "Usage: $0 [OPTIONS]" - echo "Options:" - echo " -f, --filter PATTERN Filter tests by pattern (matches test descriptions)" - echo " -t, --type TYPE Test type: all, minimal, unit, replay, or specific file path" - echo " -h, --help Show this help message" - echo "" - echo "Examples:" - echo " $0 # Run all tests" - echo " $0 -f \"Timer\" # Run tests matching 'Timer'" - echo " $0 -t unit # Run only unit tests" - echo " $0 -t replay # Run only replay tests" - echo " $0 -t tests/unit/timer_spec.lua # Run specific test file" - echo " $0 -f \"creates a new timer\" -t unit # Filter unit tests" + echo "Usage: $0 [OPTIONS]" + echo "Options:" + echo " -f, --filter PATTERN Filter tests by pattern (matches test descriptions)" + echo " -t, --type TYPE Test type: all, minimal, unit, replay, or specific file path" + echo " -h, --help Show this help message" + echo "" + echo "Examples:" + echo " $0 # Run all tests" + echo " $0 -f \"Timer\" # Run tests matching 'Timer'" + echo " $0 -t unit # Run only unit tests" + echo " $0 -t replay # Run only replay tests" + echo " $0 -t tests/unit/timer_spec.lua # Run specific test file" + echo " $0 -f \"creates a new timer\" -t unit # Filter unit tests" } while [[ $# -gt 0 ]]; do - case $1 in - -f | --filter) - FILTER="$2" - shift 2 - ;; - -t | --type) - TEST_TYPE="$2" - shift 2 - ;; - -h | --help) - print_usage - exit 0 - ;; - *) - echo "Unknown option: $1" - print_usage - exit 1 - ;; - esac + case $1 in + -f | --filter) + FILTER="$2" + shift 2 + ;; + -t | --type) + TEST_TYPE="$2" + shift 2 + ;; + -h | --help) + print_usage + exit 0 + ;; + *) + echo "Unknown option: $1" + print_usage + exit 1 + ;; + esac done # Clean test output by removing Errors lines clean_output() { - echo "$1" | grep -v "\[31mErrors : " + echo "$1" | grep -v "\[31mErrors : " +} + +# Check if output contains failed tests (red "Fail" text from mini.test) +output_contains_failed_tests() { + echo "$1" | grep -q "\[31mFail.*||" } # Build filter option for plenary FILTER_OPTION="" if [ -n "$FILTER" ]; then - FILTER_OPTION=", filter = '$FILTER'" + FILTER_OPTION=", filter = '$FILTER'" fi if [ -n "$FILTER" ]; then - echo -e "${YELLOW}Running tests for opencode.nvim (filter: '$FILTER')${NC}" + echo -e "${YELLOW}Running tests for opencode.nvim (filter: '$FILTER')${NC}" else - echo -e "${YELLOW}Running tests for opencode.nvim${NC}" + echo -e "${YELLOW}Running tests for opencode.nvim${NC}" fi echo "------------------------------------------------" @@ -79,69 +84,65 @@ unit_output="" replay_output="" if [ "$TEST_TYPE" = "all" ] || [ "$TEST_TYPE" = "minimal" ]; then - # Run minimal tests - minimal_output=$(nvim --headless -u tests/minimal/init.lua -c "lua require('plenary.test_harness').test_directory('./tests/minimal', {minimal_init = './tests/minimal/init.lua', sequential = true$FILTER_OPTION})" 2>&1) - minimal_status=$? - clean_output "$minimal_output" - - if [ $minimal_status -eq 0 ]; then - echo -e "${GREEN}✓ Minimal tests passed${NC}" - else - echo -e "${RED}✗ Minimal tests failed${NC}" - fi - echo "------------------------------------------------" + # Run minimal tests + minimal_output=$(nvim --headless -u tests/minimal/init.lua -c "lua require('plenary.test_harness').test_directory('./tests/minimal', {minimal_init = './tests/minimal/init.lua', sequential = true$FILTER_OPTION})" 2>&1) + clean_output "$minimal_output" + if output_contains_failed_tests "$minimal_output"; then + minimal_status=1 + echo -e "${RED}✗ Minimal tests failed${NC}" + else + echo -e "${GREEN}✓ Minimal tests passed${NC}" + fi + echo "------------------------------------------------" fi if [ "$TEST_TYPE" = "all" ] || [ "$TEST_TYPE" = "unit" ]; then - # Run unit tests - unit_output=$(nvim --headless -u tests/minimal/init.lua -c "lua require('plenary.test_harness').test_directory('./tests/unit', {minimal_init = './tests/minimal/init.lua'$FILTER_OPTION})" 2>&1) - unit_status=$? - clean_output "$unit_output" - - if [ $unit_status -eq 0 ]; then - echo -e "${GREEN}✓ Unit tests passed${NC}" - else - echo -e "${RED}✗ Unit tests failed${NC}" - fi - echo "------------------------------------------------" + # Run unit tests + unit_output=$(nvim --headless -u tests/minimal/init.lua -c "lua require('plenary.test_harness').test_directory('./tests/unit', {minimal_init = './tests/minimal/init.lua'$FILTER_OPTION})" 2>&1) + clean_output "$unit_output" + if output_contains_failed_tests "$unit_output"; then + unit_status=1 + echo -e "${RED}✗ Unit tests failed${NC}" + else + echo -e "${GREEN}✓ Unit tests passed${NC}" + fi + echo "------------------------------------------------" fi if [ "$TEST_TYPE" = "all" ] || [ "$TEST_TYPE" = "replay" ]; then - # Run replay tests - replay_output=$(nvim --headless -u tests/minimal/init.lua -c "lua require('plenary.test_harness').test_directory('./tests/replay', {minimal_init = './tests/minimal/init.lua'$FILTER_OPTION})" 2>&1) - replay_status=$? - clean_output "$replay_output" - - if [ $replay_status -eq 0 ]; then - echo -e "${GREEN}✓ Replay tests passed${NC}" - else - echo -e "${RED}✗ Replay tests failed${NC}" - fi - echo "------------------------------------------------" + # Run replay tests + replay_output=$(nvim --headless -u tests/minimal/init.lua -c "lua require('plenary.test_harness').test_directory('./tests/replay', {minimal_init = './tests/minimal/init.lua'$FILTER_OPTION})" 2>&1) + clean_output "$replay_output" + if output_contains_failed_tests "$replay_output"; then + replay_status=1 + echo -e "${RED}✗ Replay tests failed${NC}" + else + echo -e "${GREEN}✓ Replay tests passed${NC}" + fi + echo "------------------------------------------------" fi # Handle specific test file if [ "$TEST_TYPE" != "all" ] && [ "$TEST_TYPE" != "minimal" ] && [ "$TEST_TYPE" != "unit" ] && [ "$TEST_TYPE" != "replay" ]; then - # Assume it's a specific test file path - if [ -f "$TEST_TYPE" ]; then - specific_output=$(nvim --headless -u tests/minimal/init.lua -c "lua require('plenary.test_harness').test_directory('./$TEST_TYPE', {minimal_init = './tests/minimal/init.lua'$FILTER_OPTION})" 2>&1) - specific_status=$? - clean_output "$specific_output" - - if [ $specific_status -eq 0 ]; then - echo -e "${GREEN}✓ Specific test passed${NC}" - else - echo -e "${RED}✗ Specific test failed${NC}" - fi - echo "------------------------------------------------" - - # Use specific test output for failure analysis - unit_output="$specific_output" - unit_status=$specific_status - else - echo -e "${RED}Error: Test file '$TEST_TYPE' not found${NC}" - exit 1 - fi + # Assume it's a specific test file path + if [ -f "$TEST_TYPE" ]; then + specific_output=$(nvim --headless -u tests/minimal/init.lua -c "lua require('plenary.test_harness').test_directory('./$TEST_TYPE', {minimal_init = './tests/minimal/init.lua'$FILTER_OPTION})" 2>&1) + clean_output "$specific_output" + if output_contains_failed_tests "$specific_output"; then + specific_status=1 + echo -e "${RED}✗ Specific test failed${NC}" + else + echo -e "${GREEN}✓ Specific test passed${NC}" + fi + echo "------------------------------------------------" + + # Use specific test output for failure analysis + unit_output="$specific_output" + unit_status=$specific_status + else + echo -e "${RED}Error: Test file '$TEST_TYPE' not found${NC}" + exit 1 + fi fi # Check for any failures @@ -150,40 +151,40 @@ $unit_output $replay_output" if [ $minimal_status -ne 0 ] || [ $unit_status -ne 0 ] || [ $replay_status -ne 0 ] || echo "$all_output" | grep -q "\[31mFail.*||"; then - echo -e "\n${RED}======== TEST FAILURES SUMMARY ========${NC}" - - # Extract and format failures - failures_file=$(mktemp) - echo "$all_output" | grep -B 0 -A 6 "\[31mFail.*||" >"$failures_file" - failure_count=$(grep -c "\[31mFail.*||" "$failures_file") - - echo -e "${RED}Found $failure_count failing test(s):${NC}\n" - - # Process the output line by line - test_name="" - while IFS= read -r line; do - # Remove ANSI color codes - clean_line=$(echo "$line" | sed -E 's/\x1B\[[0-9;]*[mK]//g') - - if [[ "$clean_line" == *"Fail"*"||"* ]]; then - # Extract test name - test_name=$(echo "$clean_line" | sed -E 's/.*Fail.*\|\|\s*(.*)/\1/') - echo -e "${RED}FAILED TEST:${NC} $test_name" - elif [[ "$clean_line" == *"/Users/"*".lua:"*": "* ]]; then - # This is an error message with file:line - echo -e " ${RED}ERROR:${NC} $clean_line" - elif [[ "$clean_line" == *"stack traceback"* ]]; then - # Stack trace header - echo -e " ${YELLOW}TRACE:${NC} $clean_line" - elif [[ "$clean_line" == *"in function"* ]]; then - # Stack trace details - echo -e " $clean_line" - fi - done <"$failures_file" - - rm -f "$failures_file" - exit 1 + echo -e "\n${RED}======== TEST FAILURES SUMMARY ========${NC}" + + # Extract and format failures + failures_file=$(mktemp) + echo "$all_output" | grep -B 0 -A 6 "\[31mFail.*||" >"$failures_file" + failure_count=$(grep -c "\[31mFail.*||" "$failures_file") + + echo -e "${RED}Found $failure_count failing test(s):${NC}\n" + + # Process the output line by line + test_name="" + while IFS= read -r line; do + # Remove ANSI color codes + clean_line=$(echo "$line" | sed -E 's/\x1B\[[0-9;]*[mK]//g') + + if [[ "$clean_line" == *"Fail"*"||"* ]]; then + # Extract test name + test_name=$(echo "$clean_line" | sed -E 's/.*Fail.*\|\|\s*(.*)/\1/') + echo -e "${RED}FAILED TEST:${NC} $test_name" + elif [[ "$clean_line" == *"/Users/"*".lua:"*": "* ]]; then + # This is an error message with file:line + echo -e " ${RED}ERROR:${NC} $clean_line" + elif [[ "$clean_line" == *"stack traceback"* ]]; then + # Stack trace header + echo -e " ${YELLOW}TRACE:${NC} $clean_line" + elif [[ "$clean_line" == *"in function"* ]]; then + # Stack trace details + echo -e " $clean_line" + fi + done <"$failures_file" + + rm -f "$failures_file" + exit 1 else - echo -e "${GREEN}All tests passed successfully!${NC}" - exit 0 + echo -e "${GREEN}All tests passed successfully!${NC}" + exit 0 fi From 36b01d43ac4ec00ed6bc635dd62de21897c64af2 Mon Sep 17 00:00:00 2001 From: jinzhongjia Date: Fri, 2 Jan 2026 20:45:40 +0800 Subject: [PATCH 06/13] style: replace tabs with spaces in run_tests.sh --- run_tests.sh | 238 +++++++++++++++++++++++++-------------------------- 1 file changed, 119 insertions(+), 119 deletions(-) diff --git a/run_tests.sh b/run_tests.sh index a11a8592..d5a1a8f3 100755 --- a/run_tests.sh +++ b/run_tests.sh @@ -15,63 +15,63 @@ FILTER="" TEST_TYPE="all" print_usage() { - echo "Usage: $0 [OPTIONS]" - echo "Options:" - echo " -f, --filter PATTERN Filter tests by pattern (matches test descriptions)" - echo " -t, --type TYPE Test type: all, minimal, unit, replay, or specific file path" - echo " -h, --help Show this help message" - echo "" - echo "Examples:" - echo " $0 # Run all tests" - echo " $0 -f \"Timer\" # Run tests matching 'Timer'" - echo " $0 -t unit # Run only unit tests" - echo " $0 -t replay # Run only replay tests" - echo " $0 -t tests/unit/timer_spec.lua # Run specific test file" - echo " $0 -f \"creates a new timer\" -t unit # Filter unit tests" + echo "Usage: $0 [OPTIONS]" + echo "Options:" + echo " -f, --filter PATTERN Filter tests by pattern (matches test descriptions)" + echo " -t, --type TYPE Test type: all, minimal, unit, replay, or specific file path" + echo " -h, --help Show this help message" + echo "" + echo "Examples:" + echo " $0 # Run all tests" + echo " $0 -f \"Timer\" # Run tests matching 'Timer'" + echo " $0 -t unit # Run only unit tests" + echo " $0 -t replay # Run only replay tests" + echo " $0 -t tests/unit/timer_spec.lua # Run specific test file" + echo " $0 -f \"creates a new timer\" -t unit # Filter unit tests" } while [[ $# -gt 0 ]]; do - case $1 in - -f | --filter) - FILTER="$2" - shift 2 - ;; - -t | --type) - TEST_TYPE="$2" - shift 2 - ;; - -h | --help) - print_usage - exit 0 - ;; - *) - echo "Unknown option: $1" - print_usage - exit 1 - ;; - esac + case $1 in + -f | --filter) + FILTER="$2" + shift 2 + ;; + -t | --type) + TEST_TYPE="$2" + shift 2 + ;; + -h | --help) + print_usage + exit 0 + ;; + *) + echo "Unknown option: $1" + print_usage + exit 1 + ;; + esac done # Clean test output by removing Errors lines clean_output() { - echo "$1" | grep -v "\[31mErrors : " + echo "$1" | grep -v "\[31mErrors : " } # Check if output contains failed tests (red "Fail" text from mini.test) output_contains_failed_tests() { - echo "$1" | grep -q "\[31mFail.*||" + echo "$1" | grep -q "\[31mFail.*||" } # Build filter option for plenary FILTER_OPTION="" if [ -n "$FILTER" ]; then - FILTER_OPTION=", filter = '$FILTER'" + FILTER_OPTION=", filter = '$FILTER'" fi if [ -n "$FILTER" ]; then - echo -e "${YELLOW}Running tests for opencode.nvim (filter: '$FILTER')${NC}" + echo -e "${YELLOW}Running tests for opencode.nvim (filter: '$FILTER')${NC}" else - echo -e "${YELLOW}Running tests for opencode.nvim${NC}" + echo -e "${YELLOW}Running tests for opencode.nvim${NC}" fi echo "------------------------------------------------" @@ -84,65 +84,65 @@ unit_output="" replay_output="" if [ "$TEST_TYPE" = "all" ] || [ "$TEST_TYPE" = "minimal" ]; then - # Run minimal tests - minimal_output=$(nvim --headless -u tests/minimal/init.lua -c "lua require('plenary.test_harness').test_directory('./tests/minimal', {minimal_init = './tests/minimal/init.lua', sequential = true$FILTER_OPTION})" 2>&1) - clean_output "$minimal_output" - if output_contains_failed_tests "$minimal_output"; then - minimal_status=1 - echo -e "${RED}✗ Minimal tests failed${NC}" + # Run minimal tests + minimal_output=$(nvim --headless -u tests/minimal/init.lua -c "lua require('plenary.test_harness').test_directory('./tests/minimal', {minimal_init = './tests/minimal/init.lua', sequential = true$FILTER_OPTION})" 2>&1) + clean_output "$minimal_output" + if output_contains_failed_tests "$minimal_output"; then + minimal_status=1 + echo -e "${RED}✗ Minimal tests failed${NC}" else - echo -e "${GREEN}✓ Minimal tests passed${NC}" - fi - echo "------------------------------------------------" + echo -e "${GREEN}✓ Minimal tests passed${NC}" + fi + echo "------------------------------------------------" fi if [ "$TEST_TYPE" = "all" ] || [ "$TEST_TYPE" = "unit" ]; then - # Run unit tests - unit_output=$(nvim --headless -u tests/minimal/init.lua -c "lua require('plenary.test_harness').test_directory('./tests/unit', {minimal_init = './tests/minimal/init.lua'$FILTER_OPTION})" 2>&1) - clean_output "$unit_output" - if output_contains_failed_tests "$unit_output"; then - unit_status=1 - echo -e "${RED}✗ Unit tests failed${NC}" - else - echo -e "${GREEN}✓ Unit tests passed${NC}" - fi - echo "------------------------------------------------" + # Run unit tests + unit_output=$(nvim --headless -u tests/minimal/init.lua -c "lua require('plenary.test_harness').test_directory('./tests/unit', {minimal_init = './tests/minimal/init.lua'$FILTER_OPTION})" 2>&1) + clean_output "$unit_output" + if output_contains_failed_tests "$unit_output"; then + unit_status=1 + echo -e "${RED}✗ Unit tests failed${NC}" + else + echo -e "${GREEN}✓ Unit tests passed${NC}" + fi + echo "------------------------------------------------" fi if [ "$TEST_TYPE" = "all" ] || [ "$TEST_TYPE" = "replay" ]; then - # Run replay tests - replay_output=$(nvim --headless -u tests/minimal/init.lua -c "lua require('plenary.test_harness').test_directory('./tests/replay', {minimal_init = './tests/minimal/init.lua'$FILTER_OPTION})" 2>&1) - clean_output "$replay_output" - if output_contains_failed_tests "$replay_output"; then - replay_status=1 - echo -e "${RED}✗ Replay tests failed${NC}" - else - echo -e "${GREEN}✓ Replay tests passed${NC}" - fi - echo "------------------------------------------------" + # Run replay tests + replay_output=$(nvim --headless -u tests/minimal/init.lua -c "lua require('plenary.test_harness').test_directory('./tests/replay', {minimal_init = './tests/minimal/init.lua'$FILTER_OPTION})" 2>&1) + clean_output "$replay_output" + if output_contains_failed_tests "$replay_output"; then + replay_status=1 + echo -e "${RED}✗ Replay tests failed${NC}" + else + echo -e "${GREEN}✓ Replay tests passed${NC}" + fi + echo "------------------------------------------------" fi # Handle specific test file if [ "$TEST_TYPE" != "all" ] && [ "$TEST_TYPE" != "minimal" ] && [ "$TEST_TYPE" != "unit" ] && [ "$TEST_TYPE" != "replay" ]; then - # Assume it's a specific test file path + # Assume it's a specific test file path if [ -f "$TEST_TYPE" ]; then - specific_output=$(nvim --headless -u tests/minimal/init.lua -c "lua require('plenary.test_harness').test_directory('./$TEST_TYPE', {minimal_init = './tests/minimal/init.lua'$FILTER_OPTION})" 2>&1) - clean_output "$specific_output" - if output_contains_failed_tests "$specific_output"; then - specific_status=1 - echo -e "${RED}✗ Specific test failed${NC}" - else - echo -e "${GREEN}✓ Specific test passed${NC}" - fi - echo "------------------------------------------------" - - # Use specific test output for failure analysis - unit_output="$specific_output" - unit_status=$specific_status - else - echo -e "${RED}Error: Test file '$TEST_TYPE' not found${NC}" - exit 1 - fi + specific_output=$(nvim --headless -u tests/minimal/init.lua -c "lua require('plenary.test_harness').test_directory('./$TEST_TYPE', {minimal_init = './tests/minimal/init.lua'$FILTER_OPTION})" 2>&1) + clean_output "$specific_output" + if output_contains_failed_tests "$specific_output"; then + specific_status=1 + echo -e "${RED}✗ Specific test failed${NC}" + else + echo -e "${GREEN}✓ Specific test passed${NC}" + fi + echo "------------------------------------------------" + + # Use specific test output for failure analysis + unit_output="$specific_output" + unit_status=$specific_status + else + echo -e "${RED}Error: Test file '$TEST_TYPE' not found${NC}" + exit 1 + fi fi # Check for any failures @@ -151,40 +151,40 @@ $unit_output $replay_output" if [ $minimal_status -ne 0 ] || [ $unit_status -ne 0 ] || [ $replay_status -ne 0 ] || echo "$all_output" | grep -q "\[31mFail.*||"; then - echo -e "\n${RED}======== TEST FAILURES SUMMARY ========${NC}" - - # Extract and format failures - failures_file=$(mktemp) - echo "$all_output" | grep -B 0 -A 6 "\[31mFail.*||" >"$failures_file" - failure_count=$(grep -c "\[31mFail.*||" "$failures_file") - - echo -e "${RED}Found $failure_count failing test(s):${NC}\n" - - # Process the output line by line - test_name="" - while IFS= read -r line; do - # Remove ANSI color codes - clean_line=$(echo "$line" | sed -E 's/\x1B\[[0-9;]*[mK]//g') - - if [[ "$clean_line" == *"Fail"*"||"* ]]; then - # Extract test name - test_name=$(echo "$clean_line" | sed -E 's/.*Fail.*\|\|\s*(.*)/\1/') - echo -e "${RED}FAILED TEST:${NC} $test_name" - elif [[ "$clean_line" == *"/Users/"*".lua:"*": "* ]]; then - # This is an error message with file:line - echo -e " ${RED}ERROR:${NC} $clean_line" - elif [[ "$clean_line" == *"stack traceback"* ]]; then - # Stack trace header - echo -e " ${YELLOW}TRACE:${NC} $clean_line" - elif [[ "$clean_line" == *"in function"* ]]; then - # Stack trace details - echo -e " $clean_line" - fi - done <"$failures_file" - - rm -f "$failures_file" - exit 1 + echo -e "\n${RED}======== TEST FAILURES SUMMARY ========${NC}" + + # Extract and format failures + failures_file=$(mktemp) + echo "$all_output" | grep -B 0 -A 6 "\[31mFail.*||" >"$failures_file" + failure_count=$(grep -c "\[31mFail.*||" "$failures_file") + + echo -e "${RED}Found $failure_count failing test(s):${NC}\n" + + # Process the output line by line + test_name="" + while IFS= read -r line; do + # Remove ANSI color codes + clean_line=$(echo "$line" | sed -E 's/\x1B\[[0-9;]*[mK]//g') + + if [[ "$clean_line" == *"Fail"*"||"* ]]; then + # Extract test name + test_name=$(echo "$clean_line" | sed -E 's/.*Fail.*\|\|\s*(.*)/\1/') + echo -e "${RED}FAILED TEST:${NC} $test_name" + elif [[ "$clean_line" == *"/Users/"*".lua:"*": "* ]]; then + # This is an error message with file:line + echo -e " ${RED}ERROR:${NC} $clean_line" + elif [[ "$clean_line" == *"stack traceback"* ]]; then + # Stack trace header + echo -e " ${YELLOW}TRACE:${NC} $clean_line" + elif [[ "$clean_line" == *"in function"* ]]; then + # Stack trace details + echo -e " $clean_line" + fi + done <"$failures_file" + + rm -f "$failures_file" + exit 1 else - echo -e "${GREEN}All tests passed successfully!${NC}" - exit 0 + echo -e "${GREEN}All tests passed successfully!${NC}" + exit 0 fi From 196a3820f992430a3f464f63f65df1a276894ba4 Mon Sep 17 00:00:00 2001 From: jinzhongjia Date: Fri, 2 Jan 2026 20:47:28 +0800 Subject: [PATCH 07/13] style: replace tabs with spaces in run_tests.sh --- run_tests.sh | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/run_tests.sh b/run_tests.sh index d5a1a8f3..22fab566 100755 --- a/run_tests.sh +++ b/run_tests.sh @@ -37,9 +37,9 @@ while [[ $# -gt 0 ]]; do shift 2 ;; -t | --type) - TEST_TYPE="$2" - shift 2 - ;; + TEST_TYPE="$2" + shift 2 + ;; -h | --help) print_usage exit 0 @@ -125,7 +125,7 @@ fi # Handle specific test file if [ "$TEST_TYPE" != "all" ] && [ "$TEST_TYPE" != "minimal" ] && [ "$TEST_TYPE" != "unit" ] && [ "$TEST_TYPE" != "replay" ]; then # Assume it's a specific test file path - if [ -f "$TEST_TYPE" ]; then + if [ -f "$TEST_TYPE" ]; then specific_output=$(nvim --headless -u tests/minimal/init.lua -c "lua require('plenary.test_harness').test_directory('./$TEST_TYPE', {minimal_init = './tests/minimal/init.lua'$FILTER_OPTION})" 2>&1) clean_output "$specific_output" if output_contains_failed_tests "$specific_output"; then @@ -139,7 +139,7 @@ if [ "$TEST_TYPE" != "all" ] && [ "$TEST_TYPE" != "minimal" ] && [ "$TEST_TYPE" # Use specific test output for failure analysis unit_output="$specific_output" unit_status=$specific_status - else + else echo -e "${RED}Error: Test file '$TEST_TYPE' not found${NC}" exit 1 fi @@ -158,7 +158,7 @@ if [ $minimal_status -ne 0 ] || [ $unit_status -ne 0 ] || [ $replay_status -ne 0 echo "$all_output" | grep -B 0 -A 6 "\[31mFail.*||" >"$failures_file" failure_count=$(grep -c "\[31mFail.*||" "$failures_file") - echo -e "${RED}Found $failure_count failing test(s):${NC}\n" + echo -e "${RED}Found $failure_count failing test(s):${NC}\n" # Process the output line by line test_name="" From 6d835a0bcb6f78061c01e8281f15398c2179d1ea Mon Sep 17 00:00:00 2001 From: jinzhongjia Date: Tue, 6 Jan 2026 11:31:36 +0800 Subject: [PATCH 08/13] refactor: extract health check logic to api_client Move health check logic from OpencodeServer.try_existing_server to a new static OpencodeApiClient.check_health method using promises. --- lua/opencode/api_client.lua | 32 ++++++++++++++++++++++++++++++++ lua/opencode/opencode_server.lua | 23 ++--------------------- 2 files changed, 34 insertions(+), 21 deletions(-) diff --git a/lua/opencode/api_client.lua b/lua/opencode/api_client.lua index 3bb04c5a..947b0449 100644 --- a/lua/opencode/api_client.lua +++ b/lua/opencode/api_client.lua @@ -434,6 +434,37 @@ function OpencodeApiClient:list_tools(provider, model, directory) }) end +--- Check if a server at the given URL is healthy +--- This is a static method that doesn't require an established connection +--- @param base_url string The server URL to check +--- @param timeout_ms? number Timeout in milliseconds (default: 2000) +--- @return Promise promise that resolves to true if healthy, rejects otherwise +function OpencodeApiClient.check_health(base_url, timeout_ms) + local Promise = require('opencode.promise') + local curl = require('opencode.curl') + + local health_promise = Promise.new() + local url = base_url:gsub('/$', '') .. '/global/health' + + curl.request({ + url = url, + method = 'GET', + timeout = timeout_ms or 2000, + callback = function(response) + if response and response.status >= 200 and response.status < 300 then + health_promise:resolve(true) + else + health_promise:reject('unhealthy') + end + end, + on_error = function(err) + health_promise:reject(err or 'health check failed') + end, + }) + + return health_promise +end + --- Create a factory function for the module --- @param base_url? string The base URL of the opencode server --- @return OpencodeApiClient @@ -464,4 +495,5 @@ end return { new = OpencodeApiClient.new, create = create_client, + check_health = OpencodeApiClient.check_health, } diff --git a/lua/opencode/opencode_server.lua b/lua/opencode/opencode_server.lua index ac96ca21..015de61a 100644 --- a/lua/opencode/opencode_server.lua +++ b/lua/opencode/opencode_server.lua @@ -272,27 +272,8 @@ function OpencodeServer.try_existing_server() return nil end - local curl = require('opencode.curl') - local result_received = false - local is_healthy = false - - curl.request({ - url = server_url .. '/global/health', - method = 'GET', - timeout = 2000, - callback = function(response) - result_received = true - is_healthy = response and response.status >= 200 and response.status < 300 - end, - on_error = function() - result_received = true - is_healthy = false - end, - }) - - vim.wait(3000, function() - return result_received - end, 50) + local api_client = require('opencode.api_client') + local is_healthy = api_client.check_health(server_url, 2000):wait(3000) if is_healthy then return server_url From bf7917e92561261657532528525690ac2a5afc28 Mon Sep 17 00:00:00 2001 From: jinzhongjia Date: Tue, 6 Jan 2026 13:42:27 +0800 Subject: [PATCH 09/13] refactor(server): replace any with OpencodeServer --- lua/opencode/opencode_server.lua | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lua/opencode/opencode_server.lua b/lua/opencode/opencode_server.lua index 015de61a..a098d112 100644 --- a/lua/opencode/opencode_server.lua +++ b/lua/opencode/opencode_server.lua @@ -292,7 +292,7 @@ function OpencodeServer.from_existing(url) server.url = url server.is_owner = false register_client() - server.spawn_promise:resolve(server --[[@as any]]) + server.spawn_promise:resolve(server --[[@as OpencodeServer]]) return server end @@ -352,7 +352,7 @@ function OpencodeServer:spawn(opts) if url then self.url = url create_lock_file(url, self.job.pid) - self.spawn_promise:resolve(self --[[@as any]]) + self.spawn_promise:resolve(self --[[@as OpencodeServer]]) safe_call(opts.on_ready, self.job, url) end end From 476e7055cfeb213fffa322743707fae440b9f8f8 Mon Sep 17 00:00:00 2001 From: jinzhongjia Date: Thu, 8 Jan 2026 20:46:30 +0800 Subject: [PATCH 10/13] refactor(server): remove file lock mechanism Replace file lock-based server discovery with direct health checks. Adds configurable server port and improves shutdown handling with cross-platform support and graceful process termination. --- lua/opencode/config.lua | 3 + lua/opencode/opencode_server.lua | 193 ++++++++++++------------------- lua/opencode/server_job.lua | 44 ++++++- tests/unit/server_job_spec.lua | 11 +- 4 files changed, 127 insertions(+), 124 deletions(-) diff --git a/lua/opencode/config.lua b/lua/opencode/config.lua index 21f5a1e4..556cf50c 100644 --- a/lua/opencode/config.lua +++ b/lua/opencode/config.lua @@ -197,6 +197,9 @@ M.defaults = { capture_streamed_events = false, show_ids = true, }, + server = { + port = 41096, -- Default high port for opencode server + }, prompt_guard = nil, hooks = { on_file_edited = nil, diff --git a/lua/opencode/opencode_server.lua b/lua/opencode/opencode_server.lua index a098d112..336cbba3 100644 --- a/lua/opencode/opencode_server.lua +++ b/lua/opencode/opencode_server.lua @@ -15,8 +15,7 @@ OpencodeServer.__index = OpencodeServer local current_pid = vim.fn.getpid() -local FLOCK_TIMEOUT_MS = 5000 -local FLOCK_RETRY_INTERVAL_MS = 50 +local DEFAULT_PORT = 41096 --- @return string local function get_lock_file_path() @@ -24,59 +23,6 @@ local function get_lock_file_path() return vim.fs.joinpath(tmpdir --[[@as string]], 'opencode-server.lock') end ---- @return string -local function get_flock_path() - return get_lock_file_path() .. '.flock' -end - ---- @return boolean acquired -local function acquire_file_lock() - local flock_path = get_flock_path() - local start_time = vim.uv.now() - - while (vim.uv.now() - start_time) < FLOCK_TIMEOUT_MS do - local fd = vim.uv.fs_open(flock_path, 'wx', 384) - if fd then - vim.uv.fs_write(fd, tostring(current_pid)) - vim.uv.fs_close(fd) - return true - end - - local stat = vim.uv.fs_stat(flock_path) - if stat then - local age_ms = (vim.uv.now() - stat.mtime.sec * 1000) - if age_ms > FLOCK_TIMEOUT_MS then - os.remove(flock_path) - end - end - - vim.wait(FLOCK_RETRY_INTERVAL_MS, function() - return false - end) - end - - return false -end - -local function release_file_lock() - local flock_path = get_flock_path() - os.remove(flock_path) -end - ---- @param fn function ---- @return any -local function with_file_lock(fn) - if not acquire_file_lock() then - return nil - end - local ok, result = pcall(fn) - release_file_lock() - if not ok then - error(result) - end - return result -end - --- @param pid number --- @return boolean local function is_pid_alive(pid) @@ -171,60 +117,52 @@ end --- @param url string --- @param server_pid number local function create_lock_file(url, server_pid) - with_file_lock(function() - write_lock_file({ - url = url, - clients = { current_pid }, - server_pid = server_pid, - }) - end) + write_lock_file({ + url = url, + clients = { current_pid }, + server_pid = server_pid, + }) end --- @return boolean success local function register_client() - local result = with_file_lock(function() - local data = read_lock_file() - if not data then - return false - end + local data = read_lock_file() + if not data then + return false + end - data = cleanup_dead_pids(data) + data = cleanup_dead_pids(data) - local already_registered = vim.tbl_contains(data.clients, current_pid) - if not already_registered then - table.insert(data.clients, current_pid) - end + local already_registered = vim.tbl_contains(data.clients, current_pid) + if not already_registered then + table.insert(data.clients, current_pid) + end - write_lock_file(data) - return true - end) - return result or false + write_lock_file(data) + return true end --- @return number|nil server_pid_to_kill local function unregister_client() - local result = with_file_lock(function() - local data = read_lock_file() - if not data then - return nil - end + local data = read_lock_file() + if not data then + return nil + end - data = cleanup_dead_pids(data) + data = cleanup_dead_pids(data) - data.clients = vim.tbl_filter(function(pid) - return pid ~= current_pid - end, data.clients) + data.clients = vim.tbl_filter(function(pid) + return pid ~= current_pid + end, data.clients) - if #data.clients == 0 then - local server_pid = data.server_pid - remove_lock_file() - return server_pid - end + if #data.clients == 0 then + local server_pid = data.server_pid + remove_lock_file() + return server_pid + end - write_lock_file(data) - return nil - end) - return result + write_lock_file(data) + return nil end --- @return OpencodeServer @@ -234,7 +172,8 @@ function OpencodeServer.new() callback = function() local state = require('opencode.state') if state.opencode_server then - state.opencode_server:shutdown() + -- Block and wait for shutdown to complete (max 5 seconds) + state.opencode_server:shutdown():wait(5000) end end, }) @@ -251,27 +190,21 @@ end --- @return string|nil url Returns the server URL if an existing server is available function OpencodeServer.try_existing_server() - local server_url = with_file_lock(function() - local data = read_lock_file() - if not data then - return nil - end - - data = cleanup_dead_pids(data) - - if #data.clients == 0 then - remove_lock_file() - return nil - end + local data = read_lock_file() + if not data then + return nil + end - write_lock_file(data) - return data.url - end) + data = cleanup_dead_pids(data) - if not server_url then + if #data.clients == 0 then + remove_lock_file() return nil end + write_lock_file(data) + local server_url = data.url + local api_client = require('opencode.api_client') local is_healthy = api_client.check_health(server_url, 2000):wait(3000) @@ -279,9 +212,7 @@ function OpencodeServer.try_existing_server() return server_url end - with_file_lock(function() - remove_lock_file() - end) + remove_lock_file() return nil end @@ -308,12 +239,32 @@ function OpencodeServer:shutdown() local server_pid_to_kill = unregister_client() if server_pid_to_kill then - if self.job and self.job.pid then - pcall(function() - self.job:kill('sigterm') - end) + -- Cross-platform process termination + if vim.fn.has('win32') == 1 then + -- Windows: use taskkill + vim.fn.system({ 'taskkill', '/F', '/PID', tostring(server_pid_to_kill) }) else - pcall(vim.uv.kill, server_pid_to_kill, 'sigterm') + -- Unix: send SIGTERM first + if self.job and self.job.pid then + pcall(function() + self.job:kill('sigterm') + end) + else + pcall(vim.uv.kill, server_pid_to_kill, 'sigterm') + end + + -- Wait for process to exit (max 3 seconds) + local start = os.time() + while is_pid_alive(server_pid_to_kill) and (os.time() - start) < 3 do + vim.wait(100, function() + return false + end) + end + + -- Force kill if still running + if is_pid_alive(server_pid_to_kill) then + pcall(vim.uv.kill, server_pid_to_kill, 'sigkill') + end end end @@ -326,6 +277,7 @@ end --- @class OpencodeServerSpawnOpts --- @field cwd? string +--- @field port? number --- @field on_ready fun(job: any, url: string) --- @field on_error fun(err: any) --- @field on_exit fun(exit_opts: vim.SystemCompleted ) @@ -334,12 +286,15 @@ end --- @return Promise function OpencodeServer:spawn(opts) opts = opts or {} + local port = opts.port or DEFAULT_PORT self.is_owner = true self.job = vim.system({ 'opencode', 'serve', + '--port', + tostring(port), }, { cwd = opts.cwd, stdout = function(err, data) diff --git a/lua/opencode/server_job.lua b/lua/opencode/server_job.lua index 124e05e5..d2a1c6ca 100644 --- a/lua/opencode/server_job.lua +++ b/lua/opencode/server_job.lua @@ -2,10 +2,13 @@ local state = require('opencode.state') local curl = require('opencode.curl') local Promise = require('opencode.promise') local opencode_server = require('opencode.opencode_server') +local config = require('opencode.config') local M = {} M.requests = {} +local DEFAULT_PORT = 41096 + --- @param response {status: integer, body: string} --- @param cb fun(err: any, result: any) local function handle_api_response(response, cb) @@ -134,25 +137,58 @@ end --- Ensure the opencode server is running, starting it if necessary. --- @return Promise promise A promise that resolves with the server instance function M.ensure_server() + local port = config.server and config.server.port or DEFAULT_PORT + local server_url = 'http://127.0.0.1:' .. port + local promise = Promise.new() + + -- Fast path: check in-memory server instance if state.opencode_server and state.opencode_server:is_running() then return promise:resolve(state.opencode_server) end - local existing_url = opencode_server.try_existing_server() - if existing_url then - state.opencode_server = opencode_server.from_existing(existing_url) + -- Step 1: Health check to see if server already exists + local api_client = require('opencode.api_client') + local ok, is_healthy = pcall(function() + return api_client.check_health(server_url, 2000):wait(2500) + end) + + if ok and is_healthy then + state.opencode_server = opencode_server.from_existing(server_url) return promise:resolve(state.opencode_server) end + -- Step 2: Try to start the server (port binding is atomic mutex) state.opencode_server = opencode_server.new() state.opencode_server:spawn({ + port = port, on_ready = function(_, base_url) promise:resolve(state.opencode_server) end, on_error = function(err) - promise:reject(err) + -- Retry health check (another instance may have just started successfully) + vim.defer_fn(function() + local retry_ok, retry_healthy = pcall(function() + return api_client.check_health(server_url, 2000):wait(2500) + end) + if retry_ok and retry_healthy then + state.opencode_server = opencode_server.from_existing(server_url) + promise:resolve(state.opencode_server) + else + -- Check if port is occupied by another application + local err_msg = tostring(err) + if err_msg:match('address already in use') or err_msg:match('EADDRINUSE') then + promise:reject(string.format( + "Port %d is occupied by another application. " + .. "Configure a different port via require('opencode').setup({ server = { port = XXXX } })", + port + )) + else + promise:reject(err) + end + end + end, 500) end, on_exit = function(exit_opts) promise:reject('Server exited') diff --git a/tests/unit/server_job_spec.lua b/tests/unit/server_job_spec.lua index 1a0d35fa..041bb5d6 100644 --- a/tests/unit/server_job_spec.lua +++ b/tests/unit/server_job_spec.lua @@ -2,23 +2,28 @@ local server_job = require('opencode.server_job') local curl = require('opencode.curl') local assert = require('luassert') +local api_client = require('opencode.api_client') +local Promise = require('opencode.promise') describe('server_job', function() local original_curl_request local opencode_server = require('opencode.opencode_server') local original_new local original_try_existing_server + local original_check_health before_each(function() original_curl_request = curl.request original_new = opencode_server.new original_try_existing_server = opencode_server.try_existing_server + original_check_health = api_client.check_health end) after_each(function() curl.request = original_curl_request opencode_server.new = original_new opencode_server.try_existing_server = original_try_existing_server + api_client.check_health = original_check_health end) it('exposes expected public functions', function() @@ -85,7 +90,7 @@ describe('server_job', function() it('ensure_server spawns a new opencode server only once', function() local spawn_count = 0 local fake = { - url = 'http://127.0.0.1:4000', + url = 'http://127.0.0.1:41096', is_running = function() return spawn_count > 0 end, @@ -103,6 +108,10 @@ describe('server_job', function() opencode_server.try_existing_server = function() return nil end + -- Mock health check to return false (no existing server) + api_client.check_health = function() + return Promise.new():reject('no server') + end local first = server_job.ensure_server():wait() assert.same(fake, first._value or first) -- ensure_server returns resolved promise value From 1a04da0ed88518577497c04f7d35e9a8a3fc04a5 Mon Sep 17 00:00:00 2001 From: jinzhongjia Date: Thu, 8 Jan 2026 20:59:20 +0800 Subject: [PATCH 11/13] test: replace hardcoded PIDs with relative values --- tests/unit/opencode_server_spec.lua | 25 ++++++++++++++++++------- 1 file changed, 18 insertions(+), 7 deletions(-) diff --git a/tests/unit/opencode_server_spec.lua b/tests/unit/opencode_server_spec.lua index dc326334..5ace2f40 100644 --- a/tests/unit/opencode_server_spec.lua +++ b/tests/unit/opencode_server_spec.lua @@ -254,7 +254,8 @@ describe('opencode.opencode_server', function() it('from_existing creates server with url and registers as client', function() local current_pid = vim.fn.getpid() - write_test_lock_file('http://localhost:9999', { 12345 }, 54321) + -- Use current_pid as the existing client so it's always "alive" + write_test_lock_file('http://localhost:9999', { current_pid }, 54321) vim.uv.kill = function(pid, sig) if sig == 0 then @@ -276,6 +277,12 @@ describe('opencode.opencode_server', function() it('spawn creates lock file with server_pid', function() local current_pid = vim.fn.getpid() + vim.uv.kill = function(pid, sig) + if sig == 0 then + return 0 + end + end + vim.system = function(cmd, opts) vim.schedule(function() opts.stdout(nil, 'opencode server listening on http://127.0.0.1:5555') @@ -310,8 +317,10 @@ describe('opencode.opencode_server', function() it('shutdown removes client from lock file', function() local current_pid = vim.fn.getpid() - write_test_lock_file('http://localhost:7777', { current_pid, 99998 }, 54321) + local other_pid = current_pid + 1000000 -- Use a fake but distinct PID + write_test_lock_file('http://localhost:7777', { current_pid, other_pid }, 54321) + -- Mock to make all PIDs appear alive vim.uv.kill = function(pid, sig) if sig == 0 then return 0 @@ -328,7 +337,7 @@ describe('opencode.opencode_server', function() local lock_data = read_test_lock_file() assert.is_not_nil(lock_data) assert.is_false(vim.tbl_contains(lock_data.clients, current_pid)) - assert.is_true(vim.tbl_contains(lock_data.clients, 99998)) + assert.is_true(vim.tbl_contains(lock_data.clients, other_pid)) end) it('shutdown removes lock file when last client exits', function() @@ -354,10 +363,11 @@ describe('opencode.opencode_server', function() it('shutdown keeps remaining clients when one client exits', function() local current_pid = vim.fn.getpid() - local other_pid_1 = 88881 - local other_pid_2 = 88882 + local other_pid_1 = current_pid + 1000000 -- Use fake but distinct PIDs + local other_pid_2 = current_pid + 2000000 write_test_lock_file('http://localhost:4444', { current_pid, other_pid_1, other_pid_2 }, 54321) + -- Mock to make all PIDs appear alive vim.uv.kill = function(pid, sig) if sig == 0 then return 0 @@ -382,11 +392,12 @@ describe('opencode.opencode_server', function() it('cleanup_dead_pids removes dead processes from clients list', function() local current_pid = vim.fn.getpid() - local dead_pid = 99997 - local alive_pid = 99996 + local dead_pid = current_pid + 1000000 -- Use fake but distinct PIDs + local alive_pid = current_pid + 2000000 write_test_lock_file('http://localhost:3333', { dead_pid, alive_pid, current_pid }, 54321) + -- Mock to make dead_pid appear dead, others alive vim.uv.kill = function(pid, sig) if sig == 0 then if pid == dead_pid then From dc501c861f30adc0229410fca9e36b62994a6a76 Mon Sep 17 00:00:00 2001 From: jinzhongjia Date: Thu, 8 Jan 2026 21:48:44 +0800 Subject: [PATCH 12/13] fix(server): create cache directory if needed --- lua/opencode/opencode_server.lua | 5 +++++ tests/unit/opencode_server_spec.lua | 9 +++++++++ 2 files changed, 14 insertions(+) diff --git a/lua/opencode/opencode_server.lua b/lua/opencode/opencode_server.lua index 336cbba3..a6235b02 100644 --- a/lua/opencode/opencode_server.lua +++ b/lua/opencode/opencode_server.lua @@ -78,6 +78,11 @@ end --- @param data LockFileData local function write_lock_file(data) local lock_path = get_lock_file_path() + -- Ensure cache directory exists + local cache_dir = vim.fn.stdpath('cache') + if cache_dir and vim.fn.isdirectory(cache_dir) == 0 then + vim.fn.mkdir(cache_dir, 'p') + end local f = io.open(lock_path, 'w') if not f then return diff --git a/tests/unit/opencode_server_spec.lua b/tests/unit/opencode_server_spec.lua index 5ace2f40..c07c1435 100644 --- a/tests/unit/opencode_server_spec.lua +++ b/tests/unit/opencode_server_spec.lua @@ -7,6 +7,14 @@ local function get_lock_file_path() return vim.fs.joinpath(tmpdir --[[@as string]], 'opencode-server.lock') end +--- Ensure the cache directory exists (needed for CI environments) +local function ensure_cache_dir() + local cache_dir = vim.fn.stdpath('cache') + if cache_dir and vim.fn.isdirectory(cache_dir) == 0 then + vim.fn.mkdir(cache_dir, 'p') + end +end + --- Helper to write a lock file directly for testing --- @param url string --- @param clients number[] @@ -69,6 +77,7 @@ describe('opencode.opencode_server', function() before_each(function() original_system = vim.system original_uv_kill = vim.uv.kill + ensure_cache_dir() remove_test_lock_file() end) after_each(function() From 9f615e689b4bcb9adad84ff56ce0a67b53d133b2 Mon Sep 17 00:00:00 2001 From: jinzhongjia Date: Fri, 9 Jan 2026 13:06:07 +0800 Subject: [PATCH 13/13] refactor(server): improve shutdown handling with race condition protection Add double-check mechanism to prevent killing server when new clients register during shutdown, and extract process management utilities. --- lua/opencode/opencode_server.lua | 159 ++++++++++++++++++++++------ tests/unit/opencode_server_spec.lua | 12 ++- 2 files changed, 138 insertions(+), 33 deletions(-) diff --git a/lua/opencode/opencode_server.lua b/lua/opencode/opencode_server.lua index a6235b02..59431bfe 100644 --- a/lua/opencode/opencode_server.lua +++ b/lua/opencode/opencode_server.lua @@ -147,11 +147,15 @@ local function register_client() return true end ---- @return number|nil server_pid_to_kill +--- @class UnregisterResult +--- @field server_pid number|nil +--- @field is_last_client boolean + +--- @return UnregisterResult local function unregister_client() local data = read_lock_file() if not data then - return nil + return { server_pid = nil, is_last_client = false } end data = cleanup_dead_pids(data) @@ -160,16 +164,90 @@ local function unregister_client() return pid ~= current_pid end, data.clients) + -- Always write updated lock file (even with empty clients) + -- This allows new clients to register before we finish shutdown + write_lock_file(data) + if #data.clients == 0 then - local server_pid = data.server_pid - remove_lock_file() - return server_pid + return { server_pid = data.server_pid, is_last_client = true } end - write_lock_file(data) + return { server_pid = nil, is_last_client = false } +end + +--- Check if we're still the last client (no new clients registered) +--- @return boolean +local function is_still_last_client() + local data = read_lock_file() + if not data then + return true -- Lock file gone, we can proceed + end + data = cleanup_dead_pids(data) + return #data.clients == 0 +end + +--- Find the PID of a process listening on the given port +--- @param port number|string +--- @return number|nil +local function find_pid_by_port(port) + if vim.fn.has('win32') == 1 then + -- Windows: use netstat + local handle = io.popen('netstat -ano | findstr :' .. port .. ' | findstr LISTENING 2>nul') + if handle then + local output = handle:read('*a') + handle:close() + return tonumber(output:match('%s(%d+)%s*$')) + end + else + -- Unix: use lsof + local handle = io.popen('lsof -ti :' .. port .. ' 2>/dev/null') + if handle then + local output = handle:read('*a') + handle:close() + return tonumber(output:match('(%d+)')) + end + end return nil end +--- Kill a process by PID with graceful shutdown +--- @param pid number +--- @param job any|nil Optional vim.system job handle +--- @return boolean success +local function kill_process(pid, job) + if not pid or pid <= 0 then + return false + end + + if vim.fn.has('win32') == 1 then + vim.fn.system({ 'taskkill', '/F', '/PID', tostring(pid) }) + else + -- Unix: send SIGTERM first + if job and job.pid then + pcall(function() + job:kill('sigterm') + end) + else + pcall(vim.uv.kill, pid, 'sigterm') + end + + -- Wait for process to exit (max 3 seconds) + local start = os.time() + while is_pid_alive(pid) and (os.time() - start) < 3 do + vim.wait(100, function() + return false + end) + end + + -- Force kill if still running + if is_pid_alive(pid) then + pcall(vim.uv.kill, pid, 'sigkill') + end + end + + return not is_pid_alive(pid) +end + --- @return OpencodeServer function OpencodeServer.new() vim.api.nvim_create_autocmd('VimLeavePre', { @@ -227,7 +305,21 @@ function OpencodeServer.from_existing(url) local server = OpencodeServer.new() server.url = url server.is_owner = false - register_client() + + -- Try to register with existing lock file + local registered = register_client() + if not registered then + -- No lock file exists - we need to find the server PID + local port = url:match(':(%d+)/?$') + local server_pid = port and find_pid_by_port(port) or nil + + write_lock_file({ + url = url, + clients = { current_pid }, + server_pid = server_pid, + }) + end + server.spawn_promise:resolve(server --[[@as OpencodeServer]]) return server end @@ -241,35 +333,38 @@ end --- @return Promise function OpencodeServer:shutdown() - local server_pid_to_kill = unregister_client() - - if server_pid_to_kill then - -- Cross-platform process termination - if vim.fn.has('win32') == 1 then - -- Windows: use taskkill - vim.fn.system({ 'taskkill', '/F', '/PID', tostring(server_pid_to_kill) }) - else - -- Unix: send SIGTERM first - if self.job and self.job.pid then - pcall(function() - self.job:kill('sigterm') - end) - else - pcall(vim.uv.kill, server_pid_to_kill, 'sigterm') - end + local result = unregister_client() + + if result.is_last_client then + -- Double-check: a new client may have registered while we were processing + if not is_still_last_client() then + -- New client registered, don't kill the server + self.job = nil + self.url = nil + self.handle = nil + self.shutdown_promise:resolve(true --[[@as boolean]]) + return self.shutdown_promise + end - -- Wait for process to exit (max 3 seconds) - local start = os.time() - while is_pid_alive(server_pid_to_kill) and (os.time() - start) < 3 do - vim.wait(100, function() - return false - end) + -- Find the server PID to kill + local server_pid_to_kill = result.server_pid + if not server_pid_to_kill and self.url then + -- Try to find by port if server_pid not recorded + local port = self.url:match(':(%d+)/?$') + if port then + server_pid_to_kill = find_pid_by_port(port) end + end - -- Force kill if still running - if is_pid_alive(server_pid_to_kill) then - pcall(vim.uv.kill, server_pid_to_kill, 'sigkill') + if server_pid_to_kill then + local killed = kill_process(server_pid_to_kill, self.job) + if killed then + remove_lock_file() end + -- If kill failed, leave lock file so next client can try again + else + -- No PID found, just remove lock file + remove_lock_file() end end diff --git a/tests/unit/opencode_server_spec.lua b/tests/unit/opencode_server_spec.lua index c07c1435..45ab5d4c 100644 --- a/tests/unit/opencode_server_spec.lua +++ b/tests/unit/opencode_server_spec.lua @@ -351,10 +351,20 @@ describe('opencode.opencode_server', function() it('shutdown removes lock file when last client exits', function() local current_pid = vim.fn.getpid() - write_test_lock_file('http://localhost:6666', { current_pid }, 54321) + local server_pid = 54321 + write_test_lock_file('http://localhost:6666', { current_pid }, server_pid) + -- Track whether the process has been killed + local process_killed = false vim.uv.kill = function(pid, sig) if sig == 0 then + -- Check if process is alive + if pid == server_pid and process_killed then + return nil, 'ESRCH' -- Process not found (dead) + end + return 0 -- Process is alive + elseif sig == 'sigterm' or sig == 'sigkill' then + process_killed = true return 0 end end