diff --git a/.agents/skills/verify-diff-lifecycle/check.sh b/.agents/skills/verify-diff-lifecycle/check.sh index 418a4ada7..798ad0374 100755 --- a/.agents/skills/verify-diff-lifecycle/check.sh +++ b/.agents/skills/verify-diff-lifecycle/check.sh @@ -197,7 +197,7 @@ fact[ClientTagged]=$(as_bool client_tagged) # the running coroutine, and open_diff_blocking returns that result. setup_answer_returned() { in_order '^ *local waiting = false$' 'pcall\(M\._setup_blocking_diff' '^ *if not waiting then$' \ - '^ *setup_result = result$' '^ *return$' 'coroutine\.resume\(co, result\)' '^ *if setup_result then$' \ + '^ *setup_result = result$' '^ *return$' '^ *local .* = utils\.resume_tool_coroutine\(co, result\)$' '^ *if setup_result then$' \ '^ *return setup_result$' '^ *waiting = true$' 'coroutine\.yield\(\)' <<<"$open_blocking" } fact[SetupAnswerReturned]=$(as_bool setup_answer_returned) diff --git a/.agents/skills/verify-diff-lifecycle/covered.lock b/.agents/skills/verify-diff-lifecycle/covered.lock index 509fcdd5c..560ea3859 100644 --- a/.agents/skills/verify-diff-lifecycle/covered.lock +++ b/.agents/skills/verify-diff-lifecycle/covered.lock @@ -3,11 +3,11 @@ file lua/claudecode/config.lua 3afb1bdeff27bfb6 == lua/claudecode/cwd.lua file lua/claudecode/cwd.lua b034e82c76e8dee4 == lua/claudecode/diff.lua -file lua/claudecode/diff.lua a646d1da839b9f92 +file lua/claudecode/diff.lua 56ed24d0d5b77e5c block resolve_saved 4d34b0c7e34d561f block resolve_rejected c7a620816abff9a1 -block open_blocking 92067df7f9d4d4d9 -block setup_blocking b96c84e0961817fc +block open_blocking 13e23b610cab9f40 +block setup_blocking b45a7bfd5203a085 block close_by_name cc40c0d2ffdf7946 block close_pending 31bc827d1065da70 block close_for_client 330cc91666bc7884 @@ -204,8 +204,8 @@ file lua/claudecode/tools/get_open_editors.lua 96392f05d7af7521 == lua/claudecode/tools/get_workspace_folders.lua file lua/claudecode/tools/get_workspace_folders.lua 80f38bdf62e55c3e == lua/claudecode/tools/init.lua -file lua/claudecode/tools/init.lua 76b2b584a0799874 -block handle_invoke d70cbd96c9ab5655 +file lua/claudecode/tools/init.lua a8ff3ef7cd3c9df9 +block handle_invoke 5c2422dfe7e2ea93 == lua/claudecode/tools/open_diff.lua file lua/claudecode/tools/open_diff.lua d4050352cb09dfb9 block open_diff_tool 1a0e157e8e0a9452 @@ -216,7 +216,7 @@ file lua/claudecode/tools/save_document.lua 584b046bec6cb1f6 == lua/claudecode/types.lua file lua/claudecode/types.lua 69e2907853403759 == lua/claudecode/utils.lua -file lua/claudecode/utils.lua c4a21fe74858afe2 +file lua/claudecode/utils.lua 33fe91b68063b71d == lua/claudecode/visual_commands.lua file lua/claudecode/visual_commands.lua f396dd26cc0c1050 == plugin/claudecode.lua diff --git a/lua/claudecode/README.md b/lua/claudecode/README.md index e6ba8e323..6b46a55d3 100644 --- a/lua/claudecode/README.md +++ b/lua/claudecode/README.md @@ -94,7 +94,7 @@ Module map (direct files): - `visual_commands.lua`: `create_visual_command_wrapper`, `exit_visual_and_schedule`, `validate_visual_mode`, `get_visual_range`, `get_tree_state`, `capture_visual_selection_data`, and `get_files_from_visual_selection` [visual_commands.lua:M.create_visual_command_wrapper] - `visual_commands.get_tree_state()` knows only neo-tree (when its window is current), nvim-tree, oil, and netrw. It returns `nil` for `minifiles` and `snacks_picker_list` [visual_commands.lua:M.get_tree_state] - `cwd.lua`: `resolve(term_cfg, ctx)` and `git_root(start_dir)` [cwd.lua:M.resolve]. Nothing in `lua/` calls `cwd.resolve`. `terminal.lua:build_config` has its own copy of the cwd order and calls only `cwd.git_root`, so a change to `resolve` does not change the terminal [terminal.lua:build_config] -- `utils.lua`: `shell_split`, `parse_command`, `expand_tilde`, `normalize_focus`, `selection_type`, `utf8_sequence_length`, `valid_utf8`, `map_terminal_suspend`, `win_set_buf_without_bufenter`, and `when_windows_allowed` [utils.lua:M.parse_command] [utils.lua:M.valid_utf8] [utils.lua:M.map_terminal_suspend] [utils.lua:M.win_set_buf_without_bufenter] [utils.lua:M.when_windows_allowed] +- `utils.lua`: `shell_split`, `parse_command`, `expand_tilde`, `normalize_focus`, `selection_type`, `utf8_sequence_length`, `valid_utf8`, `map_terminal_suspend`, `win_set_buf_without_bufenter`, `when_windows_allowed`, and `resume_tool_coroutine` [utils.lua:M.parse_command] [utils.lua:M.valid_utf8] [utils.lua:M.map_terminal_suspend] [utils.lua:M.win_set_buf_without_bufenter] [utils.lua:M.when_windows_allowed] [utils.lua:M.resume_tool_coroutine] - `logger.lua`: `levels`, `setup`, `error`, `warn`, `info`, `one_line`, `debug`, `trace`, and `is_level_enabled` [logger.lua:M.setup] - `health.lua`: `check()`. It checks, in order: Neovim 0.8.0 or later, `require("claudecode")`, that `setup()` ran, the CLI (the first word of `terminal_cmd`, or `claude`) and its `--version`, the terminal provider, then the server, its lock file (present, missing, or there but not readable), and a connected client. It stops after a failed load, a missing `setup()`, or a stopped server [health.lua:M.check] [health.lua:check_cli] [health.lua:check_server] diff --git a/lua/claudecode/diff.lua b/lua/claudecode/diff.lua index 2f1287680..3bdc491a3 100644 --- a/lua/claudecode/diff.lua +++ b/lua/claudecode/diff.lua @@ -1964,13 +1964,6 @@ function M._setup_blocking_diff(params, resolution_callback) end end - -- A raising handler of the user (autocmd callback) makes the cleanup above fail inside Neovim API calls. A LuaJIT - -- older than September 2023 (bundled by Neovim 0.8 and 0.9) then keeps pointing at the main Lua thread, while this - -- code runs in the coroutine of tools.handle_invoke. The next JIT trace exit corrupts the stack of the main thread - -- and Neovim aborts or crashes, exit 134 or 139 (LuaJIT issue #1066). An API call that returns normally points - -- LuaJIT at the running coroutine again. Make one before the error leaves the coroutine. - vim.api.nvim_get_current_tabpage() - -- Re-throw the error for MCP compliance error({ code = -32000, @@ -2049,7 +2042,7 @@ function M.open_diff_blocking(old_file_path, new_file_path, new_file_contents, t return end -- Resume the coroutine with the result - local resume_success, resume_result, resume_err = coroutine.resume(co, result) + local resume_success, resume_result, resume_err = utils.resume_tool_coroutine(co, result) if not resume_success then logger.error("diff", "Coroutine failed:", tostring(resume_result)) end diff --git a/lua/claudecode/tools/README.md b/lua/claudecode/tools/README.md index 589948657..477ed35a5 100644 --- a/lua/claudecode/tools/README.md +++ b/lua/claudecode/tools/README.md @@ -15,7 +15,7 @@ - `M.register_all()`: registers 10 exposed tools and the internal `close_tab` [tools/init.lua:M.register_all] - `M.get_tool_list()`: returns `{ name, description, inputSchema }` only for tools that have a `schema`. It walks `M.tools` with `pairs()`, so the order is not defined [tools/init.lua:M.get_tool_list] - `M.handle_invoke(client, params)`: looks up `params.name` and runs the tool with `params.arguments`, or with `{}` when the call has none. It expects the params shape that the server's `tools/call` handler checked [tools/init.lua:M.handle_invoke] -- `M.build_response(ok, value, err)`: maps a handler outcome, as `pcall` or `coroutine.resume` returns it, to `{ result }` or `{ error }`. `handle_invoke` uses it, and so does `diff.open_diff_blocking` when it resumes a deferred tool [tools/init.lua:M.build_response] +- `M.build_response(ok, value, err)`: maps a handler outcome, as `pcall` or `utils.resume_tool_coroutine` returns it, to `{ result }` or `{ error }`. `handle_invoke` uses it, and so does `diff.open_diff_blocking` when it resumes a deferred tool [tools/init.lua:M.build_response] ### Registered tools @@ -42,6 +42,7 @@ Sources: [tools/init.lua:M.register_all], [tools/close_tab.lua:schema], [tools/o - A successful handler returns `{ result = }` [tools/init.lua:M.handle_invoke]. Each tool handler builds the MCP shape `{ content = { { type = "text", text = ... } } }` itself [tools/open_file.lua:handler]. `openDiff` returns the value of `diff.open_diff_blocking` unchanged, and `claudecode.diff` builds that shape, or `claudecode.diff_inline` for the unified layout [tools/open_diff.lua:handler] [diff.lua:M._resolve_diff_as_saved] - A call without `arguments` runs the tool with `{}`. A tool with a required parameter then raises `-32602`, and `getDiagnostics` returns the diagnostics of all buffers. No handler enforces `additionalProperties = false` [tools/init.lua:M.handle_invoke] [tools/get_diagnostics.lua:handler] - Tools with `requires_coroutine` run in `coroutine.create`. If the coroutine yields, `handle_invoke` returns `{ _deferred = true, coroutine, client, params }` and sends no response yet. Only `openDiff` sets `requires_coroutine = true` [tools/init.lua:M.handle_invoke] +- `handle_invoke` and the answer to a deferred `openDiff` resume the tool's coroutine through `utils.resume_tool_coroutine`. On a LuaJIT build older than 2023-09-21 (the release builds of Neovim 0.8 and 0.9 are such builds), or one whose `jit.version` the plugin cannot read, it turns the JIT off and flushes the compiled traces before it resumes the coroutine, and it gives the JIT back as it was when the resume returns. This guards against LuaJIT issue 1066, which aborts Neovim. On those builds each such resume discards the compiled traces of the whole editor, which LuaJIT compiles again when the code gets hot. Any other LuaJIT build gets a plain `coroutine.resume` [utils.lua:M.resume_tool_coroutine] [tools/init.lua:M.handle_invoke] [diff.lua:M.open_diff_blocking] - `openDiff` raises `-32602` when a required parameter is missing or is not a string. Outside a non-main coroutine it raises `-32000` with `message = "Internal server error"` and `data = "openDiff must run in coroutine context"`. It raises `-32000` with `data = "Failed to load diff module"` when `claudecode.diff` does not load [tools/open_diff.lua:handler] - After a deferred `openDiff` yields, `diff.open_diff_blocking` maps the outcome with `build_response` and sends it through `_G.claude_deferred_responses[tostring(co)]`, then clears that entry [diff.lua:M.open_diff_blocking] - `openDiff` calls `diff.open_diff_blocking(old, new, contents, tab_name, client.id)`. Structured errors pass through. Other errors become `-32000` "Error opening blocking diff" [tools/open_diff.lua:handler] diff --git a/lua/claudecode/tools/init.lua b/lua/claudecode/tools/init.lua index 18904626f..1d834917a 100644 --- a/lua/claudecode/tools/init.lua +++ b/lua/claudecode/tools/init.lua @@ -1,6 +1,8 @@ -- Tool implementation for Claude Code Neovim integration local M = {} +local utils = require("claudecode.utils") + M.ERROR_CODES = { PARSE_ERROR = -32700, INVALID_REQUEST = -32600, @@ -110,7 +112,7 @@ function M.handle_invoke(client, params) -- client needed for blocking tools end) require("claudecode.logger").debug("tools", "About to resume coroutine for " .. tool_name) - local success, result, err = coroutine.resume(co) + local success, result, err = utils.resume_tool_coroutine(co) require("claudecode.logger").debug( "tools", "Coroutine resume returned - success:", diff --git a/lua/claudecode/utils.lua b/lua/claudecode/utils.lua index fd9d8285e..56df06025 100644 --- a/lua/claudecode/utils.lua +++ b/lua/claudecode/utils.lua @@ -277,4 +277,37 @@ function M.valid_utf8(text) return table.concat(parts) end +-- Neovim 0.8 and 0.9 bundle a LuaJIT older than its commit e86990f7f of 2023-09-21, which fixes LuaJIT issue 1066: +-- after a C function runs Lua on another thread and then raises into a pcall, `cur_L` still names that thread, and +-- the next exit from a compiled trace crashes (the issue shows a crash in `lj_trace_exit`). The commit sets `cur_L` +-- again in `err_unwind` (src/lj_err.c). A Neovim API call in a coroutine that runs a Lua callback and then raises +-- behaves like that C function, and Neovim aborts (exit 134 or 139). We did not read the LuaJIT source for other code +-- that uses `cur_L`. Measured: with the JIT off and its traces flushed while the coroutine runs, Neovim survives +-- (tests/integration/diff_tool_coroutine_jit_spec.lua). Rolling releases are numbered 2.1.. +-- A version that is no string, or does not read that way, counts as affected, which costs only a flush. +local jit_affected = jit ~= nil and (tonumber(tostring(jit.version):match("^LuaJIT 2%.1%.(%d+)$")) or 0) < 1695261248 + +---Turns the JIT back on when `was_on`, and passes the other arguments on as they are, trailing nils included. +local function restore_jit(was_on, ...) + if was_on then + jit.on() + end + return ... +end + +---`coroutine.resume` for the coroutine of a tool (`handle_invoke` and the resolution of a blocking diff). On a LuaJIT +---with issue 1066 it turns the JIT off and flushes its traces before the resume, and gives the JIT back as it was. +---@param co thread +---@param ... any Values for the coroutine +---@return boolean success +---@return any ... What `coroutine.resume` returns +function M.resume_tool_coroutine(co, ...) + local was_on = jit_affected and jit.status() + if jit_affected then + jit.off() + jit.flush() + end + return restore_jit(was_on, coroutine.resume(co, ...)) +end + return M diff --git a/tests/integration/diff_failed_setup_guards_spec.lua b/tests/integration/diff_failed_setup_guards_spec.lua index eedc21713..ee16e738f 100644 --- a/tests/integration/diff_failed_setup_guards_spec.lua +++ b/tests/integration/diff_failed_setup_guards_spec.lua @@ -143,12 +143,14 @@ describe("openDiff when the setup fails in a new tab", function() closes_the_new_tab_and_goes_back(a_BufLeave_handler_raises_during_the_cleanup) end) - -- A LuaJIT older than September 2023 (bundled by Neovim 0.8 and 0.9) has a bug (LuaJIT issue #1066). When a Neovim - -- API call raises inside a coroutine because a handler it ran raised, LuaJIT keeps pointing at the main Lua thread, - -- and the next JIT trace exit corrupts the stack of that thread. Neovim then aborts (exit 134, "bad stack slot") or - -- crashes (exit 139), and plenary prints no summary. openDiff runs in a coroutine (tools.handle_invoke). On Neovim - -- 0.8.0 the three cases above hit the bug in a few of 100 runs. A JIT that compiles at once makes the passes below - -- hit it in most runs, so this case fails on Neovim 0.8 and 0.9 when diff.lua leaves the bug in place. + -- A LuaJIT older than 2023-09-21 (the release builds of Neovim 0.8 and 0.9) has a bug (LuaJIT issue #1066): after a + -- C function runs Lua on another thread and then raises into a pcall, the next exit from a compiled trace crashes. A + -- Neovim API call that raises inside a coroutine because a handler it ran raised is that case. Neovim then aborts + -- (exit 134, "bad stack slot") or crashes (exit 139), and plenary prints no summary. openDiff runs in a coroutine + -- (tools.handle_invoke). On such a LuaJIT, utils.resume_tool_coroutine turns the JIT off and flushes its traces + -- before it resumes that coroutine. Without that guard, the three cases above ended with a plenary summary in 100 of + -- 100 runs each on v0.8.0 and on v0.9.5, so they do not show the bug. A JIT that compiles at once makes the passes + -- below hit it in most runs, so this case fails on Neovim 0.8 and 0.9 when the guard is missing. it("keeps Neovim running when handlers raise on repeated failed setups under an eager JIT", function() if jit then jit.opt.start("hotloop=1", "hotexit=1", "tryside=1") diff --git a/tests/integration/diff_tool_coroutine_jit_spec.lua b/tests/integration/diff_tool_coroutine_jit_spec.lua new file mode 100644 index 000000000..a35e1933a --- /dev/null +++ b/tests/integration/diff_tool_coroutine_jit_spec.lua @@ -0,0 +1,190 @@ +-- Real-Neovim guards for the JIT around the coroutine of a tool. A LuaJIT older than 2023-09-21 (the release builds of +-- Neovim 0.8 and 0.9) has a bug (LuaJIT issue #1066): after a C function runs Lua on another thread and then raises +-- into a pcall, the next exit from a compiled trace crashes. A Neovim API call that raises inside a coroutine because a +-- handler it ran raised is that case, and Neovim aborts or crashes. The first case reads the state of the JIT, so its +-- result does not depend on a failure rate. The second case keeps a JIT that the user turned off off. The third case +-- raises in handlers under an eager JIT, as diff_failed_setup_guards_spec.lua does, for setups that succeed. Under +-- busted's mock vim this is pending. scripts/run_integration_tests_individually.sh runs it in Neovim, including CI's +-- oldest supported version. +if vim == nil or vim.fn.has("nvim") ~= 1 then + describe("openDiff and the JIT of a LuaJIT with issue 1066", function() + pending("needs a real Neovim") + end) + return +end + +describe("openDiff and the JIT of a LuaJIT with issue 1066", function() + local diff = require("claudecode.diff") + local tools = require("claudecode.tools.init") + local tab_name = "✻ [Claude Code] proposal ⧉" + local group = "diff_tool_coroutine_jit_spec" + -- Rolling releases of LuaJIT 2.1 are numbered with the unix time of their commit. 1695261248 is the time of commit + -- e86990f7f, the fix for the bug. The plugin makes the same decision, and this spec does not read it from the plugin. + local affected = jit ~= nil and (tonumber(jit.version:match("^LuaJIT 2%.1%.(%d+)$")) or 0) < 1695261248 + local jit_was_on, real_resume + local dir, path + + local function has_compiled_trace() + for number = 1, 100 do + if require("jit.util").traceinfo(number) then + return true + end + end + return false + end + + local function compile_a_trace() + local sum = 0 + for index = 1, 1000 do + sum = sum + index % 7 + end + return sum + end + + local function observe() + return { on = (jit.status()), trace = has_compiled_trace() } + end + + local function assert_the_guard_state(observed, what) + assert.equal(not affected, observed.on, what .. ": the JIT is on unless the LuaJIT has the bug") + if affected then + assert.is_false(observed.trace, what .. ": no compiled trace exists") + end + end + + local function open_diff() + local call = tools.handle_invoke({ id = "tool_coroutine_jit_spec" }, { + name = "openDiff", + arguments = { + old_file_path = path, + new_file_path = path, + new_file_contents = "alpha\nBETA\n", + tab_name = tab_name, + }, + }) + if call._deferred then + _G.claude_deferred_responses[tostring(call.coroutine)] = function() end + end + return call + end + + local function accept_the_diff() + vim.api.nvim_set_current_win(diff._get_active_diffs()[tab_name].new_window) + vim.cmd("write") + end + + local function reset() + pcall(vim.api.nvim_del_augroup_by_name, group) + diff._cleanup_all_active_diffs("test cleanup") + vim.cmd("silent! tabonly!") + vim.cmd("silent! only!") + vim.cmd("silent! %bwipeout!") + end + + before_each(function() + vim.o.columns, vim.o.lines = 160, 40 + dir = vim.fn.tempname() + vim.fn.mkdir(dir, "p") + path = dir .. "/a.txt" + vim.fn.writefile({ "alpha", "beta" }, path) + vim.cmd("edit " .. vim.fn.fnameescape(path)) + vim.api.nvim_create_augroup(group, { clear = true }) + diff.setup(vim.deepcopy(require("claudecode.config").defaults)) + tools.register_all() + _G.claude_deferred_responses = {} + jit_was_on, real_resume = jit.status(), coroutine.resume + end) + + after_each(function() + coroutine.resume = real_resume -- luacheck: ignore 122 + reset() + vim.fn.delete(dir, "rf") + _G.claude_deferred_responses = nil + jit.opt.start("hotloop=56", "hotexit=10", "tryside=4") -- LuaJIT's defaults + if jit_was_on then + jit.on() + end + end) + + it("runs the coroutine of a tool with the JIT off and no compiled trace, and turns the JIT on again", function() + compile_a_trace() + assert.is_true(jit.status(), "the JIT is on before the call") + assert.is_true(has_compiled_trace(), "a compiled trace exists before the call") + local in_handlers, in_resumes = {}, {} + vim.api.nvim_create_autocmd({ "WinNew", "WinLeave", "BufWinEnter" }, { + group = group, + callback = function() + table.insert(in_handlers, observe()) + end, + }) + coroutine.resume = function(co, ...) -- luacheck: ignore 122 + in_resumes[co] = in_resumes[co] or {} + table.insert(in_resumes[co], observe()) + return real_resume(co, ...) + end + + local call = open_diff() + assert.is_true(call._deferred, "openDiff waits for the user") + assert.is_true(jit.status(), "the JIT is on again after the call") + accept_the_diff() + assert.is_true(jit.status(), "the JIT is on again after the answer") + + assert.is_true(#in_handlers > 0, "a handler ran while the diff opened") + for _, observed in ipairs(in_handlers) do + assert_the_guard_state(observed, "in a handler") + end + assert.equal(2, #in_resumes[call.coroutine], "the coroutine resumes at the call and at the answer") + for _, observed in ipairs(in_resumes[call.coroutine]) do + assert_the_guard_state(observed, "at a resume") + end + end) + + it("keeps a JIT that was off off", function() + jit.off() + local seen = {} + vim.api.nvim_create_autocmd("WinNew", { + group = group, + callback = function() + table.insert(seen, observe()) + end, + }) + + local call = open_diff() + assert.is_true(call._deferred, "openDiff waits for the user") + assert.is_false(jit.status(), "the JIT is off after the call") + accept_the_diff() + assert.is_false(jit.status(), "the JIT is off after the answer") + + assert.is_true(#seen > 0, "a handler ran while the diff opened") + for _, observed in ipairs(seen) do + assert.is_false(observed.on) + if affected then + assert.is_false(observed.trace) + end + end + end) + + -- A second openDiff of the same tab first closes the windows of the first diff, in its own coroutine. A handler that + -- raises then is caught by a pcall of the plugin, and both calls open a diff. Without the guard this case does not + -- abort Neovim (the eager case of diff_failed_setup_guards_spec.lua does), so it shows no bug. It shows that the guard + -- leaves the answers of openDiff as they are. + it("keeps Neovim running when handlers raise on repeated setups that succeed under an eager JIT", function() + jit.opt.start("hotloop=1", "hotexit=1", "tryside=1") + local raised = 0 + for _ = 1, 6 do + vim.cmd("edit " .. vim.fn.fnameescape(path)) + vim.api.nvim_create_augroup(group, { clear = true }) + vim.api.nvim_create_autocmd({ "WinClosed", "BufWinLeave", "BufHidden" }, { + group = group, + callback = function() + raised = raised + 1 + error("a handler of the user raised") + end, + }) + assert.is_true(open_diff()._deferred, "the first openDiff waits for the user") + assert.is_true(open_diff()._deferred, "the second openDiff replaces the first and waits for the user") + reset() + end + assert.is_true(raised > 0, "a handler raised") + end) +end) diff --git a/tests/unit/tool_coroutine_resume_spec.lua b/tests/unit/tool_coroutine_resume_spec.lua new file mode 100644 index 000000000..2ccb9dd96 --- /dev/null +++ b/tests/unit/tool_coroutine_resume_spec.lua @@ -0,0 +1,59 @@ +require("tests.busted_setup") +local diff = require("claudecode.diff") +local temp_dir = require("tests.helpers.temp_dir") +local tools = require("claudecode.tools.init") +local utils = require("claudecode.utils") + +-- Every resume of a tool's coroutine goes through utils.resume_tool_coroutine, which keeps the JIT off while the +-- coroutine runs on a LuaJIT with issue 1066. +describe("resuming the coroutine of a tool", function() + local real_resume, resumes, dir + + before_each(function() + real_resume, resumes, dir = utils.resume_tool_coroutine, {}, temp_dir.new() + utils.resume_tool_coroutine = function(co, ...) + table.insert(resumes, { co = co, first = (...) }) + return real_resume(co, ...) + end + end) + + after_each(function() + utils.resume_tool_coroutine = real_resume + tools.tools.waiting_probe = nil + diff._cleanup_all_active_diffs("test cleanup") + temp_dir.remove(dir) + end) + + it("goes through the helper when handle_invoke runs the tool", function() + tools.tools.waiting_probe = { + handler = function() + coroutine.yield() + end, + requires_coroutine = true, + } + + local call = tools.handle_invoke(nil, { name = "waiting_probe" }) + + assert.is_true(call._deferred) + assert.same({ { co = call.coroutine } }, resumes) + end) + + it("goes through the helper when a blocking diff is answered", function() + local file = dir .. "/resume.txt" + local handle = assert(io.open(file, "w")) + handle:write("line 1\n") + handle:close() + local co = coroutine.create(function() + diff.open_diff_blocking(file, file, "line 1\nline 2\n", "resume tab", "client") + end) + assert.is_true(coroutine.resume(co)) + assert.equal("suspended", coroutine.status(co)) + resumes = {} + + diff.close_all_diffs("test") + + assert.equal(1, #resumes) + assert.equal(co, resumes[1].co) + assert.equal("DIFF_REJECTED", resumes[1].first.content[1].text) + end) +end) diff --git a/tests/unit/utils_resume_tool_coroutine_spec.lua b/tests/unit/utils_resume_tool_coroutine_spec.lua new file mode 100644 index 000000000..e2e6f2aba --- /dev/null +++ b/tests/unit/utils_resume_tool_coroutine_spec.lua @@ -0,0 +1,153 @@ +require("tests.busted_setup") + +-- The check for LuaJIT issue 1066 runs when the module loads, so each case loads the module again under a stubbed +-- global `jit` that records what the helper does to the JIT. +describe("claudecode.utils.resume_tool_coroutine", function() + local real_jit, real_utils + local calls, jit_on, jit_on_during_resume + + local function pack(...) + return { n = select("#", ...), ... } + end + + local function load_utils(version, on_before) + calls, jit_on = {}, on_before + local stub = { + version = version, + status = function() + return jit_on + end, + off = function() + table.insert(calls, "off") + jit_on = false + end, + flush = function() + table.insert(calls, "flush") + end, + on = function() + table.insert(calls, "on") + jit_on = true + end, + } + _G.jit = stub + package.loaded["claudecode.utils"] = nil + return require("claudecode.utils") + end + + -- A tool's coroutine: it records that it ran and whether the JIT was on then, then gives back its arguments. + local function create_coroutine() + return coroutine.create(function(...) + table.insert(calls, "resume") + jit_on_during_resume = jit and jit.status() + return ... + end) + end + + before_each(function() + real_jit, real_utils = _G.jit, package.loaded["claudecode.utils"] + jit_on_during_resume = nil + end) + + after_each(function() + _G.jit, package.loaded["claudecode.utils"] = real_jit, real_utils + end) + + it("resumes with a plain coroutine.resume when there is no jit global", function() + _G.jit = nil + package.loaded["claudecode.utils"] = nil + calls = {} + local utils = require("claudecode.utils") + + assert.same({ true, "a", "b" }, { utils.resume_tool_coroutine(create_coroutine(), "a", "b") }) + assert.same({ "resume" }, calls) + end) + + for _, version in ipairs({ "LuaJIT 2.1.1695261248", "LuaJIT 2.1.1774638290", "LuaJIT 2.1.9999999999" }) do + it("leaves the JIT alone on " .. version, function() + local utils = load_utils(version, true) + + assert.same({ true, "a" }, { utils.resume_tool_coroutine(create_coroutine(), "a") }) + assert.same({ "resume" }, calls) + assert.is_true(jit_on_during_resume) + assert.is_true(jit_on) + end) + end + + for _, version in ipairs({ "LuaJIT 2.1.0-beta3", "LuaJIT 2.1.1692716794", "LuaJIT 2.1.1695261247" }) do + it("turns the JIT off, flushes its traces and turns it on again on " .. version, function() + local utils = load_utils(version, true) + + assert.same({ true, "a" }, { utils.resume_tool_coroutine(create_coroutine(), "a") }) + assert.same({ "off", "flush", "resume", "on" }, calls) + assert.is_false(jit_on_during_resume) + assert.is_true(jit_on) + end) + end + + for _, version in ipairs({ "LuaJIT 2.0.5", "LuaJIT", "unknown", "LuaJIT 2.1.1695261248-custom", "" }) do + it("treats the version string '" .. version .. "' as affected", function() + local utils = load_utils(version, true) + + assert.same({ true }, { utils.resume_tool_coroutine(create_coroutine()) }) + assert.same({ "off", "flush", "resume", "on" }, calls) + assert.is_false(jit_on_during_resume) + end) + end + + for _, case in ipairs({ { "no version" }, { "a number as the version", 42 }, { "a table as the version", {} } }) do + it("treats " .. case[1] .. " as affected", function() + local utils = load_utils(case[2], true) + + assert.same({ true }, { utils.resume_tool_coroutine(create_coroutine()) }) + assert.same({ "off", "flush", "resume", "on" }, calls) + assert.is_false(jit_on_during_resume) + end) + end + + it("flushes the traces but keeps the JIT off when the JIT was off before", function() + local utils = load_utils("LuaJIT 2.1.0-beta3", false) + + assert.same({ true }, { utils.resume_tool_coroutine(create_coroutine()) }) + assert.same({ "off", "flush", "resume" }, calls) + assert.is_false(jit_on_during_resume) + assert.is_false(jit_on) + end) + + it("gives back what the coroutine returns and takes, trailing nils included", function() + for _, version in ipairs({ "LuaJIT 2.1.0-beta3", "LuaJIT 2.1.1774638290" }) do + local utils = load_utils(version, true) + + local echoed = pack(utils.resume_tool_coroutine(create_coroutine(), 1, nil, 3, nil)) + assert.same({ n = 5, true, 1, nil, 3 }, echoed, version) + + local nothing = pack(utils.resume_tool_coroutine(coroutine.create(function() end))) + assert.same({ n = 1, true }, nothing, version) + end + end) + + it("turns the JIT on again when the coroutine raises, and gives back the error", function() + local utils = load_utils("LuaJIT 2.1.0-beta3", true) + + local success, err = utils.resume_tool_coroutine(coroutine.create(function() + error("a tool raised", 0) + end)) + + assert.is_false(success) + assert.equal("a tool raised", err) + assert.same({ "off", "flush", "on" }, calls) + assert.is_true(jit_on) + end) + + it("guards each resume of a coroutine that yields", function() + local utils = load_utils("LuaJIT 2.1.0-beta3", true) + local co = coroutine.create(function() + local answer = coroutine.yield("waiting") + return answer + end) + + assert.same({ true, "waiting" }, { utils.resume_tool_coroutine(co) }) + assert.is_true(jit_on, "the JIT is on again while the coroutine waits") + assert.same({ true, "accepted" }, { utils.resume_tool_coroutine(co, "accepted") }) + assert.same({ "off", "flush", "on", "off", "flush", "on" }, calls) + end) +end)