From 2a4c2146cdd9735ffd1ec9cee84d3311c0d28d6e Mon Sep 17 00:00:00 2001 From: Thomas Kosiewski Date: Thu, 12 Nov 2026 22:30:46 +0000 Subject: [PATCH 01/14] fix(runner): die by SIGINT on Ctrl-C even when the cleanup fails MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The INT trap ran `cleanup_current_file` under `set -e`. When its `rm -rf` exited non-zero (a folder that was not empty), bash ended the trap there: the runner exited with 1 before `kill -INT $$`. A calling shell goes on after a child that exited, so its loop ran the next file and an interrupted run looked green. This is the failure of main's post-merge Tests run 43299040669 ("dies by SIGINT on Ctrl-C", status 0 for 130). Before #568 the subshell made the exit-status file again during the cleanup, which made `rm -rf` fail that way. Run the cleanup as `cleanup_current_file || true`, so that `set -e` is off for all of it and the trap always reaches the kill. A stub rm that exits 1 after it removed everything makes the new test fail on the old runner on every run (status 0, `ran=1 ran=2`). Also put the status, the leftovers and the output of the runner and of the calling shell into the message of each signal test, so that the next failure explains itself, and mark the test of the status file's descriptor pending where /proc//fd does not exist, because its stubs would list nothing there and the test would pass for any runner. _Generated with [`xum`](https://github.com/coder/xum) • Model: `anthropic:claude-sonnet-5-5` • Thinking: `xhigh`_ --- scripts/README.md | 2 +- scripts/run_integration_tests_individually.sh | 5 +- tests/unit/integration_runner_spec.lua | 52 +++++++++++++++---- 3 files changed, 47 insertions(+), 12 deletions(-) diff --git a/scripts/README.md b/scripts/README.md index 6e61114e..0de86ce2 100644 --- a/scripts/README.md +++ b/scripts/README.md @@ -84,7 +84,7 @@ The Lua repros run from the repository root with `nvim --headless -u NONE -l scr - CAUTION: Do not run the token-sending clients (`repro_issue_238.sh`, `repro_issue_248.sh`, and `repro_issue_277.sh`) on a shared machine. They pass the auth token on the `websocat` command line (`--header`), where other users can read it with `ps` [repro_issue_238.sh:WEBSOCAT] [repro_issue_248.sh:TOKEN] [repro_issue_277.sh:TOKEN]. - `repro_issue_238.sh`, `repro_issue_248.sh`, and `repro_issue_277.sh` connect to the plugin server on `127.0.0.1` [repro_issue_238.sh:PORT] [repro_issue_248.sh:PORT] [repro_issue_277.sh:PORT]. -- `run_integration_tests_individually.sh` creates two `mktemp` files and two `mktemp -d` folders per test (the output, the file that holds the process id of the file's `timeout`, and the `XDG_STATE_HOME` and `XDG_CONFIG_HOME` that the file's two Neovims share) and deletes all four after the file ran. The runner opens a status file in the state folder before it starts the file, and the file's subshell writes the exit statuses of nvim and of `tee` to that open file when the file ends. A subshell that ends while the cleanup removes the folder opens no path, so it leaves nothing behind. When the runner cannot read the file back, for example because the Neovim deleted the state folder, it uses the exit status of the subshell, which is the status of nvim, or of `tee` when nvim exited 0, and names it as the status of nvim. When it gets SIGTERM, SIGHUP or SIGINT (Ctrl-C) while a file runs, it kills that `timeout`, then the process group of the file, deletes the four and dies by that signal at once, so a calling shell sees status 143, 129 or 130, and a shell loop around the runner stops after Ctrl-C. Killing the `timeout` first covers the moments before it has made its own group, when no Neovim runs yet. Both kills use KILL, so a Neovim that ignores SIGTERM stops too. Two windows of a few milliseconds are not covered. (1) While the runner makes its four items it has not stored their names yet: a signal then leaves the items it already made, never deletes them, and starts no Neovim. (2) Between the moment the runner stores the names and the moment the file's shell has written the process id, there is no `timeout` to kill. A SIGTERM or SIGHUP sent to the runner alone leaves that shell running, and so does any SIGINT (Ctrl-C in a shell, for one), because a shell that runs in the background ignores SIGINT. The Neovim then starts and runs until its own timeout, and two of the four come back after the runner deleted them: the process id file, which the file's shell writes again, and the state folder, which the Neovim makes again. A SIGTERM or SIGHUP sent to the whole process group stops that shell too, so nothing starts and nothing is left. The same holds when SIGTERM, SIGHUP, SIGINT or SIGUSR1 kills a runner that leads its own session on a terminal, for example one that a terminal emulator starts directly: its death hangs up the foreground process group of the terminal, so that shell dies too. SIGKILL gives the runner no time to clean up: a runner that leads such a session and gets it starts no Neovim, but all four items stay, and without a terminal the Neovim starts as well. Bash ignores SIGQUIT, so the file runs to its end. It sets `VIMINIT`, `CLAUDECODE_MINIMAL_INIT`, `XDG_STATE_HOME` and `XDG_CONFIG_HOME` for that file's Neovims and the processes they start only, as a prefix of one command, and leaves `XDG_DATA_HOME` alone because plenary lives there. A run writes nothing under `~/.local/state/nvim`, and the developer's `ftplugin` and `after` files under `~/.config/nvim` do not run [run_integration_tests_individually.sh:temp_output, group_file, state_dir, config_dir]. +- `run_integration_tests_individually.sh` creates two `mktemp` files and two `mktemp -d` folders per test (the output, the file that holds the process id of the file's `timeout`, and the `XDG_STATE_HOME` and `XDG_CONFIG_HOME` that the file's two Neovims share) and deletes all four after the file ran. The runner opens a status file in the state folder before it starts the file, and the file's subshell writes the exit statuses of nvim and of `tee` to that open file when the file ends. A subshell that ends while the cleanup removes the folder opens no path, so it leaves nothing behind. When the runner cannot read the file back, for example because the Neovim deleted the state folder, it uses the exit status of the subshell, which is the status of nvim, or of `tee` when nvim exited 0, and names it as the status of nvim. When it gets SIGTERM, SIGHUP or SIGINT (Ctrl-C) while a file runs, it kills that `timeout`, then the process group of the file, deletes the four and dies by that signal at once, even when a delete fails, so a calling shell sees status 143, 129 or 130, and a shell loop around the runner stops after Ctrl-C. Killing the `timeout` first covers the moments before it has made its own group, when no Neovim runs yet. Both kills use KILL, so a Neovim that ignores SIGTERM stops too. Two windows of a few milliseconds are not covered. (1) While the runner makes its four items it has not stored their names yet: a signal then leaves the items it already made, never deletes them, and starts no Neovim. (2) Between the moment the runner stores the names and the moment the file's shell has written the process id, there is no `timeout` to kill. A SIGTERM or SIGHUP sent to the runner alone leaves that shell running, and so does any SIGINT (Ctrl-C in a shell, for one), because a shell that runs in the background ignores SIGINT. The Neovim then starts and runs until its own timeout, and two of the four come back after the runner deleted them: the process id file, which the file's shell writes again, and the state folder, which the Neovim makes again. A SIGTERM or SIGHUP sent to the whole process group stops that shell too, so nothing starts and nothing is left. The same holds when SIGTERM, SIGHUP, SIGINT or SIGUSR1 kills a runner that leads its own session on a terminal, for example one that a terminal emulator starts directly: its death hangs up the foreground process group of the terminal, so that shell dies too. SIGKILL gives the runner no time to clean up: a runner that leads such a session and gets it starts no Neovim, but all four items stay, and without a terminal the Neovim starts as well. Bash ignores SIGQUIT, so the file runs to its end. It sets `VIMINIT`, `CLAUDECODE_MINIMAL_INIT`, `XDG_STATE_HOME` and `XDG_CONFIG_HOME` for that file's Neovims and the processes they start only, as a prefix of one command, and leaves `XDG_DATA_HOME` alone because plenary lives there. A run writes nothing under `~/.local/state/nvim`, and the developer's `ftplugin` and `after` files under `~/.config/nvim` do not run [run_integration_tests_individually.sh:temp_output, group_file, state_dir, config_dir]. - `repro_issue_231.lua` deletes `/__issue_231_repro__.md` [repro_issue_231.lua:try_open_diff]. - `repro_issue_70.sh` and `repro_issue_277.sh` make a work dir with `mktemp -d` (`${TMPDIR:-/tmp}/issue70.XXXXXX` or `issue277.XXXXXX`) and an `att-home` folder in it before they check their tools. They install `trap cleanup EXIT` after that check, so an exit 3 for a missing tool leaves the work dir behind. The trap also runs on SIGTERM, SIGINT and SIGHUP. It destroys the `agent-tty` sessions and removes the work dir. agent-tty still leaves an empty socket folder `/tmp/agent-tty/<8 hex digits>` (it ignores `TMPDIR`) and an entry for the deleted home in `${XDG_STATE_HOME:-$HOME/.local/state}/agent-tty/homes.json`, one of each per run. The sessions inherit the caller's environment, so `CLAUDE_CONFIG_DIR` and the `XDG_*` variables reach the Neovim and the `claude` CLI that they run. `repro_issue_70.sh` launches the real `claude` CLI, and it also runs `claude --version` once outside agent-tty. Its cleanup runs `pkill -f "repro_issue_70_probe.lua $REPO_ROOT"`, which also kills the probe of another run in the same checkout [repro_issue_70.sh:WORK, cleanup] [repro_issue_277.sh:WORK, cleanup]. - `repro_issue_70_probe.lua` starts the real server with `auto_start = true`, so the plugin writes a real `PORT.lock` into the user's lock directory. Other Claude CLIs can see that lock file until `claudecode.stop()` removes it at the end. When the removal fails, `stop` only logs a warning, so a lock file that the removal failed to delete stays [repro_issue_70_probe.lua:claudecode.setup, claudecode.stop] [lua/claudecode/init.lua:M.stop]. diff --git a/scripts/run_integration_tests_individually.sh b/scripts/run_integration_tests_individually.sh index b8c8d2a2..6addb438 100755 --- a/scripts/run_integration_tests_individually.sh +++ b/scripts/run_integration_tests_individually.sh @@ -33,8 +33,9 @@ trap cleanup_current_file EXIT # A Ctrl-C reaches the runner's own process group, and timeout leaves that group, so the file runs in the background and # `wait` below returns at once when this trap runs. The trap cleans up, then kills the runner with SIGINT, not with # `exit 130`: a calling shell stops its own loop after a child that died by SIGINT, and goes on after one that exited -# 130. -trap 'cleanup_current_file; trap - EXIT INT; kill -INT $$; exit 130' INT +# 130. A cleanup that fails, such as an rm that cannot remove a folder, must not end the trap early either: with set -e +# the runner would exit with 1 before the kill, and the calling shell would go on. +trap 'cleanup_current_file || true; trap - EXIT INT; kill -INT $$; exit 130' INT echo "=== Running Integration Tests Individually ===" diff --git a/tests/unit/integration_runner_spec.lua b/tests/unit/integration_runner_spec.lua index fbb7d989..2aaa65dc 100644 --- a/tests/unit/integration_runner_spec.lua +++ b/tests/unit/integration_runner_spec.lua @@ -119,6 +119,13 @@ local function shell(command) assert(status == 0 or status == true, "failed: " .. command) end +-- The stubs below list the files they hold open from /proc//fd. Where it does not exist they list nothing, and a +-- runner that keeps a descriptor open would pass the test that reads their notes. +local has_proc_fd = (function() + local status = os.execute("test -d /proc/self/fd") + return status == 0 or status == true +end)() + -- In some locales a bash regex bracket range such as [0-9] also matches digits like the fullwidth １. ---@return string|nil locale A common locale name in which it does, or nil when the host has none local function find_wide_digit_locale() @@ -167,7 +174,8 @@ local STUB_MKTEMP = { local rm_pipe = assert(io.popen("command -v rm 2> /dev/null")) local real_rm = assert(rm_pipe:read("*l"), "needs rm") rm_pipe:close() --- Notes in $STUB_DIR/rm.holds_status_file that it holds the status file open. Runs the real rm, except when +-- Notes in $STUB_DIR/rm.holds_status_file that it holds the status file open. Runs the real rm and then exits with 1 +-- when $STUB_DIR/rm.fail exists, as rm does when a folder is not empty. Runs the real rm, except when -- $STUB_DIR/rm.slow exists: then it walks the way rm -rf does, but slowly. It deletes the -- files, waits the seconds in that file, and only then removes the folders, so a file that something makes meanwhile in -- a folder stops the removal of the folder ("Directory not empty"), as it does for a slow rm -rf under strace. @@ -176,6 +184,7 @@ local STUB_RM = { "for fd in /proc/$$/fd/*; do", ' case "$(readlink "$fd" 2> /dev/null)" in */exit_statuses*) : > "$STUB_DIR/rm.holds_status_file" ;; esac', "done", + 'if [ -f "$STUB_DIR/rm.fail" ]; then ' .. quote(real_rm) .. ' "$@"; exit 1; fi', 'if [ ! -f "$STUB_DIR/rm.slow" ]; then exec ' .. quote(real_rm) .. ' "$@"; fi', "shift", 'for path in "$@"; do find "$path" -type f -delete; done', @@ -502,12 +511,14 @@ describe("scripts/run_integration_tests_individually.sh", function() ---@field caller_loop boolean|nil Run the runner twice in a calling shell and send the signal to their process group ---@field late_tee number|nil The file's tee ends this many seconds after its input ends ---@field slow_rm number|nil The runner's rm waits this many seconds between deleting files and removing folders + ---@field rm_fails boolean|nil The runner's rm removes what it is told to and then exits with 1 ---@param signal string The signal to send ---@param run SignalRun ---@return table result `status` of the runner (of the calling shell with `caller_loop`), `left` in its TMPDIR once --- its Neovim's grandchild would have written, `ended` when the file's Neovim got to the end of its 2 second run, - --- `runner_output` (stdout and stderr of the runner), and `caller_output` of the calling shell + --- `runner_output` (stdout and stderr of the runner), `caller_output` of the calling shell, and `report`, one line + --- with all of these for the message of an assertion local function signal_runner(signal, run) write(dir .. "/tests/integration/slow_spec.lua", {}) write(dir .. "/slow_spec.lua.out", summary(1, 0, 0)) @@ -525,6 +536,9 @@ describe("scripts/run_integration_tests_individually.sh", function() if run.slow_rm then write(dir .. "/rm.slow", { tostring(run.slow_rm) }) end + if run.rm_fails then + write(dir .. "/rm.fail", {}) + end local temp = dir .. "/tmp" shell("mkdir -p " .. quote(temp)) local runner_out = " > " .. quote(dir .. "/runner.out") .. " 2>&1" @@ -575,12 +589,20 @@ describe("scripts/run_integration_tests_individually.sh", function() if ended then ended:close() end + local status = output:match("status=(%d+)") or output return { - status = output:match("status=(%d+)") or output, + status = status, left = left, ended = ended ~= nil, caller_output = caller_output, runner_output = runner_output, + report = string.format( + "status %s, left %q, caller output %q, runner output %q", + status, + left, + caller_output, + runner_output + ), } end @@ -590,7 +612,7 @@ describe("scripts/run_integration_tests_individually.sh", function() it(description .. name .. " at once, and exits " .. expected_status, function() local result = signal_runner(signal, {}) - assert.are.equal(expected_status, result.status) + assert.are.equal(expected_status, result.status, result.report) assert.are.equal("", result.left, "the runner left temp files behind, or its Neovim's grandchild ran on") assert.is_false(result.ended, "the runner waited for the file to end, it did not stop it") assert.is_nil( @@ -603,7 +625,7 @@ describe("scripts/run_integration_tests_individually.sh", function() it("stops a file whose Neovim ignores SIGTERM when the runner gets SIGTERM", function() local result = signal_runner("TERM", { deaf_to_term = true }) - assert.are.equal("143", result.status) + assert.are.equal("143", result.status, result.report) assert.are.equal("", result.left, "a Neovim that ignores SIGTERM ran on after the runner died") assert.is_false(result.ended) end) @@ -611,7 +633,7 @@ describe("scripts/run_integration_tests_individually.sh", function() it("leaves no exit status file when the file's subshell ends after the runner cleaned up", function() local result = signal_runner("TERM", { late_tee = 1 }) - assert.are.equal("143", result.status) + assert.are.equal("143", result.status, result.report) assert.are.equal("", result.left, "the file's subshell wrote its exit statuses after the runner deleted its items") assert.is_nil(result.runner_output:find("No such file", 1, true), result.runner_output) end) @@ -620,12 +642,15 @@ describe("scripts/run_integration_tests_individually.sh", function() -- The file's tee ends half a second after the signal, while the stub rm waits between its files and its folders. local result = signal_runner("TERM", { late_tee = 0.5, slow_rm = 1.5 }) - assert.are.equal("143", result.status) + assert.are.equal("143", result.status, result.report) assert.are.equal("", result.left, "something made a file in a folder that the cleanup was removing") assert.is_nil(result.runner_output:find("Directory not empty", 1, true), result.runner_output) end) it("closes the status file for nvim and for tee, which only the file's subshell needs", function() + if not has_proc_fd then + pending("needs /proc//fd to see which files a process holds open") + end local status, output = run_script({ { name = "a_spec.lua", exit = 0, output = summary(1, 0, 0) }, }) @@ -674,7 +699,7 @@ describe("scripts/run_integration_tests_individually.sh", function() it("stops a file on SIGTERM that arrives after timeout started and before it made its process group", function() local result = signal_runner("TERM", { window = true }) - assert.are.equal("143", result.status) + assert.are.equal("143", result.status, result.report) assert.are.equal("", result.left, "the runner left temp files or folders behind") assert.is_nil(io.open(dir .. "/slow_spec.lua.started"), "the file's Neovim started after the runner died") end) @@ -682,7 +707,16 @@ describe("scripts/run_integration_tests_individually.sh", function() it("dies by SIGINT on Ctrl-C, so a shell that calls it in a loop stops the loop", function() local result = signal_runner("INT", { caller_loop = true }) - assert.are.equal("130", result.status) + assert.are.equal("130", result.status, result.report) + assert.are.equal("", result.caller_output, "the calling shell went on after the runner got Ctrl-C") + assert.are.equal("", result.left, "the runner left temp files or folders behind") + assert.is_false(result.ended) + end) + + it("dies by SIGINT on Ctrl-C when its cleanup fails, so a shell that calls it in a loop stops the loop", function() + local result = signal_runner("INT", { caller_loop = true, rm_fails = true }) + + assert.are.equal("130", result.status, result.report) assert.are.equal("", result.caller_output, "the calling shell went on after the runner got Ctrl-C") assert.are.equal("", result.left, "the runner left temp files or folders behind") assert.is_false(result.ended) From 732c9aa98c5645a67242fcd8edde5c25b8823d81 Mon Sep 17 00:00:00 2001 From: Thomas Kosiewski Date: Thu, 12 Nov 2026 22:53:29 +0000 Subject: [PATCH 02/14] fix(scripts): start no Neovim and leave no temp item after a signal in the runner's start-up windows MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two windows in scripts/run_integration_tests_individually.sh survived Ctrl-C and SIGTERM. Both were known and listed in scripts/README.md. 1. The runner made four temp items with four mktemp calls, then stored their names. A signal in between left up to four items that no cleanup knew. The runner now picks the name of one folder with `mktemp -u`, stores it, and only then makes the folder with mkdir. Every item lives in that folder. mkdir fails when the name exists, and the runner then stores nothing, so it removes only a folder that it made. 2. The shell that execs `timeout` wrote its process id to a file that the cleanup reads. A signal before that write left the cleanup with no process to kill, and the shell then started a Neovim that ran on after the runner died. The cleanup now makes `cancel` in the folder before it reads the process id. The shell writes its id first, then looks for the folder and for `cancel`, and exits when it finds `cancel` or no folder. One of the two always sees the other, so no Neovim starts that the cleanup cannot find. Each fix has tests that fail on the runner of 2a4c2146 and do not depend on timing. A mktemp stub and a mkdir stub send SIGINT right after they made something. A BASH_ENV file stops the shell before its first command until the test lets it go, after the runner died or while its rm waits with the folder still there. Another test checks that the runner removes no folder that it did not make. The test of the status file opens it in a folder that a mkdir stub removed, as a mktemp stub did before. _Generated with [`xum`](https://github.com/coder/xum) • Model: `anthropic:claude-sonnet-5-5` • Thinking: `xhigh`_ --- scripts/README.md | 2 +- scripts/run_integration_tests_individually.sh | 55 +++++--- tests/unit/integration_runner_spec.lua | 125 ++++++++++++++++-- 3 files changed, 151 insertions(+), 31 deletions(-) diff --git a/scripts/README.md b/scripts/README.md index 0de86ce2..bcb8189e 100644 --- a/scripts/README.md +++ b/scripts/README.md @@ -84,7 +84,7 @@ The Lua repros run from the repository root with `nvim --headless -u NONE -l scr - CAUTION: Do not run the token-sending clients (`repro_issue_238.sh`, `repro_issue_248.sh`, and `repro_issue_277.sh`) on a shared machine. They pass the auth token on the `websocat` command line (`--header`), where other users can read it with `ps` [repro_issue_238.sh:WEBSOCAT] [repro_issue_248.sh:TOKEN] [repro_issue_277.sh:TOKEN]. - `repro_issue_238.sh`, `repro_issue_248.sh`, and `repro_issue_277.sh` connect to the plugin server on `127.0.0.1` [repro_issue_238.sh:PORT] [repro_issue_248.sh:PORT] [repro_issue_277.sh:PORT]. -- `run_integration_tests_individually.sh` creates two `mktemp` files and two `mktemp -d` folders per test (the output, the file that holds the process id of the file's `timeout`, and the `XDG_STATE_HOME` and `XDG_CONFIG_HOME` that the file's two Neovims share) and deletes all four after the file ran. The runner opens a status file in the state folder before it starts the file, and the file's subshell writes the exit statuses of nvim and of `tee` to that open file when the file ends. A subshell that ends while the cleanup removes the folder opens no path, so it leaves nothing behind. When the runner cannot read the file back, for example because the Neovim deleted the state folder, it uses the exit status of the subshell, which is the status of nvim, or of `tee` when nvim exited 0, and names it as the status of nvim. When it gets SIGTERM, SIGHUP or SIGINT (Ctrl-C) while a file runs, it kills that `timeout`, then the process group of the file, deletes the four and dies by that signal at once, even when a delete fails, so a calling shell sees status 143, 129 or 130, and a shell loop around the runner stops after Ctrl-C. Killing the `timeout` first covers the moments before it has made its own group, when no Neovim runs yet. Both kills use KILL, so a Neovim that ignores SIGTERM stops too. Two windows of a few milliseconds are not covered. (1) While the runner makes its four items it has not stored their names yet: a signal then leaves the items it already made, never deletes them, and starts no Neovim. (2) Between the moment the runner stores the names and the moment the file's shell has written the process id, there is no `timeout` to kill. A SIGTERM or SIGHUP sent to the runner alone leaves that shell running, and so does any SIGINT (Ctrl-C in a shell, for one), because a shell that runs in the background ignores SIGINT. The Neovim then starts and runs until its own timeout, and two of the four come back after the runner deleted them: the process id file, which the file's shell writes again, and the state folder, which the Neovim makes again. A SIGTERM or SIGHUP sent to the whole process group stops that shell too, so nothing starts and nothing is left. The same holds when SIGTERM, SIGHUP, SIGINT or SIGUSR1 kills a runner that leads its own session on a terminal, for example one that a terminal emulator starts directly: its death hangs up the foreground process group of the terminal, so that shell dies too. SIGKILL gives the runner no time to clean up: a runner that leads such a session and gets it starts no Neovim, but all four items stay, and without a terminal the Neovim starts as well. Bash ignores SIGQUIT, so the file runs to its end. It sets `VIMINIT`, `CLAUDECODE_MINIMAL_INIT`, `XDG_STATE_HOME` and `XDG_CONFIG_HOME` for that file's Neovims and the processes they start only, as a prefix of one command, and leaves `XDG_DATA_HOME` alone because plenary lives there. A run writes nothing under `~/.local/state/nvim`, and the developer's `ftplugin` and `after` files under `~/.config/nvim` do not run [run_integration_tests_individually.sh:temp_output, group_file, state_dir, config_dir]. +- `run_integration_tests_individually.sh` creates one temp folder per test, and puts the output, the file that holds the process id of the file's `timeout`, and the `XDG_STATE_HOME` and `XDG_CONFIG_HOME` that the file's two Neovims share in it, and deletes the folder after the file ran. The runner stores the name of the folder (from `mktemp -u`) before it makes the folder, so a signal never finds a folder that the runner does not know, and `mkdir` fails when the name exists, so the runner removes nothing that it did not make. The runner opens a status file in the state folder before it starts the file, and the file's subshell writes the exit statuses of nvim and of `tee` to that open file when the file ends. A subshell that ends while the cleanup removes the folder opens no path, so it leaves nothing behind. When the runner cannot read the file back, for example because the Neovim deleted the state folder, it uses the exit status of the subshell, which is the status of nvim, or of `tee` when nvim exited 0, and names it as the status of nvim. When it gets SIGTERM, SIGHUP or SIGINT (Ctrl-C) while a file runs, it makes `cancel` in the folder, kills that `timeout`, then the process group of the file, deletes the folder and dies by that signal at once, even when a delete fails, so a calling shell sees status 143, 129 or 130, and a shell loop around the runner stops after Ctrl-C. Killing the `timeout` first covers the moments before it has made its own group, when no Neovim runs yet. Both kills use KILL, so a Neovim that ignores SIGTERM stops too. The shell of the file writes its process id to the file in the folder, then looks for the folder and for `cancel`, and it starts no Neovim when it finds `cancel` or no folder. The runner makes `cancel` before it reads the process id, so one of the two sees the other: a signal at any moment leaves no Neovim that the runner cannot find. SIGKILL gives the runner no time to clean up, so the folder stays. Bash ignores SIGQUIT, so the file runs to its end. It sets `VIMINIT`, `CLAUDECODE_MINIMAL_INIT`, `XDG_STATE_HOME` and `XDG_CONFIG_HOME` for that file's Neovims and the processes they start only, as a prefix of one command, and leaves `XDG_DATA_HOME` alone because plenary lives there. A run writes nothing under `~/.local/state/nvim`, and the developer's `ftplugin` and `after` files under `~/.config/nvim` do not run [run_integration_tests_individually.sh:work_dir, temp_output, group_file, state_dir, config_dir]. - `repro_issue_231.lua` deletes `/__issue_231_repro__.md` [repro_issue_231.lua:try_open_diff]. - `repro_issue_70.sh` and `repro_issue_277.sh` make a work dir with `mktemp -d` (`${TMPDIR:-/tmp}/issue70.XXXXXX` or `issue277.XXXXXX`) and an `att-home` folder in it before they check their tools. They install `trap cleanup EXIT` after that check, so an exit 3 for a missing tool leaves the work dir behind. The trap also runs on SIGTERM, SIGINT and SIGHUP. It destroys the `agent-tty` sessions and removes the work dir. agent-tty still leaves an empty socket folder `/tmp/agent-tty/<8 hex digits>` (it ignores `TMPDIR`) and an entry for the deleted home in `${XDG_STATE_HOME:-$HOME/.local/state}/agent-tty/homes.json`, one of each per run. The sessions inherit the caller's environment, so `CLAUDE_CONFIG_DIR` and the `XDG_*` variables reach the Neovim and the `claude` CLI that they run. `repro_issue_70.sh` launches the real `claude` CLI, and it also runs `claude --version` once outside agent-tty. Its cleanup runs `pkill -f "repro_issue_70_probe.lua $REPO_ROOT"`, which also kills the probe of another run in the same checkout [repro_issue_70.sh:WORK, cleanup] [repro_issue_277.sh:WORK, cleanup]. - `repro_issue_70_probe.lua` starts the real server with `auto_start = true`, so the plugin writes a real `PORT.lock` into the user's lock directory. Other Claude CLIs can see that lock file until `claudecode.stop()` removes it at the end. When the removal fails, `stop` only logs a warning, so a lock file that the removal failed to delete stays [repro_issue_70_probe.lua:claudecode.setup, claudecode.stop] [lua/claudecode/init.lua:M.stop]. diff --git a/scripts/run_integration_tests_individually.sh b/scripts/run_integration_tests_individually.sh index 6addb438..75d1e136 100755 --- a/scripts/run_integration_tests_individually.sh +++ b/scripts/run_integration_tests_individually.sh @@ -12,22 +12,30 @@ set -o pipefail # selection_held_spec.lua sleeps 21.6s in 77 vim.wait calls, so 30s left each of them 0.1s of slack under load. FILE_TIMEOUT=40s -# The temp files and folders of the file that runs, and the file that holds the process id of its `timeout` while it -# runs. The cleanup stops the file's Neovims before it removes the folders, because one that ends later writes ShaDa and -# makes its state folder again. It kills `timeout` itself first: until timeout has made its own group, no Neovim runs -# yet. It uses KILL for both, because with `timeout` dead its -k grace never starts, and a Neovim that ignores TERM -# would run on. SIGTERM and SIGHUP kill bash, which runs the EXIT trap first. -CURRENT_TEMP=() +# The temp folder of the file that runs, and the file in it that holds the process id of its `timeout` while it runs. The +# cleanup stops the file's Neovims before it removes the folder, because one that ends later writes ShaDa and makes its +# state folder again. It kills `timeout` itself first: until timeout has made its own group, no Neovim runs yet. It +# uses KILL for both, because with `timeout` dead its -k grace never starts, and a Neovim that ignores TERM would run +# on. SIGTERM and SIGHUP kill bash, which runs the EXIT trap first. +# The cleanup makes `cancel` in the folder before it reads the process id file, and the shell that starts the Neovim +# writes its process id to that file before it looks for `cancel` and for the folder (see below). So one of them sees +# the other: a shell that wrote its id too late for the cleanup finds `cancel` or no folder, and starts no Neovim. +CURRENT_WORK="" CURRENT_GROUP_FILE="" # shellcheck disable=SC2329 # The two traps below run it. cleanup_current_file() { local timeout_pid + if [[ -n $CURRENT_WORK ]]; then + : >"$CURRENT_WORK/cancel" 2>/dev/null || true + fi if [[ -s $CURRENT_GROUP_FILE ]]; then timeout_pid=$(cat "$CURRENT_GROUP_FILE") kill -KILL "$timeout_pid" 2>/dev/null || true kill -KILL -- "-$timeout_pid" 2>/dev/null || true fi - rm -rf "${CURRENT_TEMP[@]}" + if [[ -n $CURRENT_WORK ]]; then + rm -rf "$CURRENT_WORK" + fi } trap cleanup_current_file EXIT # A Ctrl-C reaches the runner's own process group, and timeout leaves that group, so the file runs in the background and @@ -62,13 +70,22 @@ run_test_file() { echo "" echo "Running: $basename" - local temp_output group_file state_dir config_dir status_file - temp_output=$(mktemp) - group_file=$(mktemp) - state_dir=$(mktemp -d) - config_dir=$(mktemp -d) + # All temp items of the file live in one folder. The runner picks the name of the folder and stores it before it makes + # the folder, so that no signal finds a folder that the runner made but does not know yet. `mktemp -u` makes nothing + # and prints a random name. mkdir fails when that name exists, and the runner then removes nothing. + local work_dir temp_output group_file state_dir config_dir status_file + work_dir=$(mktemp -u) + CURRENT_WORK=$work_dir + mkdir -m 700 "$work_dir" || { + CURRENT_WORK="" + false + } + temp_output=$work_dir/output + group_file=$work_dir/group + state_dir=$work_dir/state + config_dir=$work_dir/config status_file=$state_dir/exit_statuses - CURRENT_TEMP=("$temp_output" "$group_file" "$state_dir" "$config_dir") + mkdir -m 700 "$state_dir" "$config_dir" CURRENT_GROUP_FILE=$group_file exec 9>"$status_file" @@ -89,13 +106,17 @@ run_test_file() { # being removed. The subshell closes descriptor 9 for nvim and `tee`, which do not need it. The number is fixed, # because bash 3.2 (the macOS /bin/bash) does not know a `{name}>` redirection: it fails the `exec`, and with an EXIT # trap set it then exits 0 without running a spec. + # The shell that execs `timeout` writes its process id first, then looks for the folder and for `cancel`, and it exits + # when it writes nothing, finds no folder, or finds `cancel`. A signal that stopped the runner before that shell wrote + # its id has made `cancel` or removed the folder, so no Neovim starts that the cleanup cannot find. ( set +o pipefail set +e VIMINIT='lua local f = vim.env.CLAUDECODE_MINIMAL_INIT if f then dofile(f) end' \ CLAUDECODE_MINIMAL_INIT="$PWD/tests/minimal_init.lua" \ XDG_STATE_HOME="$state_dir" XDG_CONFIG_HOME="$config_dir" \ - bash -c 'echo $$ > "$1"; shift; exec timeout -k 5s "$@"' _ "$group_file" "$FILE_TIMEOUT" \ + bash -c 'echo $$ 2> /dev/null > "$1" && [ -d "$2" ] && [ ! -e "$2/cancel" ] || exit 1; shift 2; exec timeout -k 5s "$@"' \ + _ "$group_file" "$work_dir" "$FILE_TIMEOUT" \ nvim --headless -i NONE -u tests/minimal_init.lua \ -c "lua require('plenary.test_harness').test_file('$test_file')" \ 9>&- 2>&1 | tee "$temp_output" 9>&- @@ -159,7 +180,7 @@ run_test_file() { # nvim busy in a Lua loop ignores SIGTERM and outlives timeout, and when the nvim started here died, the one that # plenary started for the spec runs on. if [[ $EXIT_CODE -ne 0 ]]; then - kill -KILL -- "-$(cat "$group_file")" 2>/dev/null || true + kill -KILL -- "-$(cat "$group_file" 2>/dev/null)" 2>/dev/null || true fi # A file whose nvim exited non-zero gets a line with a reason: the no-summary line, the TIMEOUT line, or this one. A @@ -177,8 +198,8 @@ run_test_file() { fi # Cleanup - rm -rf "$temp_output" "$group_file" "$state_dir" "$config_dir" - CURRENT_TEMP=() + rm -rf "$work_dir" + CURRENT_WORK="" } for test_file in tests/integration/*_spec.lua; do diff --git a/tests/unit/integration_runner_spec.lua b/tests/unit/integration_runner_spec.lua index 2aaa65dc..78f6e196 100644 --- a/tests/unit/integration_runner_spec.lua +++ b/tests/unit/integration_runner_spec.lua @@ -161,20 +161,54 @@ local STUB_TEE = { local mktemp_pipe = assert(io.popen("command -v mktemp 2> /dev/null")) local real_mktemp = assert(mktemp_pipe:read("*l"), "needs mktemp") mktemp_pipe:close() --- Runs the real mktemp, except when $STUB_DIR/mktemp.drop_dirs exists: then mktemp -d prints the name of a folder --- that it made and removed again, so the folder does not exist. +-- Runs the real mktemp. When $STUB_DIR/mktemp.interrupt exists, its first call sends SIGINT to its process group, which +-- has the runner in it, after the real mktemp ran and before it prints the name. When $STUB_DIR/mktemp.name exists, +-- it makes nothing and prints the name in that file. local STUB_MKTEMP = { "#!/bin/sh", - 'if [ ! -f "$STUB_DIR/mktemp.drop_dirs" ]; then exec ' .. quote(real_mktemp) .. ' "$@"; fi', + 'if [ -f "$STUB_DIR/mktemp.name" ]; then cat "$STUB_DIR/mktemp.name"; exit 0; fi', "path=$(" .. quote(real_mktemp) .. ' "$@") || exit $?', - 'case "$1" in -d) rmdir "$path" ;; esac', + 'if [ -f "$STUB_DIR/mktemp.interrupt" ] && [ ! -f "$STUB_DIR/mktemp.interrupted" ]; then', + ' : > "$STUB_DIR/mktemp.interrupted"', + ' trap "" INT', + " kill -s INT 0", + "fi", 'echo "$path"', } +local mkdir_pipe = assert(io.popen("command -v mkdir 2> /dev/null")) +local real_mkdir = assert(mkdir_pipe:read("*l"), "needs mkdir") +mkdir_pipe:close() +-- Runs the real mkdir. When $STUB_DIR/mkdir.drop_state exists, it then removes each folder named state that it made, so +-- that the folder does not exist when the runner opens the status file in it. When $STUB_DIR/mkdir.interrupt exists, +-- its first call sends SIGINT to its process group, which has the runner in it, after the real mkdir ran. +local STUB_MKDIR = { + "#!/bin/sh", + quote(real_mkdir) .. ' "$@" || exit $?', + 'if [ -f "$STUB_DIR/mkdir.drop_state" ]; then', + ' for path in "$@"; do case "$path" in */state) rmdir "$path" ;; esac; done', + "fi", + 'if [ -f "$STUB_DIR/mkdir.interrupt" ] && [ ! -f "$STUB_DIR/mkdir.interrupted" ]; then', + ' : > "$STUB_DIR/mkdir.interrupted"', + ' trap "" INT', + " kill -s INT 0", + "fi", +} + +-- Stops the shell that execs timeout before its first command, as long as $STUB_DIR/a.release does not exist. Bash runs +-- this file first in each non-interactive shell when BASH_ENV names it, and only that shell has the name _ for $0. +local HOLD_SHELL = { + 'if [ "$0" = _ ]; then', + ' : > "$STUB_DIR/a.held"', + ' while [ ! -e "$STUB_DIR/a.release" ]; do sleep 0.05; done', + "fi", +} + local rm_pipe = assert(io.popen("command -v rm 2> /dev/null")) local real_rm = assert(rm_pipe:read("*l"), "needs rm") rm_pipe:close() --- Notes in $STUB_DIR/rm.holds_status_file that it holds the status file open. Runs the real rm and then exits with 1 +-- Notes in $STUB_DIR/rm.holds_status_file that it holds the status file open. Notes in $STUB_DIR/rm.held and waits for +-- $STUB_DIR/rm.release when $STUB_DIR/rm.hold exists. Runs the real rm and then exits with 1 -- when $STUB_DIR/rm.fail exists, as rm does when a folder is not empty. Runs the real rm, except when -- $STUB_DIR/rm.slow exists: then it walks the way rm -rf does, but slowly. It deletes the -- files, waits the seconds in that file, and only then removes the folders, so a file that something makes meanwhile in @@ -184,6 +218,7 @@ local STUB_RM = { "for fd in /proc/$$/fd/*; do", ' case "$(readlink "$fd" 2> /dev/null)" in */exit_statuses*) : > "$STUB_DIR/rm.holds_status_file" ;; esac', "done", + 'if [ -f "$STUB_DIR/rm.hold" ]; then : > "$STUB_DIR/rm.held"; while [ ! -e "$STUB_DIR/rm.release" ]; do sleep 0.05; done; fi', 'if [ -f "$STUB_DIR/rm.fail" ]; then ' .. quote(real_rm) .. ' "$@"; exit 1; fi', 'if [ ! -f "$STUB_DIR/rm.slow" ]; then exec ' .. quote(real_rm) .. ' "$@"; fi', "shift", @@ -248,6 +283,7 @@ describe("scripts/run_integration_tests_individually.sh", function() write(dir .. "/bin/tee", STUB_TEE, true) write(dir .. "/bin/rm", STUB_RM, true) write(dir .. "/bin/mktemp", STUB_MKTEMP, true) + write(dir .. "/bin/mkdir", STUB_MKDIR, true) end) after_each(function() @@ -512,6 +548,10 @@ describe("scripts/run_integration_tests_individually.sh", function() ---@field late_tee number|nil The file's tee ends this many seconds after its input ends ---@field slow_rm number|nil The runner's rm waits this many seconds between deleting files and removing folders ---@field rm_fails boolean|nil The runner's rm removes what it is told to and then exits with 1 + ---@field hold boolean|nil Send the signal while the shell that execs timeout waits before its first command, and let it + --- go after the runner is gone + ---@field hold_cleanup boolean|nil With `hold`, let the shell go while the runner's rm waits, with the folder still there + ---@field interrupt "mktemp"|"mkdir"|nil The first call of that command sends the signal itself, and nothing else does ---@param signal string The signal to send ---@param run SignalRun @@ -539,6 +579,15 @@ describe("scripts/run_integration_tests_individually.sh", function() if run.rm_fails then write(dir .. "/rm.fail", {}) end + if run.hold then + write(dir .. "/hold.sh", HOLD_SHELL) + end + if run.hold_cleanup then + write(dir .. "/rm.hold", {}) + end + if run.interrupt then + write(dir .. "/" .. run.interrupt .. ".interrupt", {}) + end local temp = dir .. "/tmp" shell("mkdir -p " .. quote(temp)) local runner_out = " > " .. quote(dir .. "/runner.out") .. " 2>&1" @@ -549,6 +598,16 @@ describe("scripts/run_integration_tests_individually.sh", function() job = job .. " " .. quote(script) .. " > " .. quote(dir .. "/caller.out") .. " 2>&1" target = "-- -$pid" end + local gate = run.window and "/slow_spec.lua.timeout_started" or run.hold and "/a.held" or "/slow_spec.lua.started" + local send = "" + if not run.interrupt then + send = "for _ in $(seq 100); do [ -e " .. quote(dir .. gate) .. " ] && break; sleep 0.1; done; " + send = send .. "kill -" .. signal .. " " .. target .. "; " + end + if run.hold_cleanup then + send = send .. "for _ in $(seq 100); do [ -e " .. quote(dir .. "/rm.held") .. " ] && break; sleep 0.1; done; " + send = send .. ": > " .. quote(dir .. "/a.release") .. "; sleep 1; : > " .. quote(dir .. "/rm.release") .. "; " + end -- `set -m` gives the background job its own process group and leaves SIGINT unignored, as a runner has at a prompt. local inner = "cd " .. quote(dir) @@ -560,15 +619,14 @@ describe("scripts/run_integration_tests_individually.sh", function() .. quote(dir) .. " REAL_TIMEOUT=" .. quote(real_timeout) + .. (run.hold and " BASH_ENV=" .. quote(dir .. "/hold.sh") or "") .. "; set -m; " .. job - .. " & pid=$!; for _ in $(seq 100); do [ -e " - .. quote(dir .. (run.window and "/slow_spec.lua.timeout_started" or "/slow_spec.lua.started")) - .. " ] && break; sleep 0.1; done; kill -" - .. signal - .. " " - .. target - .. "; wait $pid; echo status=$?; sleep 3" + .. " & pid=$!; " + .. send + .. "wait $pid; echo status=$?; " + .. (run.hold and not run.hold_cleanup and ": > " .. quote(dir .. "/a.release") .. "; " or "") + .. "sleep 3" local pipe = assert(io.popen("bash -c " .. quote(inner))) local output = pipe:read("*a") pipe:close() @@ -674,7 +732,7 @@ describe("scripts/run_integration_tests_individually.sh", function() end) it("stops and does not pass when it cannot open the status file", function() - write(dir .. "/mktemp.drop_dirs", {}) + write(dir .. "/mkdir.drop_state", {}) local status, output, ran = run_script({ { name = "a_spec.lua", exit = 0, output = summary(1, 0, 0) }, }) @@ -722,6 +780,47 @@ describe("scripts/run_integration_tests_individually.sh", function() assert.is_false(result.ended) end) + for _, command in ipairs({ "mktemp", "mkdir" }) do + it("removes its temp items when SIGINT comes right after its first " .. command .. " made something", function() + local result = signal_runner("INT", { interrupt = command }) + + assert.are.equal("130", result.status, result.report) + assert.are.equal("", result.left, "the runner left a temp item that it made before it stored the name") + assert.is_nil(io.open(dir .. "/ran"), "the file's Neovim started after the runner died") + end) + end + + for _, case in ipairs({ { "SIGTERM", "TERM", "143" }, { "SIGINT", "INT", "130" } }) do + local name, signal, expected_status = case[1], case[2], case[3] + it("starts no Neovim when " .. name .. " comes before the shell of the file wrote its process id", function() + local result = signal_runner(signal, { hold = true }) + + assert.are.equal(expected_status, result.status, result.report) + assert.are.equal("", result.left, "the shell of the file made a file after the runner cleaned up") + assert.is_nil(io.open(dir .. "/slow_spec.lua.started"), "the file's Neovim started after the runner died") + end) + end + + it("starts no Neovim when SIGINT comes while the cleanup runs, after it looked for the process id", function() + local result = signal_runner("INT", { hold = true, hold_cleanup = true }) + + assert.are.equal("130", result.status, result.report) + assert.are.equal("", result.left, "the shell of the file made a file after the runner cleaned up") + assert.is_nil(io.open(dir .. "/slow_spec.lua.started"), "the file's Neovim started while the runner cleaned up") + end) + + it("does not remove a folder that it did not make, when the name that mktemp printed exists", function() + shell("mkdir " .. quote(dir .. "/taken") .. " && : > " .. quote(dir .. "/taken/keep")) + write(dir .. "/mktemp.name", { dir .. "/taken" }) + local status, output, ran = run_script({ + { name = "a_spec.lua", exit = 0, output = summary(1, 0, 0) }, + }) + + assert.are_not.equal("0", status, output) + assert.are.same({}, ran) + assert.is_not_nil(io.open(dir .. "/taken/keep"), "the runner removed a folder that it did not make: " .. output) + end) + it("shortens only the kill grace of the file that ignores SIGTERM", function() local status, output, ran = run_script({ { name = "a_spec.lua", exit = 0, output = summary(1, 0, 0) }, From c4e00f8181bc1295065a62409d9a5e07b5355281 Mon Sep 17 00:00:00 2001 From: Thomas Kosiewski Date: Thu, 12 Nov 2026 22:56:26 +0000 Subject: [PATCH 03/14] docs(scripts): correct runner comments on temp folder name and test stubs Say that a signal before the runner drops a taken name can still remove that folder, name the stubs that read /proc, and drop a comment that repeated the cancel ordering note above it. --- scripts/README.md | 2 +- scripts/run_integration_tests_individually.sh | 6 ++---- tests/unit/integration_runner_spec.lua | 8 ++++---- 3 files changed, 7 insertions(+), 9 deletions(-) diff --git a/scripts/README.md b/scripts/README.md index bcb8189e..ce15cbe5 100644 --- a/scripts/README.md +++ b/scripts/README.md @@ -84,7 +84,7 @@ The Lua repros run from the repository root with `nvim --headless -u NONE -l scr - CAUTION: Do not run the token-sending clients (`repro_issue_238.sh`, `repro_issue_248.sh`, and `repro_issue_277.sh`) on a shared machine. They pass the auth token on the `websocat` command line (`--header`), where other users can read it with `ps` [repro_issue_238.sh:WEBSOCAT] [repro_issue_248.sh:TOKEN] [repro_issue_277.sh:TOKEN]. - `repro_issue_238.sh`, `repro_issue_248.sh`, and `repro_issue_277.sh` connect to the plugin server on `127.0.0.1` [repro_issue_238.sh:PORT] [repro_issue_248.sh:PORT] [repro_issue_277.sh:PORT]. -- `run_integration_tests_individually.sh` creates one temp folder per test, and puts the output, the file that holds the process id of the file's `timeout`, and the `XDG_STATE_HOME` and `XDG_CONFIG_HOME` that the file's two Neovims share in it, and deletes the folder after the file ran. The runner stores the name of the folder (from `mktemp -u`) before it makes the folder, so a signal never finds a folder that the runner does not know, and `mkdir` fails when the name exists, so the runner removes nothing that it did not make. The runner opens a status file in the state folder before it starts the file, and the file's subshell writes the exit statuses of nvim and of `tee` to that open file when the file ends. A subshell that ends while the cleanup removes the folder opens no path, so it leaves nothing behind. When the runner cannot read the file back, for example because the Neovim deleted the state folder, it uses the exit status of the subshell, which is the status of nvim, or of `tee` when nvim exited 0, and names it as the status of nvim. When it gets SIGTERM, SIGHUP or SIGINT (Ctrl-C) while a file runs, it makes `cancel` in the folder, kills that `timeout`, then the process group of the file, deletes the folder and dies by that signal at once, even when a delete fails, so a calling shell sees status 143, 129 or 130, and a shell loop around the runner stops after Ctrl-C. Killing the `timeout` first covers the moments before it has made its own group, when no Neovim runs yet. Both kills use KILL, so a Neovim that ignores SIGTERM stops too. The shell of the file writes its process id to the file in the folder, then looks for the folder and for `cancel`, and it starts no Neovim when it finds `cancel` or no folder. The runner makes `cancel` before it reads the process id, so one of the two sees the other: a signal at any moment leaves no Neovim that the runner cannot find. SIGKILL gives the runner no time to clean up, so the folder stays. Bash ignores SIGQUIT, so the file runs to its end. It sets `VIMINIT`, `CLAUDECODE_MINIMAL_INIT`, `XDG_STATE_HOME` and `XDG_CONFIG_HOME` for that file's Neovims and the processes they start only, as a prefix of one command, and leaves `XDG_DATA_HOME` alone because plenary lives there. A run writes nothing under `~/.local/state/nvim`, and the developer's `ftplugin` and `after` files under `~/.config/nvim` do not run [run_integration_tests_individually.sh:work_dir, temp_output, group_file, state_dir, config_dir]. +- `run_integration_tests_individually.sh` creates one temp folder per test, and puts the output, the file that holds the process id of the file's `timeout`, and the `XDG_STATE_HOME` and `XDG_CONFIG_HOME` that the file's two Neovims share in it, and deletes the folder after the file ran. The runner stores the name of the folder (from `mktemp -u`) before it makes the folder, so a signal never finds a folder that the runner does not know, and `mkdir` fails when the name exists, and the runner then drops the name. Only a signal that comes before it drops the name can make it remove a folder that it did not make. The runner opens a status file in the state folder before it starts the file, and the file's subshell writes the exit statuses of nvim and of `tee` to that open file when the file ends. A subshell that ends while the cleanup removes the folder opens no path, so it leaves nothing behind. When the runner cannot read the file back, for example because the Neovim deleted the state folder, it uses the exit status of the subshell, which is the status of nvim, or of `tee` when nvim exited 0, and names it as the status of nvim. When it gets SIGTERM, SIGHUP or SIGINT (Ctrl-C) while a file runs, it makes `cancel` in the folder, kills that `timeout`, then the process group of the file, deletes the folder and dies by that signal at once, even when a delete fails, so a calling shell sees status 143, 129 or 130, and a shell loop around the runner stops after Ctrl-C. Killing the `timeout` first covers the moments before it has made its own group, when no Neovim runs yet. Both kills use KILL, so a Neovim that ignores SIGTERM stops too. The shell of the file writes its process id to the file in the folder, then looks for the folder and for `cancel`, and it starts no Neovim when it finds `cancel` or no folder. The runner makes `cancel` before it reads the process id, so one of the two sees the other: a signal at any moment leaves no Neovim that the runner cannot find. SIGKILL gives the runner no time to clean up, so the folder stays. Bash ignores SIGQUIT, so the file runs to its end. It sets `VIMINIT`, `CLAUDECODE_MINIMAL_INIT`, `XDG_STATE_HOME` and `XDG_CONFIG_HOME` for that file's Neovims and the processes they start only, as a prefix of one command, and leaves `XDG_DATA_HOME` alone because plenary lives there. A run writes nothing under `~/.local/state/nvim`, and the developer's `ftplugin` and `after` files under `~/.config/nvim` do not run [run_integration_tests_individually.sh:work_dir, temp_output, group_file, state_dir, config_dir]. - `repro_issue_231.lua` deletes `/__issue_231_repro__.md` [repro_issue_231.lua:try_open_diff]. - `repro_issue_70.sh` and `repro_issue_277.sh` make a work dir with `mktemp -d` (`${TMPDIR:-/tmp}/issue70.XXXXXX` or `issue277.XXXXXX`) and an `att-home` folder in it before they check their tools. They install `trap cleanup EXIT` after that check, so an exit 3 for a missing tool leaves the work dir behind. The trap also runs on SIGTERM, SIGINT and SIGHUP. It destroys the `agent-tty` sessions and removes the work dir. agent-tty still leaves an empty socket folder `/tmp/agent-tty/<8 hex digits>` (it ignores `TMPDIR`) and an entry for the deleted home in `${XDG_STATE_HOME:-$HOME/.local/state}/agent-tty/homes.json`, one of each per run. The sessions inherit the caller's environment, so `CLAUDE_CONFIG_DIR` and the `XDG_*` variables reach the Neovim and the `claude` CLI that they run. `repro_issue_70.sh` launches the real `claude` CLI, and it also runs `claude --version` once outside agent-tty. Its cleanup runs `pkill -f "repro_issue_70_probe.lua $REPO_ROOT"`, which also kills the probe of another run in the same checkout [repro_issue_70.sh:WORK, cleanup] [repro_issue_277.sh:WORK, cleanup]. - `repro_issue_70_probe.lua` starts the real server with `auto_start = true`, so the plugin writes a real `PORT.lock` into the user's lock directory. Other Claude CLIs can see that lock file until `claudecode.stop()` removes it at the end. When the removal fails, `stop` only logs a warning, so a lock file that the removal failed to delete stays [repro_issue_70_probe.lua:claudecode.setup, claudecode.stop] [lua/claudecode/init.lua:M.stop]. diff --git a/scripts/run_integration_tests_individually.sh b/scripts/run_integration_tests_individually.sh index 75d1e136..111cdcda 100755 --- a/scripts/run_integration_tests_individually.sh +++ b/scripts/run_integration_tests_individually.sh @@ -72,7 +72,8 @@ run_test_file() { # All temp items of the file live in one folder. The runner picks the name of the folder and stores it before it makes # the folder, so that no signal finds a folder that the runner made but does not know yet. `mktemp -u` makes nothing - # and prints a random name. mkdir fails when that name exists, and the runner then removes nothing. + # and prints a random name. mkdir fails when that name exists, and the runner then drops the name. Only a signal that + # comes before it drops the name can make it remove a folder that it did not make. local work_dir temp_output group_file state_dir config_dir status_file work_dir=$(mktemp -u) CURRENT_WORK=$work_dir @@ -106,9 +107,6 @@ run_test_file() { # being removed. The subshell closes descriptor 9 for nvim and `tee`, which do not need it. The number is fixed, # because bash 3.2 (the macOS /bin/bash) does not know a `{name}>` redirection: it fails the `exec`, and with an EXIT # trap set it then exits 0 without running a spec. - # The shell that execs `timeout` writes its process id first, then looks for the folder and for `cancel`, and it exits - # when it writes nothing, finds no folder, or finds `cancel`. A signal that stopped the runner before that shell wrote - # its id has made `cancel` or removed the folder, so no Neovim starts that the cleanup cannot find. ( set +o pipefail set +e diff --git a/tests/unit/integration_runner_spec.lua b/tests/unit/integration_runner_spec.lua index 78f6e196..79f26a49 100644 --- a/tests/unit/integration_runner_spec.lua +++ b/tests/unit/integration_runner_spec.lua @@ -119,8 +119,8 @@ local function shell(command) assert(status == 0 or status == true, "failed: " .. command) end --- The stubs below list the files they hold open from /proc//fd. Where it does not exist they list nothing, and a --- runner that keeps a descriptor open would pass the test that reads their notes. +-- The nvim, tee and rm stubs list the files they hold open from /proc//fd. Where it does not exist they list +-- nothing, and a runner that keeps a descriptor open would pass the test that reads their notes. local has_proc_fd = (function() local status = os.execute("test -d /proc/self/fd") return status == 0 or status == true @@ -551,14 +551,14 @@ describe("scripts/run_integration_tests_individually.sh", function() ---@field hold boolean|nil Send the signal while the shell that execs timeout waits before its first command, and let it --- go after the runner is gone ---@field hold_cleanup boolean|nil With `hold`, let the shell go while the runner's rm waits, with the folder still there - ---@field interrupt "mktemp"|"mkdir"|nil The first call of that command sends the signal itself, and nothing else does + ---@field interrupt "mktemp"|"mkdir"|nil The first call of that command sends SIGINT itself, and `signal` goes unused ---@param signal string The signal to send ---@param run SignalRun ---@return table result `status` of the runner (of the calling shell with `caller_loop`), `left` in its TMPDIR once --- its Neovim's grandchild would have written, `ended` when the file's Neovim got to the end of its 2 second run, --- `runner_output` (stdout and stderr of the runner), `caller_output` of the calling shell, and `report`, one line - --- with all of these for the message of an assertion + --- with all of these but `ended` for the message of an assertion local function signal_runner(signal, run) write(dir .. "/tests/integration/slow_spec.lua", {}) write(dir .. "/slow_spec.lua.out", summary(1, 0, 0)) From 02637103c9bb8edb86488242d58057bfe6e492e7 Mon Sep 17 00:00:00 2001 From: Thomas Kosiewski Date: Fri, 13 Nov 2026 00:07:11 +0000 Subject: [PATCH 04/14] fix(scripts): keep the runner's start-up guard until its folder is gone and trap TERM and HUP MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two Codex findings on the first version, both valid. 1. The cleanup made a `cancel` file in the work folder and then ran rm -rf. rm removes `cancel` before it removes the folder, so a held launcher could find the folder and no `cancel` in between, and start `timeout` after the cleanup had found no process id. The cleanup now renames the folder first and reads the process id file and removes the folder from the new name. The launcher looks only for the folder at its old path, which is gone from the moment of the rename. 2. SIGTERM and SIGHUP were not trapped, so bash ended and ran the EXIT cleanup before a foreground mkdir that was already running had made the folder. The folder was left. INT, TERM and HUP now share one handler. Bash runs a trap after the foreground command, so the cleanup finds the folder. The handler removes everything, then kills the runner with the same signal. The test that lets the launcher go now does it while the stub rm has deleted the files and not yet the folders, and it fails on the previous version. New tests make a mkdir stub send SIGTERM or SIGHUP to the runner alone and make the folder only after the runner would have died, and they fail on the previous version too. The inner shell of the tests no longer prints the job report of a runner that SIGHUP killed. _Generated with [`xum`](https://github.com/coder/xum) • Model: `anthropic:claude-sonnet-5-5` • Thinking: `xhigh`_ --- scripts/README.md | 2 +- scripts/run_integration_tests_individually.sh | 57 ++++++++++-------- tests/unit/integration_runner_spec.lua | 59 ++++++++++++++----- 3 files changed, 77 insertions(+), 41 deletions(-) diff --git a/scripts/README.md b/scripts/README.md index ce15cbe5..4f4131cf 100644 --- a/scripts/README.md +++ b/scripts/README.md @@ -84,7 +84,7 @@ The Lua repros run from the repository root with `nvim --headless -u NONE -l scr - CAUTION: Do not run the token-sending clients (`repro_issue_238.sh`, `repro_issue_248.sh`, and `repro_issue_277.sh`) on a shared machine. They pass the auth token on the `websocat` command line (`--header`), where other users can read it with `ps` [repro_issue_238.sh:WEBSOCAT] [repro_issue_248.sh:TOKEN] [repro_issue_277.sh:TOKEN]. - `repro_issue_238.sh`, `repro_issue_248.sh`, and `repro_issue_277.sh` connect to the plugin server on `127.0.0.1` [repro_issue_238.sh:PORT] [repro_issue_248.sh:PORT] [repro_issue_277.sh:PORT]. -- `run_integration_tests_individually.sh` creates one temp folder per test, and puts the output, the file that holds the process id of the file's `timeout`, and the `XDG_STATE_HOME` and `XDG_CONFIG_HOME` that the file's two Neovims share in it, and deletes the folder after the file ran. The runner stores the name of the folder (from `mktemp -u`) before it makes the folder, so a signal never finds a folder that the runner does not know, and `mkdir` fails when the name exists, and the runner then drops the name. Only a signal that comes before it drops the name can make it remove a folder that it did not make. The runner opens a status file in the state folder before it starts the file, and the file's subshell writes the exit statuses of nvim and of `tee` to that open file when the file ends. A subshell that ends while the cleanup removes the folder opens no path, so it leaves nothing behind. When the runner cannot read the file back, for example because the Neovim deleted the state folder, it uses the exit status of the subshell, which is the status of nvim, or of `tee` when nvim exited 0, and names it as the status of nvim. When it gets SIGTERM, SIGHUP or SIGINT (Ctrl-C) while a file runs, it makes `cancel` in the folder, kills that `timeout`, then the process group of the file, deletes the folder and dies by that signal at once, even when a delete fails, so a calling shell sees status 143, 129 or 130, and a shell loop around the runner stops after Ctrl-C. Killing the `timeout` first covers the moments before it has made its own group, when no Neovim runs yet. Both kills use KILL, so a Neovim that ignores SIGTERM stops too. The shell of the file writes its process id to the file in the folder, then looks for the folder and for `cancel`, and it starts no Neovim when it finds `cancel` or no folder. The runner makes `cancel` before it reads the process id, so one of the two sees the other: a signal at any moment leaves no Neovim that the runner cannot find. SIGKILL gives the runner no time to clean up, so the folder stays. Bash ignores SIGQUIT, so the file runs to its end. It sets `VIMINIT`, `CLAUDECODE_MINIMAL_INIT`, `XDG_STATE_HOME` and `XDG_CONFIG_HOME` for that file's Neovims and the processes they start only, as a prefix of one command, and leaves `XDG_DATA_HOME` alone because plenary lives there. A run writes nothing under `~/.local/state/nvim`, and the developer's `ftplugin` and `after` files under `~/.config/nvim` do not run [run_integration_tests_individually.sh:work_dir, temp_output, group_file, state_dir, config_dir]. +- `run_integration_tests_individually.sh` creates one temp folder per test, and puts the output, the file that holds the process id of the file's `timeout`, and the `XDG_STATE_HOME` and `XDG_CONFIG_HOME` that the file's two Neovims share in it, and deletes the folder after the file ran. The runner stores the name of the folder (from `mktemp -u`) before it makes the folder, so a signal never finds a folder that the runner does not know, and `mkdir` fails when the name exists, and the runner then drops the name. Only a signal that comes before it drops the name can make it remove a folder that it did not make. The runner opens a status file in the state folder before it starts the file, and the file's subshell writes the exit statuses of nvim and of `tee` to that open file when the file ends. A subshell that ends while the cleanup removes the folder opens no path, so it leaves nothing behind. When the runner cannot read the file back, for example because the Neovim deleted the state folder, it uses the exit status of the subshell, which is the status of nvim, or of `tee` when nvim exited 0, and names it as the status of nvim. When it gets SIGTERM, SIGHUP or SIGINT (Ctrl-C) while a file runs, it renames the folder, kills that `timeout`, then the process group of the file, deletes the renamed folder and dies by that signal as soon as the command that it waits for ends, even when a delete fails, so a calling shell sees status 143, 129 or 130, and a shell loop around the runner stops after Ctrl-C. Killing the `timeout` first covers the moments before it has made its own group, when no Neovim runs yet. Both kills use KILL, so a Neovim that ignores SIGTERM stops too. The shell of the file writes its process id to the file in the folder, then looks for the folder, and it starts no Neovim when it finds none. The runner renames the folder before it reads the process id, so one of the two sees the other: a signal at any moment leaves no Neovim that the runner cannot find. SIGKILL gives the runner no time to clean up, so the folder stays. Bash ignores SIGQUIT, so the file runs to its end. It sets `VIMINIT`, `CLAUDECODE_MINIMAL_INIT`, `XDG_STATE_HOME` and `XDG_CONFIG_HOME` for that file's Neovims and the processes they start only, as a prefix of one command, and leaves `XDG_DATA_HOME` alone because plenary lives there. A run writes nothing under `~/.local/state/nvim`, and the developer's `ftplugin` and `after` files under `~/.config/nvim` do not run [run_integration_tests_individually.sh:work_dir, temp_output, group_file, state_dir, config_dir]. - `repro_issue_231.lua` deletes `/__issue_231_repro__.md` [repro_issue_231.lua:try_open_diff]. - `repro_issue_70.sh` and `repro_issue_277.sh` make a work dir with `mktemp -d` (`${TMPDIR:-/tmp}/issue70.XXXXXX` or `issue277.XXXXXX`) and an `att-home` folder in it before they check their tools. They install `trap cleanup EXIT` after that check, so an exit 3 for a missing tool leaves the work dir behind. The trap also runs on SIGTERM, SIGINT and SIGHUP. It destroys the `agent-tty` sessions and removes the work dir. agent-tty still leaves an empty socket folder `/tmp/agent-tty/<8 hex digits>` (it ignores `TMPDIR`) and an entry for the deleted home in `${XDG_STATE_HOME:-$HOME/.local/state}/agent-tty/homes.json`, one of each per run. The sessions inherit the caller's environment, so `CLAUDE_CONFIG_DIR` and the `XDG_*` variables reach the Neovim and the `claude` CLI that they run. `repro_issue_70.sh` launches the real `claude` CLI, and it also runs `claude --version` once outside agent-tty. Its cleanup runs `pkill -f "repro_issue_70_probe.lua $REPO_ROOT"`, which also kills the probe of another run in the same checkout [repro_issue_70.sh:WORK, cleanup] [repro_issue_277.sh:WORK, cleanup]. - `repro_issue_70_probe.lua` starts the real server with `auto_start = true`, so the plugin writes a real `PORT.lock` into the user's lock directory. Other Claude CLIs can see that lock file until `claudecode.stop()` removes it at the end. When the removal fails, `stop` only logs a warning, so a lock file that the removal failed to delete stays [repro_issue_70_probe.lua:claudecode.setup, claudecode.stop] [lua/claudecode/init.lua:M.stop]. diff --git a/scripts/run_integration_tests_individually.sh b/scripts/run_integration_tests_individually.sh index 111cdcda..445601ff 100755 --- a/scripts/run_integration_tests_individually.sh +++ b/scripts/run_integration_tests_individually.sh @@ -16,34 +16,43 @@ FILE_TIMEOUT=40s # cleanup stops the file's Neovims before it removes the folder, because one that ends later writes ShaDa and makes its # state folder again. It kills `timeout` itself first: until timeout has made its own group, no Neovim runs yet. It # uses KILL for both, because with `timeout` dead its -k grace never starts, and a Neovim that ignores TERM would run -# on. SIGTERM and SIGHUP kill bash, which runs the EXIT trap first. -# The cleanup makes `cancel` in the folder before it reads the process id file, and the shell that starts the Neovim -# writes its process id to that file before it looks for `cancel` and for the folder (see below). So one of them sees -# the other: a shell that wrote its id too late for the cleanup finds `cancel` or no folder, and starts no Neovim. +# on. +# The cleanup renames the folder before it reads the process id file, so the old path is gone at once, and rm removes +# the renamed folder. The shell that starts the Neovim writes its process id to the file at the old path, then looks for +# the folder at the old path (the `bash -c` line in run_test_file). So one of them sees the other: a shell that wrote +# its id too late for the cleanup to read it finds no folder, and starts no Neovim. CURRENT_WORK="" -CURRENT_GROUP_FILE="" -# shellcheck disable=SC2329 # The two traps below run it. +CURRENT_FILE_RUNS="" +# shellcheck disable=SC2329 # The traps below run it. cleanup_current_file() { - local timeout_pid - if [[ -n $CURRENT_WORK ]]; then - : >"$CURRENT_WORK/cancel" 2>/dev/null || true - fi - if [[ -s $CURRENT_GROUP_FILE ]]; then - timeout_pid=$(cat "$CURRENT_GROUP_FILE") + local gone timeout_pid + [[ -n $CURRENT_WORK ]] || return 0 + gone=$CURRENT_WORK.gone + mv "$CURRENT_WORK" "$gone" 2>/dev/null || gone=$CURRENT_WORK + if [[ -n $CURRENT_FILE_RUNS && -s $gone/group ]]; then + timeout_pid=$(cat "$gone/group") kill -KILL "$timeout_pid" 2>/dev/null || true kill -KILL -- "-$timeout_pid" 2>/dev/null || true fi - if [[ -n $CURRENT_WORK ]]; then - rm -rf "$CURRENT_WORK" - fi + rm -rf "$gone" +} +# A signal reaches the runner's own process group, and timeout leaves that group, so the file runs in the background and +# `wait` below returns at once when a trap runs. Bash runs a trap after the foreground command that it waits for, so a +# signal that comes while mkdir makes the folder finds the folder. The handler cleans up, then kills the runner with the +# signal, not with `exit`: a calling shell stops its own loop after a child that died by SIGINT, and goes on after one +# that exited 130. A cleanup that fails, such as an rm that cannot remove a folder, must not end the handler early +# either: with set -e the runner would exit with 1 before the kill, and the calling shell would go on. +# shellcheck disable=SC2329 # The traps below run it. +die_by_signal() { + cleanup_current_file || true + trap - EXIT INT TERM HUP + kill -"$1" $$ + exit "$2" } trap cleanup_current_file EXIT -# A Ctrl-C reaches the runner's own process group, and timeout leaves that group, so the file runs in the background and -# `wait` below returns at once when this trap runs. The trap cleans up, then kills the runner with SIGINT, not with -# `exit 130`: a calling shell stops its own loop after a child that died by SIGINT, and goes on after one that exited -# 130. A cleanup that fails, such as an rm that cannot remove a folder, must not end the trap early either: with set -e -# the runner would exit with 1 before the kill, and the calling shell would go on. -trap 'cleanup_current_file || true; trap - EXIT INT; kill -INT $$; exit 130' INT +trap 'die_by_signal INT 130' INT +trap 'die_by_signal TERM 143' TERM +trap 'die_by_signal HUP 129' HUP echo "=== Running Integration Tests Individually ===" @@ -87,7 +96,7 @@ run_test_file() { config_dir=$work_dir/config status_file=$state_dir/exit_statuses mkdir -m 700 "$state_dir" "$config_dir" - CURRENT_GROUP_FILE=$group_file + CURRENT_FILE_RUNS=1 exec 9>"$status_file" # timeout runs nvim in a process group named after its own pid, which is this shell's pid until it execs timeout. @@ -113,7 +122,7 @@ run_test_file() { VIMINIT='lua local f = vim.env.CLAUDECODE_MINIMAL_INIT if f then dofile(f) end' \ CLAUDECODE_MINIMAL_INIT="$PWD/tests/minimal_init.lua" \ XDG_STATE_HOME="$state_dir" XDG_CONFIG_HOME="$config_dir" \ - bash -c 'echo $$ 2> /dev/null > "$1" && [ -d "$2" ] && [ ! -e "$2/cancel" ] || exit 1; shift 2; exec timeout -k 5s "$@"' \ + bash -c 'echo $$ 2> /dev/null > "$1" && [ -d "$2" ] || exit 1; shift 2; exec timeout -k 5s "$@"' \ _ "$group_file" "$work_dir" "$FILE_TIMEOUT" \ nvim --headless -i NONE -u tests/minimal_init.lua \ -c "lua require('plenary.test_harness').test_file('$test_file')" \ @@ -127,7 +136,7 @@ run_test_file() { else EXIT_CODE=$? fi - CURRENT_GROUP_FILE="" + CURRENT_FILE_RUNS="" exec 9>&- local tee_status=0 status_line if status_line=$(cat "$status_file" 2>/dev/null) && [[ $status_line =~ ^([0-9]+)\ ([0-9]+)$ ]]; then diff --git a/tests/unit/integration_runner_spec.lua b/tests/unit/integration_runner_spec.lua index 79f26a49..5d320e28 100644 --- a/tests/unit/integration_runner_spec.lua +++ b/tests/unit/integration_runner_spec.lua @@ -181,15 +181,25 @@ local real_mkdir = assert(mkdir_pipe:read("*l"), "needs mkdir") mkdir_pipe:close() -- Runs the real mkdir. When $STUB_DIR/mkdir.drop_state exists, it then removes each folder named state that it made, so -- that the folder does not exist when the runner opens the status file in it. When $STUB_DIR/mkdir.interrupt exists, --- its first call sends SIGINT to its process group, which has the runner in it, after the real mkdir ran. +-- its first call sends the signal named in that file. SIGINT goes to its process group, which has the runner in it, +-- after the real mkdir ran. Any other signal goes to the runner alone, before the real mkdir runs, and the stub then +-- waits up to 3 seconds for the runner to end, so a runner that dies at once is gone before the folder exists. local STUB_MKDIR = { "#!/bin/sh", + "sig=", + 'if [ -f "$STUB_DIR/mkdir.interrupt" ] && [ ! -f "$STUB_DIR/mkdir.interrupted" ]; then', + ' : > "$STUB_DIR/mkdir.interrupted"', + ' sig=$(cat "$STUB_DIR/mkdir.interrupt")', + ' if [ "$sig" != INT ]; then', + ' kill -s "$sig" "$PPID"', + ' for _ in $(seq 60); do kill -0 "$PPID" 2> /dev/null || break; sleep 0.05; done', + " fi", + "fi", quote(real_mkdir) .. ' "$@" || exit $?', 'if [ -f "$STUB_DIR/mkdir.drop_state" ]; then', ' for path in "$@"; do case "$path" in */state) rmdir "$path" ;; esac; done', "fi", - 'if [ -f "$STUB_DIR/mkdir.interrupt" ] && [ ! -f "$STUB_DIR/mkdir.interrupted" ]; then', - ' : > "$STUB_DIR/mkdir.interrupted"', + 'if [ "$sig" = INT ]; then', ' trap "" INT', " kill -s INT 0", "fi", @@ -207,23 +217,27 @@ local HOLD_SHELL = { local rm_pipe = assert(io.popen("command -v rm 2> /dev/null")) local real_rm = assert(rm_pipe:read("*l"), "needs rm") rm_pipe:close() --- Notes in $STUB_DIR/rm.holds_status_file that it holds the status file open. Notes in $STUB_DIR/rm.held and waits for --- $STUB_DIR/rm.release when $STUB_DIR/rm.hold exists. Runs the real rm and then exits with 1 +-- Notes in $STUB_DIR/rm.holds_status_file that it holds the status file open. Runs the real rm and then exits with 1 -- when $STUB_DIR/rm.fail exists, as rm does when a folder is not empty. Runs the real rm, except when --- $STUB_DIR/rm.slow exists: then it walks the way rm -rf does, but slowly. It deletes the --- files, waits the seconds in that file, and only then removes the folders, so a file that something makes meanwhile in --- a folder stops the removal of the folder ("Directory not empty"), as it does for a slow rm -rf under strace. +-- $STUB_DIR/rm.slow or $STUB_DIR/rm.hold exists: then it walks the way rm -rf does, but slowly. It deletes the +-- files, waits, and only then removes the folders, so a file that something makes meanwhile in a folder stops the +-- removal of the folder ("Directory not empty"), as it does for a slow rm -rf under strace. It waits the seconds in +-- rm.slow, or with rm.hold it notes in $STUB_DIR/rm.held and waits for $STUB_DIR/rm.release. local STUB_RM = { "#!/bin/sh", "for fd in /proc/$$/fd/*; do", ' case "$(readlink "$fd" 2> /dev/null)" in */exit_statuses*) : > "$STUB_DIR/rm.holds_status_file" ;; esac', "done", - 'if [ -f "$STUB_DIR/rm.hold" ]; then : > "$STUB_DIR/rm.held"; while [ ! -e "$STUB_DIR/rm.release" ]; do sleep 0.05; done; fi', 'if [ -f "$STUB_DIR/rm.fail" ]; then ' .. quote(real_rm) .. ' "$@"; exit 1; fi', - 'if [ ! -f "$STUB_DIR/rm.slow" ]; then exec ' .. quote(real_rm) .. ' "$@"; fi', + 'if [ ! -f "$STUB_DIR/rm.slow" ] && [ ! -f "$STUB_DIR/rm.hold" ]; then exec ' .. quote(real_rm) .. ' "$@"; fi', "shift", 'for path in "$@"; do find "$path" -type f -delete; done', - 'sleep "$(cat "$STUB_DIR/rm.slow")"', + 'if [ -f "$STUB_DIR/rm.hold" ]; then', + ' : > "$STUB_DIR/rm.held"', + ' while [ ! -e "$STUB_DIR/rm.release" ]; do sleep 0.05; done', + "else", + ' sleep "$(cat "$STUB_DIR/rm.slow")"', + "fi", 'for path in "$@"; do find "$path" -depth -type d -exec rmdir {} +; done', } @@ -550,8 +564,10 @@ describe("scripts/run_integration_tests_individually.sh", function() ---@field rm_fails boolean|nil The runner's rm removes what it is told to and then exits with 1 ---@field hold boolean|nil Send the signal while the shell that execs timeout waits before its first command, and let it --- go after the runner is gone - ---@field hold_cleanup boolean|nil With `hold`, let the shell go while the runner's rm waits, with the folder still there - ---@field interrupt "mktemp"|"mkdir"|nil The first call of that command sends SIGINT itself, and `signal` goes unused + ---@field hold_cleanup boolean|nil With `hold`, let the shell go while the runner's rm waits, after it deleted the files + --- and before it removed the folders + ---@field interrupt "mktemp"|"mkdir"|nil The first call of that command sends the signal itself, and nothing else does. + --- A mktemp sends only SIGINT ---@param signal string The signal to send ---@param run SignalRun @@ -586,7 +602,7 @@ describe("scripts/run_integration_tests_individually.sh", function() write(dir .. "/rm.hold", {}) end if run.interrupt then - write(dir .. "/" .. run.interrupt .. ".interrupt", {}) + write(dir .. "/" .. run.interrupt .. ".interrupt", { signal }) end local temp = dir .. "/tmp" shell("mkdir -p " .. quote(temp)) @@ -624,7 +640,7 @@ describe("scripts/run_integration_tests_individually.sh", function() .. job .. " & pid=$!; " .. send - .. "wait $pid; echo status=$?; " + .. "wait $pid 2> /dev/null; echo status=$?; " .. (run.hold and not run.hold_cleanup and ": > " .. quote(dir .. "/a.release") .. "; " or "") .. "sleep 3" local pipe = assert(io.popen("bash -c " .. quote(inner))) @@ -790,6 +806,17 @@ describe("scripts/run_integration_tests_individually.sh", function() end) end + for _, case in ipairs({ { "SIGTERM", "TERM", "143" }, { "SIGHUP", "HUP", "129" } }) do + local name, signal, expected_status = case[1], case[2], case[3] + it("removes its temp folder when " .. name .. " comes while mkdir makes it", function() + local result = signal_runner(signal, { interrupt = "mkdir" }) + + assert.are.equal(expected_status, result.status, result.report) + assert.are.equal("", result.left, "mkdir made the folder after the runner had cleaned up") + assert.is_nil(io.open(dir .. "/ran"), "the file's Neovim started after the runner died") + end) + end + for _, case in ipairs({ { "SIGTERM", "TERM", "143" }, { "SIGINT", "INT", "130" } }) do local name, signal, expected_status = case[1], case[2], case[3] it("starts no Neovim when " .. name .. " comes before the shell of the file wrote its process id", function() @@ -801,7 +828,7 @@ describe("scripts/run_integration_tests_individually.sh", function() end) end - it("starts no Neovim when SIGINT comes while the cleanup runs, after it looked for the process id", function() + it("starts no Neovim when SIGINT comes while its rm has deleted the files and not yet the folders", function() local result = signal_runner("INT", { hold = true, hold_cleanup = true }) assert.are.equal("130", result.status, result.report) From 0876d3c0806337192a0efd20e31463c85665d0e3 Mon Sep 17 00:00:00 2001 From: Thomas Kosiewski Date: Fri, 13 Nov 2026 00:22:59 +0000 Subject: [PATCH 05/14] test(scripts): skip the runner's SIGHUP test where SIGHUP is ignored MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A process that nohup or a similar launcher started has SIGHUP ignored, and bash cannot trap a signal that was ignored when it started. The runner then ignores the signal, as nohup intends, and the test that makes a mkdir stub send SIGHUP to the runner could not see the cleanup. The test now probes the signal with `trap -p` and reports pending where it is ignored. _Generated with [`xum`](https://github.com/coder/xum) • Model: `anthropic:claude-sonnet-5-5` • Thinking: `xhigh`_ --- tests/unit/integration_runner_spec.lua | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/tests/unit/integration_runner_spec.lua b/tests/unit/integration_runner_spec.lua index 5d320e28..753b4102 100644 --- a/tests/unit/integration_runner_spec.lua +++ b/tests/unit/integration_runner_spec.lua @@ -126,6 +126,17 @@ local has_proc_fd = (function() return status == 0 or status == true end)() +-- A process that nohup or a similar launcher started has SIGHUP ignored, and every process below it ignores it too. Bash +-- cannot trap a signal that was ignored when it started, so a runner there ignores that signal. +---@param signal string +---@return boolean ignored +local function signal_is_ignored(signal) + local pipe = assert(io.popen("bash -c 'trap -p " .. signal .. "'")) + local trapped = pipe:read("*a") + pipe:close() + return trapped:find("''", 1, true) ~= nil +end + -- In some locales a bash regex bracket range such as [0-9] also matches digits like the fullwidth １. ---@return string|nil locale A common locale name in which it does, or nil when the host has none local function find_wide_digit_locale() @@ -809,6 +820,9 @@ describe("scripts/run_integration_tests_individually.sh", function() for _, case in ipairs({ { "SIGTERM", "TERM", "143" }, { "SIGHUP", "HUP", "129" } }) do local name, signal, expected_status = case[1], case[2], case[3] it("removes its temp folder when " .. name .. " comes while mkdir makes it", function() + if signal_is_ignored(signal) then + pending(name .. " is ignored in this environment, so the runner ignores it too") + end local result = signal_runner(signal, { interrupt = "mkdir" }) assert.are.equal(expected_status, result.status, result.report) From be6aad00369c03c132c0d705d5daa3500eb0d024 Mon Sep 17 00:00:00 2001 From: Thomas Kosiewski Date: Fri, 13 Nov 2026 01:48:21 +0000 Subject: [PATCH 06/14] fix(scripts): test every guard of the runner's start-up and give each Neovim a temp folder of its own MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The verifier of this PR found holes in the tests and four defects. Each new test fails on the runner without its guard, and the mutant table covers each guard. - The shell that execs `timeout` checks that its folder is still there after it wrote its process id. A test now stops the shell after its redirection made the process id file and before the write. The runner then renames the folder, reads no id, and removes it, and the shell has to find no folder. The check that the write succeeded has a test too: the process id file is a folder, and no Neovim may start. - A tee that opens `output` only while the cleanup's rm runs left `output` and the folder in the old design. The rename makes tee fail instead, and tests cover SIGINT and SIGTERM. The second mkdir has SIGTERM, SIGHUP and SIGINT tests too. The rm stub removes only the folders that it listed first, as rm -rf does, so a folder made meanwhile stops the removal. - The BASH_ENV files that hold the shell no longer read $0, which bash 3.2 sets after it ran them. They test CLAUDECODE_MINIMAL_INIT, which only that shell has. - The runner gives the Neovims of a file a TMPDIR and an XDG_RUNTIME_DIR inside the work folder. A Neovim that the cleanup kills does not remove its socket folder, and the folder was left in the developer's TMPDIR. - scripts/README.md keeps main's statement on SIGKILL. _Generated with [`xum`](https://github.com/coder/xum) • Model: `anthropic:claude-sonnet-5-5` • Thinking: `xhigh`_ --- scripts/README.md | 2 +- scripts/run_integration_tests_individually.sh | 9 +- tests/unit/integration_runner_spec.lua | 206 ++++++++++++------ 3 files changed, 151 insertions(+), 66 deletions(-) diff --git a/scripts/README.md b/scripts/README.md index 4f4131cf..2dbe199f 100644 --- a/scripts/README.md +++ b/scripts/README.md @@ -84,7 +84,7 @@ The Lua repros run from the repository root with `nvim --headless -u NONE -l scr - CAUTION: Do not run the token-sending clients (`repro_issue_238.sh`, `repro_issue_248.sh`, and `repro_issue_277.sh`) on a shared machine. They pass the auth token on the `websocat` command line (`--header`), where other users can read it with `ps` [repro_issue_238.sh:WEBSOCAT] [repro_issue_248.sh:TOKEN] [repro_issue_277.sh:TOKEN]. - `repro_issue_238.sh`, `repro_issue_248.sh`, and `repro_issue_277.sh` connect to the plugin server on `127.0.0.1` [repro_issue_238.sh:PORT] [repro_issue_248.sh:PORT] [repro_issue_277.sh:PORT]. -- `run_integration_tests_individually.sh` creates one temp folder per test, and puts the output, the file that holds the process id of the file's `timeout`, and the `XDG_STATE_HOME` and `XDG_CONFIG_HOME` that the file's two Neovims share in it, and deletes the folder after the file ran. The runner stores the name of the folder (from `mktemp -u`) before it makes the folder, so a signal never finds a folder that the runner does not know, and `mkdir` fails when the name exists, and the runner then drops the name. Only a signal that comes before it drops the name can make it remove a folder that it did not make. The runner opens a status file in the state folder before it starts the file, and the file's subshell writes the exit statuses of nvim and of `tee` to that open file when the file ends. A subshell that ends while the cleanup removes the folder opens no path, so it leaves nothing behind. When the runner cannot read the file back, for example because the Neovim deleted the state folder, it uses the exit status of the subshell, which is the status of nvim, or of `tee` when nvim exited 0, and names it as the status of nvim. When it gets SIGTERM, SIGHUP or SIGINT (Ctrl-C) while a file runs, it renames the folder, kills that `timeout`, then the process group of the file, deletes the renamed folder and dies by that signal as soon as the command that it waits for ends, even when a delete fails, so a calling shell sees status 143, 129 or 130, and a shell loop around the runner stops after Ctrl-C. Killing the `timeout` first covers the moments before it has made its own group, when no Neovim runs yet. Both kills use KILL, so a Neovim that ignores SIGTERM stops too. The shell of the file writes its process id to the file in the folder, then looks for the folder, and it starts no Neovim when it finds none. The runner renames the folder before it reads the process id, so one of the two sees the other: a signal at any moment leaves no Neovim that the runner cannot find. SIGKILL gives the runner no time to clean up, so the folder stays. Bash ignores SIGQUIT, so the file runs to its end. It sets `VIMINIT`, `CLAUDECODE_MINIMAL_INIT`, `XDG_STATE_HOME` and `XDG_CONFIG_HOME` for that file's Neovims and the processes they start only, as a prefix of one command, and leaves `XDG_DATA_HOME` alone because plenary lives there. A run writes nothing under `~/.local/state/nvim`, and the developer's `ftplugin` and `after` files under `~/.config/nvim` do not run [run_integration_tests_individually.sh:work_dir, temp_output, group_file, state_dir, config_dir]. +- `run_integration_tests_individually.sh` creates one temp folder per test, and puts the output, the file that holds the process id of the file's `timeout`, and the `XDG_STATE_HOME`, `XDG_CONFIG_HOME` and `TMPDIR` that the file's two Neovims share in it, and deletes the folder after the file ran. The runner stores the name of the folder (from `mktemp -u`) before it makes the folder, so a signal never finds a folder that the runner does not know, and `mkdir` fails when the name exists, and the runner then drops the name. Only a signal that comes before it drops the name can make it remove a folder that it did not make. The runner opens a status file in the state folder before it starts the file, and the file's subshell writes the exit statuses of nvim and of `tee` to that open file when the file ends. A subshell that ends while the cleanup removes the folder opens no path, so it leaves nothing behind. When the runner cannot read the file back, for example because the Neovim deleted the state folder, it uses the exit status of the subshell, which is the status of nvim, or of `tee` when nvim exited 0, and names it as the status of nvim. When it gets SIGTERM, SIGHUP or SIGINT (Ctrl-C) while a file runs, it renames the folder, kills that `timeout`, then the process group of the file, deletes the renamed folder and dies by that signal as soon as the command that it waits for ends, even when a delete fails, so a calling shell sees status 143, 129 or 130, and a shell loop around the runner stops after Ctrl-C. Killing the `timeout` first covers the moments before it has made its own group, when no Neovim runs yet. Both kills use KILL, so a Neovim that ignores SIGTERM stops too. The shell of the file writes its process id to the file in the folder, then looks for the folder, and it starts no Neovim when it finds none. The runner renames the folder before it reads the process id, so one of the two sees the other: a signal at any moment leaves no Neovim that the runner cannot find. SIGKILL gives the runner no time to clean up, so the folder stays. A runner that leads its own session on a terminal, for example one that a terminal emulator starts directly, hangs up the foreground process group of the terminal when it dies, so the shell of the file dies too and starts no Neovim. Without a terminal that shell goes on, also when it has not written its process id yet, and its Neovim runs until its own timeout. Bash ignores SIGQUIT, so the file runs to its end. It sets `VIMINIT`, `CLAUDECODE_MINIMAL_INIT`, `XDG_STATE_HOME`, `XDG_CONFIG_HOME`, `TMPDIR` and `XDG_RUNTIME_DIR` for that file's Neovims and the processes they start only, as a prefix of one command, and leaves `XDG_DATA_HOME` alone because plenary lives there. A run writes nothing under `~/.local/state/nvim`, leaves no `nvim.` socket folder in the developer's `TMPDIR`, even when it kills a Neovim, and the developer's `ftplugin` and `after` files under `~/.config/nvim` do not run [run_integration_tests_individually.sh:work_dir, temp_output, group_file, state_dir, config_dir, tmp_dir]. - `repro_issue_231.lua` deletes `/__issue_231_repro__.md` [repro_issue_231.lua:try_open_diff]. - `repro_issue_70.sh` and `repro_issue_277.sh` make a work dir with `mktemp -d` (`${TMPDIR:-/tmp}/issue70.XXXXXX` or `issue277.XXXXXX`) and an `att-home` folder in it before they check their tools. They install `trap cleanup EXIT` after that check, so an exit 3 for a missing tool leaves the work dir behind. The trap also runs on SIGTERM, SIGINT and SIGHUP. It destroys the `agent-tty` sessions and removes the work dir. agent-tty still leaves an empty socket folder `/tmp/agent-tty/<8 hex digits>` (it ignores `TMPDIR`) and an entry for the deleted home in `${XDG_STATE_HOME:-$HOME/.local/state}/agent-tty/homes.json`, one of each per run. The sessions inherit the caller's environment, so `CLAUDE_CONFIG_DIR` and the `XDG_*` variables reach the Neovim and the `claude` CLI that they run. `repro_issue_70.sh` launches the real `claude` CLI, and it also runs `claude --version` once outside agent-tty. Its cleanup runs `pkill -f "repro_issue_70_probe.lua $REPO_ROOT"`, which also kills the probe of another run in the same checkout [repro_issue_70.sh:WORK, cleanup] [repro_issue_277.sh:WORK, cleanup]. - `repro_issue_70_probe.lua` starts the real server with `auto_start = true`, so the plugin writes a real `PORT.lock` into the user's lock directory. Other Claude CLIs can see that lock file until `claudecode.stop()` removes it at the end. When the removal fails, `stop` only logs a warning, so a lock file that the removal failed to delete stays [repro_issue_70_probe.lua:claudecode.setup, claudecode.stop] [lua/claudecode/init.lua:M.stop]. diff --git a/scripts/run_integration_tests_individually.sh b/scripts/run_integration_tests_individually.sh index 445601ff..a7f0c6f8 100755 --- a/scripts/run_integration_tests_individually.sh +++ b/scripts/run_integration_tests_individually.sh @@ -83,7 +83,7 @@ run_test_file() { # the folder, so that no signal finds a folder that the runner made but does not know yet. `mktemp -u` makes nothing # and prints a random name. mkdir fails when that name exists, and the runner then drops the name. Only a signal that # comes before it drops the name can make it remove a folder that it did not make. - local work_dir temp_output group_file state_dir config_dir status_file + local work_dir temp_output group_file state_dir config_dir tmp_dir status_file work_dir=$(mktemp -u) CURRENT_WORK=$work_dir mkdir -m 700 "$work_dir" || { @@ -94,8 +94,9 @@ run_test_file() { group_file=$work_dir/group state_dir=$work_dir/state config_dir=$work_dir/config + tmp_dir=$work_dir/tmp status_file=$state_dir/exit_statuses - mkdir -m 700 "$state_dir" "$config_dir" + mkdir -m 700 "$state_dir" "$config_dir" "$tmp_dir" CURRENT_FILE_RUNS=1 exec 9>"$status_file" @@ -107,6 +108,8 @@ run_test_file() { # State and config folders in temp folders keep a run out of ~/.local/state/nvim (ShaDa, log, swap files) and out of # the developer's ftplugin and after files. plenary lives under XDG_DATA_HOME, which stays. Both Neovims share the # folders, so the one started here takes -i NONE: two ShaDa writers made the first fail with E886 after a timeout. + # TMPDIR and XDG_RUNTIME_DIR name the temp folder in the work folder: a Neovim that the cleanup kills does not remove + # its socket folder, and the folder goes away with the work folder instead of staying in the developer's TMPDIR. # The file runs in the background and the runner waits for it: `wait` ends at once when the INT trap runs. The # subshell has no pipefail, because with it bash prints a job report ("Killed", two lines) when the cleanup above # kills `timeout`. It writes the exit statuses of nvim and of `tee` to a file in the state folder, so that the reason @@ -121,7 +124,7 @@ run_test_file() { set +e VIMINIT='lua local f = vim.env.CLAUDECODE_MINIMAL_INIT if f then dofile(f) end' \ CLAUDECODE_MINIMAL_INIT="$PWD/tests/minimal_init.lua" \ - XDG_STATE_HOME="$state_dir" XDG_CONFIG_HOME="$config_dir" \ + XDG_STATE_HOME="$state_dir" XDG_CONFIG_HOME="$config_dir" TMPDIR="$tmp_dir" XDG_RUNTIME_DIR="$tmp_dir" \ bash -c 'echo $$ 2> /dev/null > "$1" && [ -d "$2" ] || exit 1; shift 2; exec timeout -k 5s "$@"' \ _ "$group_file" "$work_dir" "$FILE_TIMEOUT" \ nvim --headless -i NONE -u tests/minimal_init.lua \ diff --git a/tests/unit/integration_runner_spec.lua b/tests/unit/integration_runner_spec.lua index 753b4102..552e48f2 100644 --- a/tests/unit/integration_runner_spec.lua +++ b/tests/unit/integration_runner_spec.lua @@ -55,9 +55,10 @@ local STUB_NVIM = concat({ "#!/bin/sh" }, SPEC_NAME, { ' : > "$STUB_DIR/$name.ended"', "fi", "folders=missing", - 'if [ -d "$XDG_STATE_HOME" ] && [ -d "$XDG_CONFIG_HOME" ]; then folders=present; fi', - 'printf "%s\\t%s\\t%s\\t%s\\t%s\\t%s\\n" "${VIMINIT-unset}" "${CLAUDECODE_MINIMAL_INIT-unset}" \\', - ' "${XDG_STATE_HOME-unset}" "${XDG_CONFIG_HOME-unset}" "$folders" "${XDG_DATA_HOME-unset}" >> "$STUB_DIR/env_seen"', + 'if [ -d "$XDG_STATE_HOME" ] && [ -d "$XDG_CONFIG_HOME" ] && [ -d "$TMPDIR" ]; then folders=present; fi', + 'printf "%s\\t%s\\t%s\\t%s\\t%s\\t%s\\t%s\\t%s\\n" "${VIMINIT-unset}" "${CLAUDECODE_MINIMAL_INIT-unset}" \\', + ' "${XDG_STATE_HOME-unset}" "${XDG_CONFIG_HOME-unset}" "$folders" "${XDG_DATA_HOME-unset}" \\', + ' "${TMPDIR-unset}" "${XDG_RUNTIME_DIR-unset}" >> "$STUB_DIR/env_seen"', 'if [ -f "$STUB_DIR/$name.ignore_term" ] || [ -f "$STUB_DIR/$name.outside_term" ]; then', ' if [ -f "$STUB_DIR/$name.ignore_term" ]; then trap "" TERM; fi', ' : > "$STUB_DIR/$name.ready"', @@ -155,9 +156,11 @@ local tee_pipe = assert(io.popen("command -v tee 2> /dev/null")) local real_tee = assert(tee_pipe:read("*l"), "needs tee") tee_pipe:close() -- Copies its input like tee, then exits with the status in $STUB_DIR/tee.exit when that file exists, after the seconds --- in $STUB_DIR/tee.delay when that file exists. +-- in $STUB_DIR/tee.delay when that file exists. When $STUB_DIR/tee.hold exists, it notes in $STUB_DIR/tee.held and waits +-- for $STUB_DIR/tee.go before it starts the real tee, which opens its output file. local STUB_TEE = { "#!/bin/sh", + 'if [ -f "$STUB_DIR/tee.hold" ]; then : > "$STUB_DIR/tee.held"; while [ ! -e "$STUB_DIR/tee.go" ]; do sleep 0.05; done; fi', quote(real_tee) .. ' "$@"', "status=$?", -- After the real tee, so that the output file is opened as early as it is for the real tee. @@ -191,49 +194,73 @@ local mkdir_pipe = assert(io.popen("command -v mkdir 2> /dev/null")) local real_mkdir = assert(mkdir_pipe:read("*l"), "needs mkdir") mkdir_pipe:close() -- Runs the real mkdir. When $STUB_DIR/mkdir.drop_state exists, it then removes each folder named state that it made, so --- that the folder does not exist when the runner opens the status file in it. When $STUB_DIR/mkdir.interrupt exists, --- its first call sends the signal named in that file. SIGINT goes to its process group, which has the runner in it, --- after the real mkdir ran. Any other signal goes to the runner alone, before the real mkdir runs, and the stub then --- waits up to 3 seconds for the runner to end, so a runner that dies at once is gone before the folder exists. +-- that the folder does not exist when the runner opens the status file in it. When $STUB_DIR/mkdir.block_group exists, +-- its first call also makes a folder named group in the folder that it made, where the runner's shell writes a file. +-- When $STUB_DIR/mkdir.interrupt exists, the call that the number in $STUB_DIR/mkdir.at names (the first by default) +-- sends the signal named in that file. SIGINT goes to its process group, which has the runner in it, after the real +-- mkdir ran. Any other signal goes to the runner alone, before the real mkdir runs, and the stub then waits up to 3 +-- seconds for the runner to end or for its rm to stop in the middle (see the rm stub), so that a runner that dies at +-- once is gone, or removing its folder, before this mkdir makes a folder. It lets the rm go afterwards. local STUB_MKDIR = { "#!/bin/sh", + 'n=$(( $(cat "$STUB_DIR/mkdir.count" 2> /dev/null || echo 0) + 1 ))', + 'echo "$n" > "$STUB_DIR/mkdir.count"', + "at=1", + '[ -f "$STUB_DIR/mkdir.at" ] && at=$(cat "$STUB_DIR/mkdir.at")', "sig=", - 'if [ -f "$STUB_DIR/mkdir.interrupt" ] && [ ! -f "$STUB_DIR/mkdir.interrupted" ]; then', - ' : > "$STUB_DIR/mkdir.interrupted"', + 'if [ -f "$STUB_DIR/mkdir.interrupt" ] && [ "$n" = "$at" ]; then', ' sig=$(cat "$STUB_DIR/mkdir.interrupt")', ' if [ "$sig" != INT ]; then', ' kill -s "$sig" "$PPID"', - ' for _ in $(seq 60); do kill -0 "$PPID" 2> /dev/null || break; sleep 0.05; done', + ' for _ in $(seq 60); do kill -0 "$PPID" 2> /dev/null && [ ! -e "$STUB_DIR/rm.held" ] || break; sleep 0.05; done', " fi", "fi", - quote(real_mkdir) .. ' "$@" || exit $?', + quote(real_mkdir) .. ' "$@"', + "status=$?", 'if [ -f "$STUB_DIR/mkdir.drop_state" ]; then', ' for path in "$@"; do case "$path" in */state) rmdir "$path" ;; esac; done', "fi", + 'if [ -f "$STUB_DIR/mkdir.block_group" ] && [ "$n" = 1 ]; then', + ' for path in "$@"; do [ -d "$path" ] && ' .. quote(real_mkdir) .. ' "$path/group"; done', + "fi", 'if [ "$sig" = INT ]; then', ' trap "" INT', " kill -s INT 0", + 'elif [ -n "$sig" ]; then', + ' : > "$STUB_DIR/rm.release"', "fi", + 'exit "$status"', } --- Stops the shell that execs timeout before its first command, as long as $STUB_DIR/a.release does not exist. Bash runs --- this file first in each non-interactive shell when BASH_ENV names it, and only that shell has the name _ for $0. -local HOLD_SHELL = { - 'if [ "$0" = _ ]; then', +-- BASH_ENV files for the shell that execs timeout. Bash reads such a file first in each non-interactive shell, and only +-- that shell has CLAUDECODE_MINIMAL_INIT. The first file stops the shell before its first command, as long as +-- $STUB_DIR/a.release does not exist. The second file wraps echo, which the shell calls to write its process id: the +-- redirection of the call has made the file by then, and the wrapper stops the shell before the write. +local HOLD_START = { + 'if [ -n "${CLAUDECODE_MINIMAL_INIT:-}" ]; then', ' : > "$STUB_DIR/a.held"', ' while [ ! -e "$STUB_DIR/a.release" ]; do sleep 0.05; done', "fi", } +local HOLD_WRITE = { + 'if [ -n "${CLAUDECODE_MINIMAL_INIT:-}" ]; then', + " echo() {", + ' : > "$STUB_DIR/a.held"', + ' while [ ! -e "$STUB_DIR/a.release" ]; do sleep 0.05; done', + ' builtin echo "$@"', + " }", + "fi", +} local rm_pipe = assert(io.popen("command -v rm 2> /dev/null")) local real_rm = assert(rm_pipe:read("*l"), "needs rm") rm_pipe:close() -- Notes in $STUB_DIR/rm.holds_status_file that it holds the status file open. Runs the real rm and then exits with 1 -- when $STUB_DIR/rm.fail exists, as rm does when a folder is not empty. Runs the real rm, except when --- $STUB_DIR/rm.slow or $STUB_DIR/rm.hold exists: then it walks the way rm -rf does, but slowly. It deletes the --- files, waits, and only then removes the folders, so a file that something makes meanwhile in a folder stops the --- removal of the folder ("Directory not empty"), as it does for a slow rm -rf under strace. It waits the seconds in --- rm.slow, or with rm.hold it notes in $STUB_DIR/rm.held and waits for $STUB_DIR/rm.release. +-- $STUB_DIR/rm.slow or $STUB_DIR/rm.hold exists: then it walks the way rm -rf does, but slowly. It lists the folders, +-- deletes the files, waits, and only then removes the listed folders, so a file or a folder that something makes +-- meanwhile stops the removal of its parent ("Directory not empty"), as it does for a slow rm -rf under strace. It waits +-- the seconds in rm.slow, or with rm.hold it notes in $STUB_DIR/rm.held and waits for $STUB_DIR/rm.release. local STUB_RM = { "#!/bin/sh", "for fd in /proc/$$/fd/*; do", @@ -242,14 +269,17 @@ local STUB_RM = { 'if [ -f "$STUB_DIR/rm.fail" ]; then ' .. quote(real_rm) .. ' "$@"; exit 1; fi', 'if [ ! -f "$STUB_DIR/rm.slow" ] && [ ! -f "$STUB_DIR/rm.hold" ]; then exec ' .. quote(real_rm) .. ' "$@"; fi', "shift", - 'for path in "$@"; do find "$path" -type f -delete; done', + ': > "$STUB_DIR/rm.dirs"', + 'for path in "$@"; do find "$path" -depth -type d >> "$STUB_DIR/rm.dirs"; find "$path" -type f -delete; done', 'if [ -f "$STUB_DIR/rm.hold" ]; then', ' : > "$STUB_DIR/rm.held"', ' while [ ! -e "$STUB_DIR/rm.release" ]; do sleep 0.05; done', "else", ' sleep "$(cat "$STUB_DIR/rm.slow")"', "fi", - 'for path in "$@"; do find "$path" -depth -type d -exec rmdir {} +; done', + "status=0", + 'while read -r folder; do rmdir "$folder" || status=1; done < "$STUB_DIR/rm.dirs"', + 'exit "$status"', } local timeout_pipe = assert(io.popen("command -v timeout 2> /dev/null")) @@ -450,13 +480,13 @@ describe("scripts/run_integration_tests_individually.sh", function() end) ---@return table[] seen One entry for each file the stub nvim ran, with the strings viminit, minimal_init, state_home, - ---config_home, folders and data_home + ---config_home, folders, data_home, tmpdir and runtime_dir local function env_seen() local seen = {} local log = assert(io.open(dir .. "/env_seen")) for line in log:lines() do - local viminit, minimal_init, state_home, config_home, folders, data_home = - line:match("^([^\t]*)\t([^\t]*)\t([^\t]*)\t([^\t]*)\t([^\t]*)\t([^\t]*)$") + local viminit, minimal_init, state_home, config_home, folders, data_home, tmpdir, runtime_dir = + line:match("^([^\t]*)\t([^\t]*)\t([^\t]*)\t([^\t]*)\t([^\t]*)\t([^\t]*)\t([^\t]*)\t([^\t]*)$") assert(viminit, line) table.insert(seen, { viminit = viminit, @@ -465,6 +495,8 @@ describe("scripts/run_integration_tests_individually.sh", function() config_home = config_home, folders = folders, data_home = data_home, + tmpdir = tmpdir, + runtime_dir = runtime_dir, }) end log:close() @@ -517,7 +549,7 @@ describe("scripts/run_integration_tests_individually.sh", function() assert.are.equal(os.getenv("XDG_DATA_HOME") or "unset", env_seen()[1].data_home) end) - it("gives the Neovim of each file its own state and config folders and removes them afterwards", function() + it("gives the Neovim of each file its own state, config and temp folders and removes them afterwards", function() local status, output = run_script({ { name = "a_spec.lua", exit = 0, output = summary(1, 0, 0) }, { name = "b_spec.lua", exit = 0, output = summary(1, 0, 0) }, @@ -528,12 +560,16 @@ describe("scripts/run_integration_tests_individually.sh", function() assert.are.equal(2, #seen) assert.are_not.equal(seen[1].state_home, seen[2].state_home) assert.are_not.equal(seen[1].config_home, seen[2].config_home) + assert.are_not.equal(seen[1].tmpdir, seen[2].tmpdir) for _, entry in ipairs(seen) do assert.are_not.equal("unset", entry.state_home) assert.are_not.equal("unset", entry.config_home) assert.are_not.equal(entry.state_home, entry.config_home) - assert.are.equal("present", entry.folders, "both folders exist while the Neovim runs") + assert.are_not.equal(dir .. "/tmp", entry.tmpdir, "the Neovim of the file shares the runner's TMPDIR") + assert.are.equal(entry.tmpdir, entry.runtime_dir, "Neovim puts its socket folders in TMPDIR and XDG_RUNTIME_DIR") + assert.are.equal("present", entry.folders, "the three folders exist while the Neovim runs") shell("test ! -e " .. quote(entry.state_home) .. " && test ! -e " .. quote(entry.config_home)) + shell("test ! -e " .. quote(entry.tmpdir)) end end) @@ -573,12 +609,15 @@ describe("scripts/run_integration_tests_individually.sh", function() ---@field late_tee number|nil The file's tee ends this many seconds after its input ends ---@field slow_rm number|nil The runner's rm waits this many seconds between deleting files and removing folders ---@field rm_fails boolean|nil The runner's rm removes what it is told to and then exits with 1 - ---@field hold boolean|nil Send the signal while the shell that execs timeout waits before its first command, and let it - --- go after the runner is gone - ---@field hold_cleanup boolean|nil With `hold`, let the shell go while the runner's rm waits, after it deleted the files - --- and before it removed the folders - ---@field interrupt "mktemp"|"mkdir"|nil The first call of that command sends the signal itself, and nothing else does. - --- A mktemp sends only SIGINT + ---@field hold "start"|"write"|nil Send the signal while the shell that execs timeout waits, before its first command + --- (start) or after its redirection made the process id file and before it writes it (write), and let it go after the + --- runner is gone + ---@field during_rm "launcher"|"tee"|nil Let the shell that execs timeout (which then waits before its first command) + --- or the file's tee go while the runner's rm waits, after it deleted the files and before it removed the folders + ---@field interrupt "mktemp"|"mkdir"|nil A call of that command sends the signal itself, and nothing else does. A mktemp + --- sends only SIGINT + ---@field interrupt_at integer|nil The number of the mkdir call that sends the signal, 1 by default + ---@field rm_holds boolean|nil The runner's rm stops in the middle, as with `during_rm`, until that mkdir call ends ---@param signal string The signal to send ---@param run SignalRun @@ -606,15 +645,22 @@ describe("scripts/run_integration_tests_individually.sh", function() if run.rm_fails then write(dir .. "/rm.fail", {}) end - if run.hold then - write(dir .. "/hold.sh", HOLD_SHELL) + local hold = run.hold or run.during_rm == "launcher" and "start" or nil + if hold then + write(dir .. "/hold.sh", hold == "write" and HOLD_WRITE or HOLD_START) + end + if run.during_rm == "tee" then + write(dir .. "/tee.hold", {}) end - if run.hold_cleanup then + if run.during_rm or run.rm_holds then write(dir .. "/rm.hold", {}) end if run.interrupt then write(dir .. "/" .. run.interrupt .. ".interrupt", { signal }) end + if run.interrupt_at then + write(dir .. "/" .. run.interrupt .. ".at", { tostring(run.interrupt_at) }) + end local temp = dir .. "/tmp" shell("mkdir -p " .. quote(temp)) local runner_out = " > " .. quote(dir .. "/runner.out") .. " 2>&1" @@ -625,15 +671,19 @@ describe("scripts/run_integration_tests_individually.sh", function() job = job .. " " .. quote(script) .. " > " .. quote(dir .. "/caller.out") .. " 2>&1" target = "-- -$pid" end - local gate = run.window and "/slow_spec.lua.timeout_started" or run.hold and "/a.held" or "/slow_spec.lua.started" + local gate = run.window and "/slow_spec.lua.timeout_started" + or hold and "/a.held" + or run.during_rm == "tee" and "/tee.held" + or "/slow_spec.lua.started" local send = "" if not run.interrupt then send = "for _ in $(seq 100); do [ -e " .. quote(dir .. gate) .. " ] && break; sleep 0.1; done; " send = send .. "kill -" .. signal .. " " .. target .. "; " end - if run.hold_cleanup then + if run.during_rm then + local release = run.during_rm == "tee" and "/tee.go" or "/a.release" send = send .. "for _ in $(seq 100); do [ -e " .. quote(dir .. "/rm.held") .. " ] && break; sleep 0.1; done; " - send = send .. ": > " .. quote(dir .. "/a.release") .. "; sleep 1; : > " .. quote(dir .. "/rm.release") .. "; " + send = send .. ": > " .. quote(dir .. release) .. "; sleep 1; : > " .. quote(dir .. "/rm.release") .. "; " end -- `set -m` gives the background job its own process group and leaves SIGINT unignored, as a runner has at a prompt. local inner = "cd " @@ -646,13 +696,13 @@ describe("scripts/run_integration_tests_individually.sh", function() .. quote(dir) .. " REAL_TIMEOUT=" .. quote(real_timeout) - .. (run.hold and " BASH_ENV=" .. quote(dir .. "/hold.sh") or "") + .. (hold and " BASH_ENV=" .. quote(dir .. "/hold.sh") or "") .. "; set -m; " .. job .. " & pid=$!; " .. send .. "wait $pid 2> /dev/null; echo status=$?; " - .. (run.hold and not run.hold_cleanup and ": > " .. quote(dir .. "/a.release") .. "; " or "") + .. (hold and not run.during_rm and ": > " .. quote(dir .. "/a.release") .. "; " or "") .. "sleep 3" local pipe = assert(io.popen("bash -c " .. quote(inner))) local output = pipe:read("*a") @@ -807,34 +857,46 @@ describe("scripts/run_integration_tests_individually.sh", function() assert.is_false(result.ended) end) - for _, command in ipairs({ "mktemp", "mkdir" }) do - it("removes its temp items when SIGINT comes right after its first " .. command .. " made something", function() - local result = signal_runner("INT", { interrupt = command }) + for _, case in ipairs({ { "mktemp", 1 }, { "mkdir", 1 }, { "mkdir", 2 } }) do + local command, at = case[1], case[2] + it( + "removes its temp items when SIGINT comes right after its " .. command .. " number " .. at .. " made them", + function() + local result = signal_runner("INT", { interrupt = command, interrupt_at = command == "mkdir" and at or nil }) - assert.are.equal("130", result.status, result.report) - assert.are.equal("", result.left, "the runner left a temp item that it made before it stored the name") - assert.is_nil(io.open(dir .. "/ran"), "the file's Neovim started after the runner died") - end) + assert.are.equal("130", result.status, result.report) + assert.are.equal("", result.left, "the runner left a temp item that it made before it stored the name") + assert.is_nil(io.open(dir .. "/ran"), "the file's Neovim started after the runner died") + end + ) end for _, case in ipairs({ { "SIGTERM", "TERM", "143" }, { "SIGHUP", "HUP", "129" } }) do local name, signal, expected_status = case[1], case[2], case[3] - it("removes its temp folder when " .. name .. " comes while mkdir makes it", function() - if signal_is_ignored(signal) then - pending(name .. " is ignored in this environment, so the runner ignores it too") - end - local result = signal_runner(signal, { interrupt = "mkdir" }) + for _, at in ipairs({ 1, 2 }) do + it("removes its temp folder when " .. name .. " comes while its mkdir number " .. at .. " runs", function() + if signal_is_ignored(signal) then + pending(name .. " is ignored in this environment, so the runner ignores it too") + end + local result = signal_runner(signal, { interrupt = "mkdir", interrupt_at = at, rm_holds = at == 2 }) - assert.are.equal(expected_status, result.status, result.report) - assert.are.equal("", result.left, "mkdir made the folder after the runner had cleaned up") - assert.is_nil(io.open(dir .. "/ran"), "the file's Neovim started after the runner died") - end) + assert.are.equal(expected_status, result.status, result.report) + assert.are.equal("", result.left, "mkdir made a folder after the runner had cleaned up") + assert.is_nil(io.open(dir .. "/ran"), "the file's Neovim started after the runner died") + end) + end end - for _, case in ipairs({ { "SIGTERM", "TERM", "143" }, { "SIGINT", "INT", "130" } }) do - local name, signal, expected_status = case[1], case[2], case[3] - it("starts no Neovim when " .. name .. " comes before the shell of the file wrote its process id", function() - local result = signal_runner(signal, { hold = true }) + for _, case in ipairs({ + { "SIGTERM", "TERM", "143", "start" }, + { "SIGINT", "INT", "130", "start" }, + { "SIGINT", "INT", "130", "write" }, + }) do + local name, signal, expected_status, hold = case[1], case[2], case[3], case[4] + local where = hold == "start" and "before the shell of the file made its process id file" + or "after the shell of the file made its process id file and before it wrote the id" + it("starts no Neovim when " .. name .. " comes " .. where, function() + local result = signal_runner(signal, { hold = hold }) assert.are.equal(expected_status, result.status, result.report) assert.are.equal("", result.left, "the shell of the file made a file after the runner cleaned up") @@ -843,13 +905,33 @@ describe("scripts/run_integration_tests_individually.sh", function() end it("starts no Neovim when SIGINT comes while its rm has deleted the files and not yet the folders", function() - local result = signal_runner("INT", { hold = true, hold_cleanup = true }) + local result = signal_runner("INT", { during_rm = "launcher" }) assert.are.equal("130", result.status, result.report) assert.are.equal("", result.left, "the shell of the file made a file after the runner cleaned up") assert.is_nil(io.open(dir .. "/slow_spec.lua.started"), "the file's Neovim started while the runner cleaned up") end) + for _, case in ipairs({ { "SIGTERM", "TERM", "143" }, { "SIGINT", "INT", "130" } }) do + local name, signal, expected_status = case[1], case[2], case[3] + it("leaves no output file when " .. name .. " comes before tee opened it and tee opens it during the rm", function() + local result = signal_runner(signal, { during_rm = "tee" }) + + assert.are.equal(expected_status, result.status, result.report) + assert.are.equal("", result.left, "tee made its output file in the folder that the runner was removing") + end) + end + + it("starts no Neovim when the shell of the file cannot write its process id", function() + write(dir .. "/mkdir.block_group", {}) + local status, output, ran = run_script({ + { name = "a_spec.lua", exit = 0, output = summary(1, 0, 0) }, + }) + + assert.are_not.equal("0", status, output) + assert.are.same({}, ran) + end) + it("does not remove a folder that it did not make, when the name that mktemp printed exists", function() shell("mkdir " .. quote(dir .. "/taken") .. " && : > " .. quote(dir .. "/taken/keep")) write(dir .. "/mktemp.name", { dir .. "/taken" }) From fcce6f6c77d0247f427ca5616bc4e73537ddce24 Mon Sep 17 00:00:00 2001 From: Thomas Kosiewski Date: Fri, 13 Nov 2026 02:34:37 +0000 Subject: [PATCH 07/14] fix(scripts): remove the renamed folder too when a second signal stops the first cleanup MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The first cleanup renames the work folder, and a second SIGINT can stop its rm half way. The second cleanup then found no folder at the old path and left the renamed one. It now removes both names. A test makes the first rm die by a second SIGINT and expects no folder left. --- _Generated with [`xum`](https://github.com/coder/xum) • Model: `anthropic:claude-sonnet-5-5` • Thinking: `xhigh`_ --- scripts/README.md | 2 +- scripts/run_integration_tests_individually.sh | 22 +++++++++-------- tests/unit/integration_runner_spec.lua | 24 +++++++++++++++++-- 3 files changed, 35 insertions(+), 13 deletions(-) diff --git a/scripts/README.md b/scripts/README.md index 2dbe199f..d8dba05d 100644 --- a/scripts/README.md +++ b/scripts/README.md @@ -84,7 +84,7 @@ The Lua repros run from the repository root with `nvim --headless -u NONE -l scr - CAUTION: Do not run the token-sending clients (`repro_issue_238.sh`, `repro_issue_248.sh`, and `repro_issue_277.sh`) on a shared machine. They pass the auth token on the `websocat` command line (`--header`), where other users can read it with `ps` [repro_issue_238.sh:WEBSOCAT] [repro_issue_248.sh:TOKEN] [repro_issue_277.sh:TOKEN]. - `repro_issue_238.sh`, `repro_issue_248.sh`, and `repro_issue_277.sh` connect to the plugin server on `127.0.0.1` [repro_issue_238.sh:PORT] [repro_issue_248.sh:PORT] [repro_issue_277.sh:PORT]. -- `run_integration_tests_individually.sh` creates one temp folder per test, and puts the output, the file that holds the process id of the file's `timeout`, and the `XDG_STATE_HOME`, `XDG_CONFIG_HOME` and `TMPDIR` that the file's two Neovims share in it, and deletes the folder after the file ran. The runner stores the name of the folder (from `mktemp -u`) before it makes the folder, so a signal never finds a folder that the runner does not know, and `mkdir` fails when the name exists, and the runner then drops the name. Only a signal that comes before it drops the name can make it remove a folder that it did not make. The runner opens a status file in the state folder before it starts the file, and the file's subshell writes the exit statuses of nvim and of `tee` to that open file when the file ends. A subshell that ends while the cleanup removes the folder opens no path, so it leaves nothing behind. When the runner cannot read the file back, for example because the Neovim deleted the state folder, it uses the exit status of the subshell, which is the status of nvim, or of `tee` when nvim exited 0, and names it as the status of nvim. When it gets SIGTERM, SIGHUP or SIGINT (Ctrl-C) while a file runs, it renames the folder, kills that `timeout`, then the process group of the file, deletes the renamed folder and dies by that signal as soon as the command that it waits for ends, even when a delete fails, so a calling shell sees status 143, 129 or 130, and a shell loop around the runner stops after Ctrl-C. Killing the `timeout` first covers the moments before it has made its own group, when no Neovim runs yet. Both kills use KILL, so a Neovim that ignores SIGTERM stops too. The shell of the file writes its process id to the file in the folder, then looks for the folder, and it starts no Neovim when it finds none. The runner renames the folder before it reads the process id, so one of the two sees the other: a signal at any moment leaves no Neovim that the runner cannot find. SIGKILL gives the runner no time to clean up, so the folder stays. A runner that leads its own session on a terminal, for example one that a terminal emulator starts directly, hangs up the foreground process group of the terminal when it dies, so the shell of the file dies too and starts no Neovim. Without a terminal that shell goes on, also when it has not written its process id yet, and its Neovim runs until its own timeout. Bash ignores SIGQUIT, so the file runs to its end. It sets `VIMINIT`, `CLAUDECODE_MINIMAL_INIT`, `XDG_STATE_HOME`, `XDG_CONFIG_HOME`, `TMPDIR` and `XDG_RUNTIME_DIR` for that file's Neovims and the processes they start only, as a prefix of one command, and leaves `XDG_DATA_HOME` alone because plenary lives there. A run writes nothing under `~/.local/state/nvim`, leaves no `nvim.` socket folder in the developer's `TMPDIR`, even when it kills a Neovim, and the developer's `ftplugin` and `after` files under `~/.config/nvim` do not run [run_integration_tests_individually.sh:work_dir, temp_output, group_file, state_dir, config_dir, tmp_dir]. +- `run_integration_tests_individually.sh` creates one temp folder per test, and puts the output, the file that holds the process id of the file's `timeout`, and the `XDG_STATE_HOME`, `XDG_CONFIG_HOME` and `TMPDIR` that the file's two Neovims share in it, and deletes the folder after the file ran. The runner stores the name of the folder (from `mktemp -u`) before it makes the folder, so a signal never finds a folder that the runner does not know, and `mkdir` fails when the name exists, and the runner then drops the name. Only a signal that comes before it drops the name can make it remove a folder that it did not make. The runner opens a status file in the state folder before it starts the file, and the file's subshell writes the exit statuses of nvim and of `tee` to that open file when the file ends. A subshell that ends while the cleanup removes the folder opens no path, so it leaves nothing behind. When the runner cannot read the file back, for example because the Neovim deleted the state folder, it uses the exit status of the subshell, which is the status of nvim, or of `tee` when nvim exited 0, and names it as the status of nvim. When it gets SIGTERM, SIGHUP or SIGINT (Ctrl-C) while a file runs, it renames the folder, kills that `timeout`, then the process group of the file, deletes the renamed folder and dies by that signal as soon as the command that it waits for ends, even when a delete fails, so a calling shell sees status 143, 129 or 130, and a shell loop around the runner stops after Ctrl-C. Killing the `timeout` first covers the moments before it has made its own group, when no Neovim runs yet. Both kills use KILL, so a Neovim that ignores SIGTERM stops too. The shell of the file writes its process id to the file in the folder, then looks for the folder, and it starts no Neovim when it finds none. The runner renames the folder before it reads the process id, so one of the two sees the other. SIGKILL gives the runner no time to clean up, so the folder stays. A runner that leads its own session on a terminal, for example one that a terminal emulator starts directly, hangs up the foreground process group of the terminal when it dies, so the shell of the file dies too and starts no Neovim. Without a terminal that shell goes on, also when it has not written its process id yet, and its Neovim runs until its own timeout. Bash ignores SIGQUIT, so the file runs to its end. It sets `VIMINIT`, `CLAUDECODE_MINIMAL_INIT`, `XDG_STATE_HOME`, `XDG_CONFIG_HOME`, `TMPDIR` and `XDG_RUNTIME_DIR` for that file's Neovims and the processes they start only, as a prefix of one command, and leaves `XDG_DATA_HOME` alone because plenary lives there. A run writes nothing under `~/.local/state/nvim`, leaves no `nvim.` socket folder in the developer's `TMPDIR`, even when it kills a Neovim, and the developer's `ftplugin` and `after` files under `~/.config/nvim` do not run [run_integration_tests_individually.sh:work_dir, temp_output, group_file, state_dir, config_dir, tmp_dir]. - `repro_issue_231.lua` deletes `/__issue_231_repro__.md` [repro_issue_231.lua:try_open_diff]. - `repro_issue_70.sh` and `repro_issue_277.sh` make a work dir with `mktemp -d` (`${TMPDIR:-/tmp}/issue70.XXXXXX` or `issue277.XXXXXX`) and an `att-home` folder in it before they check their tools. They install `trap cleanup EXIT` after that check, so an exit 3 for a missing tool leaves the work dir behind. The trap also runs on SIGTERM, SIGINT and SIGHUP. It destroys the `agent-tty` sessions and removes the work dir. agent-tty still leaves an empty socket folder `/tmp/agent-tty/<8 hex digits>` (it ignores `TMPDIR`) and an entry for the deleted home in `${XDG_STATE_HOME:-$HOME/.local/state}/agent-tty/homes.json`, one of each per run. The sessions inherit the caller's environment, so `CLAUDE_CONFIG_DIR` and the `XDG_*` variables reach the Neovim and the `claude` CLI that they run. `repro_issue_70.sh` launches the real `claude` CLI, and it also runs `claude --version` once outside agent-tty. Its cleanup runs `pkill -f "repro_issue_70_probe.lua $REPO_ROOT"`, which also kills the probe of another run in the same checkout [repro_issue_70.sh:WORK, cleanup] [repro_issue_277.sh:WORK, cleanup]. - `repro_issue_70_probe.lua` starts the real server with `auto_start = true`, so the plugin writes a real `PORT.lock` into the user's lock directory. Other Claude CLIs can see that lock file until `claudecode.stop()` removes it at the end. When the removal fails, `stop` only logs a warning, so a lock file that the removal failed to delete stays [repro_issue_70_probe.lua:claudecode.setup, claudecode.stop] [lua/claudecode/init.lua:M.stop]. diff --git a/scripts/run_integration_tests_individually.sh b/scripts/run_integration_tests_individually.sh index a7f0c6f8..0153a268 100755 --- a/scripts/run_integration_tests_individually.sh +++ b/scripts/run_integration_tests_individually.sh @@ -17,24 +17,26 @@ FILE_TIMEOUT=40s # state folder again. It kills `timeout` itself first: until timeout has made its own group, no Neovim runs yet. It # uses KILL for both, because with `timeout` dead its -k grace never starts, and a Neovim that ignores TERM would run # on. -# The cleanup renames the folder before it reads the process id file, so the old path is gone at once, and rm removes -# the renamed folder. The shell that starts the Neovim writes its process id to the file at the old path, then looks for -# the folder at the old path (the `bash -c` line in run_test_file). So one of them sees the other: a shell that wrote -# its id too late for the cleanup to read it finds no folder, and starts no Neovim. +# The cleanup renames the folder before it reads the process id file, so the old path is gone at once. It removes both +# names, so a second signal that stops the first cleanup leaves nothing to the second. The shell that starts the Neovim +# writes its process id to the file at the old path, then looks for the folder at the old path (the `bash -c` line in +# run_test_file). So one of them sees the other: a shell that wrote its id too late for the cleanup to read it finds no +# folder, and starts no Neovim. CURRENT_WORK="" CURRENT_FILE_RUNS="" # shellcheck disable=SC2329 # The traps below run it. cleanup_current_file() { - local gone timeout_pid + local group timeout_pid [[ -n $CURRENT_WORK ]] || return 0 - gone=$CURRENT_WORK.gone - mv "$CURRENT_WORK" "$gone" 2>/dev/null || gone=$CURRENT_WORK - if [[ -n $CURRENT_FILE_RUNS && -s $gone/group ]]; then - timeout_pid=$(cat "$gone/group") + mv "$CURRENT_WORK" "$CURRENT_WORK.gone" 2>/dev/null || true + group=$CURRENT_WORK.gone/group + [[ -s $group ]] || group=$CURRENT_WORK/group + if [[ -n $CURRENT_FILE_RUNS && -s $group ]]; then + timeout_pid=$(cat "$group") kill -KILL "$timeout_pid" 2>/dev/null || true kill -KILL -- "-$timeout_pid" 2>/dev/null || true fi - rm -rf "$gone" + rm -rf "$CURRENT_WORK.gone" "$CURRENT_WORK" } # A signal reaches the runner's own process group, and timeout leaves that group, so the file runs in the background and # `wait` below returns at once when a trap runs. Bash runs a trap after the foreground command that it waits for, so a diff --git a/tests/unit/integration_runner_spec.lua b/tests/unit/integration_runner_spec.lua index 552e48f2..f8f7a546 100644 --- a/tests/unit/integration_runner_spec.lua +++ b/tests/unit/integration_runner_spec.lua @@ -255,7 +255,9 @@ local HOLD_WRITE = { local rm_pipe = assert(io.popen("command -v rm 2> /dev/null")) local real_rm = assert(rm_pipe:read("*l"), "needs rm") rm_pipe:close() --- Notes in $STUB_DIR/rm.holds_status_file that it holds the status file open. Runs the real rm and then exits with 1 +-- Notes in $STUB_DIR/rm.holds_status_file that it holds the status file open. When $STUB_DIR/rm.die exists, its first +-- call deletes the files, sends SIGINT to its process group, which has the runner in it, and dies of it, as an rm does +-- that a second Ctrl-C stops. Runs the real rm and then exits with 1 -- when $STUB_DIR/rm.fail exists, as rm does when a folder is not empty. Runs the real rm, except when -- $STUB_DIR/rm.slow or $STUB_DIR/rm.hold exists: then it walks the way rm -rf does, but slowly. It lists the folders, -- deletes the files, waits, and only then removes the listed folders, so a file or a folder that something makes @@ -267,10 +269,17 @@ local STUB_RM = { ' case "$(readlink "$fd" 2> /dev/null)" in */exit_statuses*) : > "$STUB_DIR/rm.holds_status_file" ;; esac', "done", 'if [ -f "$STUB_DIR/rm.fail" ]; then ' .. quote(real_rm) .. ' "$@"; exit 1; fi', + 'if [ -f "$STUB_DIR/rm.die" ] && [ ! -f "$STUB_DIR/rm.died" ]; then', + ' : > "$STUB_DIR/rm.died"', + " shift", + ' for path in "$@"; do find "$path" -type f -delete; done 2> /dev/null', + " kill -s INT 0", + " sleep 5", + "fi", 'if [ ! -f "$STUB_DIR/rm.slow" ] && [ ! -f "$STUB_DIR/rm.hold" ]; then exec ' .. quote(real_rm) .. ' "$@"; fi', "shift", ': > "$STUB_DIR/rm.dirs"', - 'for path in "$@"; do find "$path" -depth -type d >> "$STUB_DIR/rm.dirs"; find "$path" -type f -delete; done', + 'for path in "$@"; do find "$path" -depth -type d >> "$STUB_DIR/rm.dirs"; find "$path" -type f -delete; done 2> /dev/null', 'if [ -f "$STUB_DIR/rm.hold" ]; then', ' : > "$STUB_DIR/rm.held"', ' while [ ! -e "$STUB_DIR/rm.release" ]; do sleep 0.05; done', @@ -618,6 +627,7 @@ describe("scripts/run_integration_tests_individually.sh", function() --- sends only SIGINT ---@field interrupt_at integer|nil The number of the mkdir call that sends the signal, 1 by default ---@field rm_holds boolean|nil The runner's rm stops in the middle, as with `during_rm`, until that mkdir call ends + ---@field rm_dies boolean|nil The first rm of the runner deletes the files and then dies by a second SIGINT ---@param signal string The signal to send ---@param run SignalRun @@ -655,6 +665,9 @@ describe("scripts/run_integration_tests_individually.sh", function() if run.during_rm or run.rm_holds then write(dir .. "/rm.hold", {}) end + if run.rm_dies then + write(dir .. "/rm.die", {}) + end if run.interrupt then write(dir .. "/" .. run.interrupt .. ".interrupt", { signal }) end @@ -857,6 +870,13 @@ describe("scripts/run_integration_tests_individually.sh", function() assert.is_false(result.ended) end) + it("removes its folder when a second SIGINT stops the rm of the first cleanup", function() + local result = signal_runner("INT", { rm_dies = true }) + + assert.are.equal("130", result.status, result.report) + assert.are.equal("", result.left, "the second cleanup left the folder that the first cleanup renamed") + end) + for _, case in ipairs({ { "mktemp", 1 }, { "mkdir", 1 }, { "mkdir", 2 } }) do local command, at = case[1], case[2] it( From 53e5166e158f57c316c185ef9b4ac4201cba7b1b Mon Sep 17 00:00:00 2001 From: Thomas Kosiewski Date: Fri, 13 Nov 2026 02:52:29 +0000 Subject: [PATCH 08/14] test(scripts): cover the runner's cleanup when the rename of its folder fails MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A mv stub that fails makes the cleanup read the process id file at the old path and remove the folder under the old name. Without the fallback to the old path in either place, the test fails. --- _Generated with [`xum`](https://github.com/coder/xum) • Model: `anthropic:claude-sonnet-5-5` • Thinking: `xhigh`_ --- tests/unit/integration_runner_spec.lua | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/tests/unit/integration_runner_spec.lua b/tests/unit/integration_runner_spec.lua index f8f7a546..4346eb79 100644 --- a/tests/unit/integration_runner_spec.lua +++ b/tests/unit/integration_runner_spec.lua @@ -232,6 +232,16 @@ local STUB_MKDIR = { 'exit "$status"', } +local mv_pipe = assert(io.popen("command -v mv 2> /dev/null")) +local real_mv = assert(mv_pipe:read("*l"), "needs mv") +mv_pipe:close() +-- Runs the real mv, except when $STUB_DIR/mv.fail exists: then it fails and moves nothing. +local STUB_MV = { + "#!/bin/sh", + 'if [ -f "$STUB_DIR/mv.fail" ]; then exit 1; fi', + "exec " .. quote(real_mv) .. ' "$@"', +} + -- BASH_ENV files for the shell that execs timeout. Bash reads such a file first in each non-interactive shell, and only -- that shell has CLAUDECODE_MINIMAL_INIT. The first file stops the shell before its first command, as long as -- $STUB_DIR/a.release does not exist. The second file wraps echo, which the shell calls to write its process id: the @@ -348,6 +358,7 @@ describe("scripts/run_integration_tests_individually.sh", function() write(dir .. "/bin/rm", STUB_RM, true) write(dir .. "/bin/mktemp", STUB_MKTEMP, true) write(dir .. "/bin/mkdir", STUB_MKDIR, true) + write(dir .. "/bin/mv", STUB_MV, true) end) after_each(function() @@ -628,6 +639,7 @@ describe("scripts/run_integration_tests_individually.sh", function() ---@field interrupt_at integer|nil The number of the mkdir call that sends the signal, 1 by default ---@field rm_holds boolean|nil The runner's rm stops in the middle, as with `during_rm`, until that mkdir call ends ---@field rm_dies boolean|nil The first rm of the runner deletes the files and then dies by a second SIGINT + ---@field mv_fails boolean|nil The runner's mv, which renames the folder, fails ---@param signal string The signal to send ---@param run SignalRun @@ -668,6 +680,9 @@ describe("scripts/run_integration_tests_individually.sh", function() if run.rm_dies then write(dir .. "/rm.die", {}) end + if run.mv_fails then + write(dir .. "/mv.fail", {}) + end if run.interrupt then write(dir .. "/" .. run.interrupt .. ".interrupt", { signal }) end @@ -877,6 +892,14 @@ describe("scripts/run_integration_tests_individually.sh", function() assert.are.equal("", result.left, "the second cleanup left the folder that the first cleanup renamed") end) + it("stops the file and removes its folder under the old name when the rename of the folder fails", function() + local result = signal_runner("INT", { mv_fails = true }) + + assert.are.equal("130", result.status, result.report) + assert.are.equal("", result.left, "the runner left its folder, or its Neovim's grandchild ran on") + assert.is_false(result.ended) + end) + for _, case in ipairs({ { "mktemp", 1 }, { "mkdir", 1 }, { "mkdir", 2 } }) do local command, at = case[1], case[2] it( From dd55cffdecdba3e94d3d0cad3c0b85aebc6470ae Mon Sep 17 00:00:00 2001 From: Thomas Kosiewski Date: Fri, 13 Nov 2026 03:26:22 +0000 Subject: [PATCH 09/14] fix(scripts): ignore INT, TERM and HUP while the runner's cleanup runs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A second Ctrl-C reaches the rm of the cleanup too and stops it half way. With bash 3.2 (the macOS /bin/bash) the runner then dies with 130 without running its handler again, and the renamed folder stays in TMPDIR. The handler now ignores the three signals from its first line on, and its rm inherits that. The test sends a second SIGINT from the stub rm and expects the rm to live on, the folder to be gone and status 130. It fails on the earlier runner on bash 5 too, because the stub rm died there. It also fails the whole spec's earlier version of the test under bash 3.2. --- _Generated with [`xum`](https://github.com/coder/xum) • Model: `anthropic:claude-sonnet-5-5` • Thinking: `xhigh`_ --- scripts/README.md | 2 +- scripts/run_integration_tests_individually.sh | 9 ++++-- tests/unit/integration_runner_spec.lua | 31 ++++++++++--------- 3 files changed, 24 insertions(+), 18 deletions(-) diff --git a/scripts/README.md b/scripts/README.md index d8dba05d..519cdcf4 100644 --- a/scripts/README.md +++ b/scripts/README.md @@ -84,7 +84,7 @@ The Lua repros run from the repository root with `nvim --headless -u NONE -l scr - CAUTION: Do not run the token-sending clients (`repro_issue_238.sh`, `repro_issue_248.sh`, and `repro_issue_277.sh`) on a shared machine. They pass the auth token on the `websocat` command line (`--header`), where other users can read it with `ps` [repro_issue_238.sh:WEBSOCAT] [repro_issue_248.sh:TOKEN] [repro_issue_277.sh:TOKEN]. - `repro_issue_238.sh`, `repro_issue_248.sh`, and `repro_issue_277.sh` connect to the plugin server on `127.0.0.1` [repro_issue_238.sh:PORT] [repro_issue_248.sh:PORT] [repro_issue_277.sh:PORT]. -- `run_integration_tests_individually.sh` creates one temp folder per test, and puts the output, the file that holds the process id of the file's `timeout`, and the `XDG_STATE_HOME`, `XDG_CONFIG_HOME` and `TMPDIR` that the file's two Neovims share in it, and deletes the folder after the file ran. The runner stores the name of the folder (from `mktemp -u`) before it makes the folder, so a signal never finds a folder that the runner does not know, and `mkdir` fails when the name exists, and the runner then drops the name. Only a signal that comes before it drops the name can make it remove a folder that it did not make. The runner opens a status file in the state folder before it starts the file, and the file's subshell writes the exit statuses of nvim and of `tee` to that open file when the file ends. A subshell that ends while the cleanup removes the folder opens no path, so it leaves nothing behind. When the runner cannot read the file back, for example because the Neovim deleted the state folder, it uses the exit status of the subshell, which is the status of nvim, or of `tee` when nvim exited 0, and names it as the status of nvim. When it gets SIGTERM, SIGHUP or SIGINT (Ctrl-C) while a file runs, it renames the folder, kills that `timeout`, then the process group of the file, deletes the renamed folder and dies by that signal as soon as the command that it waits for ends, even when a delete fails, so a calling shell sees status 143, 129 or 130, and a shell loop around the runner stops after Ctrl-C. Killing the `timeout` first covers the moments before it has made its own group, when no Neovim runs yet. Both kills use KILL, so a Neovim that ignores SIGTERM stops too. The shell of the file writes its process id to the file in the folder, then looks for the folder, and it starts no Neovim when it finds none. The runner renames the folder before it reads the process id, so one of the two sees the other. SIGKILL gives the runner no time to clean up, so the folder stays. A runner that leads its own session on a terminal, for example one that a terminal emulator starts directly, hangs up the foreground process group of the terminal when it dies, so the shell of the file dies too and starts no Neovim. Without a terminal that shell goes on, also when it has not written its process id yet, and its Neovim runs until its own timeout. Bash ignores SIGQUIT, so the file runs to its end. It sets `VIMINIT`, `CLAUDECODE_MINIMAL_INIT`, `XDG_STATE_HOME`, `XDG_CONFIG_HOME`, `TMPDIR` and `XDG_RUNTIME_DIR` for that file's Neovims and the processes they start only, as a prefix of one command, and leaves `XDG_DATA_HOME` alone because plenary lives there. A run writes nothing under `~/.local/state/nvim`, leaves no `nvim.` socket folder in the developer's `TMPDIR`, even when it kills a Neovim, and the developer's `ftplugin` and `after` files under `~/.config/nvim` do not run [run_integration_tests_individually.sh:work_dir, temp_output, group_file, state_dir, config_dir, tmp_dir]. +- `run_integration_tests_individually.sh` creates one temp folder per test, and puts the output, the file that holds the process id of the file's `timeout`, and the `XDG_STATE_HOME`, `XDG_CONFIG_HOME` and `TMPDIR` that the file's two Neovims share in it, and deletes the folder after the file ran. The runner stores the name of the folder (from `mktemp -u`) before it makes the folder, so a signal never finds a folder that the runner does not know, and `mkdir` fails when the name exists, and the runner then drops the name. Only a signal that comes before it drops the name can make it remove a folder that it did not make. The runner opens a status file in the state folder before it starts the file, and the file's subshell writes the exit statuses of nvim and of `tee` to that open file when the file ends. A subshell that ends while the cleanup removes the folder opens no path, so it leaves nothing behind. When the runner cannot read the file back, for example because the Neovim deleted the state folder, it uses the exit status of the subshell, which is the status of nvim, or of `tee` when nvim exited 0, and names it as the status of nvim. When it gets SIGTERM, SIGHUP or SIGINT (Ctrl-C) while a file runs, it renames the folder, kills that `timeout`, then the process group of the file, deletes the renamed folder and dies by that signal as soon as the command that it waits for ends, even when a delete fails, so a calling shell sees status 143, 129 or 130, and a shell loop around the runner stops after Ctrl-C. It ignores INT, TERM and HUP while it cleans up, so a second Ctrl-C does not stop the delete half way (with bash 3.2 it would leave the renamed folder). Killing the `timeout` first covers the moments before it has made its own group, when no Neovim runs yet. Both kills use KILL, so a Neovim that ignores SIGTERM stops too. The shell of the file writes its process id to the file in the folder, then looks for the folder, and it starts no Neovim when it finds none. The runner renames the folder before it reads the process id, so one of the two sees the other. SIGKILL gives the runner no time to clean up, so the folder stays. A runner that leads its own session on a terminal, for example one that a terminal emulator starts directly, hangs up the foreground process group of the terminal when it dies, so the shell of the file dies too and starts no Neovim. Without a terminal that shell goes on, also when it has not written its process id yet, and its Neovim runs until its own timeout. Bash ignores SIGQUIT, so the file runs to its end. It sets `VIMINIT`, `CLAUDECODE_MINIMAL_INIT`, `XDG_STATE_HOME`, `XDG_CONFIG_HOME`, `TMPDIR` and `XDG_RUNTIME_DIR` for that file's Neovims and the processes they start only, as a prefix of one command, and leaves `XDG_DATA_HOME` alone because plenary lives there. A run writes nothing under `~/.local/state/nvim`, leaves no `nvim.` socket folder in the developer's `TMPDIR`, even when it kills a Neovim, and the developer's `ftplugin` and `after` files under `~/.config/nvim` do not run [run_integration_tests_individually.sh:work_dir, temp_output, group_file, state_dir, config_dir, tmp_dir]. - `repro_issue_231.lua` deletes `/__issue_231_repro__.md` [repro_issue_231.lua:try_open_diff]. - `repro_issue_70.sh` and `repro_issue_277.sh` make a work dir with `mktemp -d` (`${TMPDIR:-/tmp}/issue70.XXXXXX` or `issue277.XXXXXX`) and an `att-home` folder in it before they check their tools. They install `trap cleanup EXIT` after that check, so an exit 3 for a missing tool leaves the work dir behind. The trap also runs on SIGTERM, SIGINT and SIGHUP. It destroys the `agent-tty` sessions and removes the work dir. agent-tty still leaves an empty socket folder `/tmp/agent-tty/<8 hex digits>` (it ignores `TMPDIR`) and an entry for the deleted home in `${XDG_STATE_HOME:-$HOME/.local/state}/agent-tty/homes.json`, one of each per run. The sessions inherit the caller's environment, so `CLAUDE_CONFIG_DIR` and the `XDG_*` variables reach the Neovim and the `claude` CLI that they run. `repro_issue_70.sh` launches the real `claude` CLI, and it also runs `claude --version` once outside agent-tty. Its cleanup runs `pkill -f "repro_issue_70_probe.lua $REPO_ROOT"`, which also kills the probe of another run in the same checkout [repro_issue_70.sh:WORK, cleanup] [repro_issue_277.sh:WORK, cleanup]. - `repro_issue_70_probe.lua` starts the real server with `auto_start = true`, so the plugin writes a real `PORT.lock` into the user's lock directory. Other Claude CLIs can see that lock file until `claudecode.stop()` removes it at the end. When the removal fails, `stop` only logs a warning, so a lock file that the removal failed to delete stays [repro_issue_70_probe.lua:claudecode.setup, claudecode.stop] [lua/claudecode/init.lua:M.stop]. diff --git a/scripts/run_integration_tests_individually.sh b/scripts/run_integration_tests_individually.sh index 0153a268..f2c3bfe8 100755 --- a/scripts/run_integration_tests_individually.sh +++ b/scripts/run_integration_tests_individually.sh @@ -18,8 +18,8 @@ FILE_TIMEOUT=40s # uses KILL for both, because with `timeout` dead its -k grace never starts, and a Neovim that ignores TERM would run # on. # The cleanup renames the folder before it reads the process id file, so the old path is gone at once. It removes both -# names, so a second signal that stops the first cleanup leaves nothing to the second. The shell that starts the Neovim -# writes its process id to the file at the old path, then looks for the folder at the old path (the `bash -c` line in +# names, because the rename can fail, and then the folder keeps the old name. The shell that starts the Neovim writes +# its process id to the file at the old path, then looks for the folder at the old path (the `bash -c` line in # run_test_file). So one of them sees the other: a shell that wrote its id too late for the cleanup to read it finds no # folder, and starts no Neovim. CURRENT_WORK="" @@ -43,9 +43,12 @@ cleanup_current_file() { # signal that comes while mkdir makes the folder finds the folder. The handler cleans up, then kills the runner with the # signal, not with `exit`: a calling shell stops its own loop after a child that died by SIGINT, and goes on after one # that exited 130. A cleanup that fails, such as an rm that cannot remove a folder, must not end the handler early -# either: with set -e the runner would exit with 1 before the kill, and the calling shell would go on. +# either: with set -e the runner would exit with 1 before the kill, and the calling shell would go on. The handler +# ignores INT, TERM and HUP from its first line on, and so does its rm: a second Ctrl-C reaches the rm too and stops it +# half way, and bash 3.2 (the macOS /bin/bash) then dies with 130 and leaves the renamed folder. # shellcheck disable=SC2329 # The traps below run it. die_by_signal() { + trap '' INT TERM HUP cleanup_current_file || true trap - EXIT INT TERM HUP kill -"$1" $$ diff --git a/tests/unit/integration_runner_spec.lua b/tests/unit/integration_runner_spec.lua index 4346eb79..1bff03da 100644 --- a/tests/unit/integration_runner_spec.lua +++ b/tests/unit/integration_runner_spec.lua @@ -265,9 +265,10 @@ local HOLD_WRITE = { local rm_pipe = assert(io.popen("command -v rm 2> /dev/null")) local real_rm = assert(rm_pipe:read("*l"), "needs rm") rm_pipe:close() --- Notes in $STUB_DIR/rm.holds_status_file that it holds the status file open. When $STUB_DIR/rm.die exists, its first --- call deletes the files, sends SIGINT to its process group, which has the runner in it, and dies of it, as an rm does --- that a second Ctrl-C stops. Runs the real rm and then exits with 1 +-- Notes in $STUB_DIR/rm.holds_status_file that it holds the status file open. When $STUB_DIR/rm.second_sigint exists, +-- its first call deletes the files and sends SIGINT to its process group, which has the runner in it, as a second +-- Ctrl-C does. An rm that does not ignore SIGINT dies of it. A call that lives on notes it in $STUB_DIR/rm.survived and +-- goes on as below. Runs the real rm and then exits with 1 -- when $STUB_DIR/rm.fail exists, as rm does when a folder is not empty. Runs the real rm, except when -- $STUB_DIR/rm.slow or $STUB_DIR/rm.hold exists: then it walks the way rm -rf does, but slowly. It lists the folders, -- deletes the files, waits, and only then removes the listed folders, so a file or a folder that something makes @@ -279,12 +280,11 @@ local STUB_RM = { ' case "$(readlink "$fd" 2> /dev/null)" in */exit_statuses*) : > "$STUB_DIR/rm.holds_status_file" ;; esac', "done", 'if [ -f "$STUB_DIR/rm.fail" ]; then ' .. quote(real_rm) .. ' "$@"; exit 1; fi', - 'if [ -f "$STUB_DIR/rm.die" ] && [ ! -f "$STUB_DIR/rm.died" ]; then', - ' : > "$STUB_DIR/rm.died"', - " shift", - ' for path in "$@"; do find "$path" -type f -delete; done 2> /dev/null', + 'if [ -f "$STUB_DIR/rm.second_sigint" ] && [ ! -f "$STUB_DIR/rm.sent" ]; then', + ' : > "$STUB_DIR/rm.sent"', + ' (shift; for path in "$@"; do find "$path" -type f -delete; done) 2> /dev/null', " kill -s INT 0", - " sleep 5", + ' : > "$STUB_DIR/rm.survived"', "fi", 'if [ ! -f "$STUB_DIR/rm.slow" ] && [ ! -f "$STUB_DIR/rm.hold" ]; then exec ' .. quote(real_rm) .. ' "$@"; fi', "shift", @@ -638,7 +638,7 @@ describe("scripts/run_integration_tests_individually.sh", function() --- sends only SIGINT ---@field interrupt_at integer|nil The number of the mkdir call that sends the signal, 1 by default ---@field rm_holds boolean|nil The runner's rm stops in the middle, as with `during_rm`, until that mkdir call ends - ---@field rm_dies boolean|nil The first rm of the runner deletes the files and then dies by a second SIGINT + ---@field second_sigint boolean|nil The first rm of the runner deletes the files and then gets a second SIGINT ---@field mv_fails boolean|nil The runner's mv, which renames the folder, fails ---@param signal string The signal to send @@ -677,8 +677,8 @@ describe("scripts/run_integration_tests_individually.sh", function() if run.during_rm or run.rm_holds then write(dir .. "/rm.hold", {}) end - if run.rm_dies then - write(dir .. "/rm.die", {}) + if run.second_sigint then + write(dir .. "/rm.second_sigint", {}) end if run.mv_fails then write(dir .. "/mv.fail", {}) @@ -885,11 +885,14 @@ describe("scripts/run_integration_tests_individually.sh", function() assert.is_false(result.ended) end) - it("removes its folder when a second SIGINT stops the rm of the first cleanup", function() - local result = signal_runner("INT", { rm_dies = true }) + it("finishes its cleanup when a second SIGINT comes while its rm runs", function() + local result = signal_runner("INT", { second_sigint = true }) assert.are.equal("130", result.status, result.report) - assert.are.equal("", result.left, "the second cleanup left the folder that the first cleanup renamed") + assert.are.equal("", result.left, "the runner left the folder that its cleanup renamed") + local survived = io.open(dir .. "/rm.survived") + assert.is_not_nil(survived, "the second SIGINT stopped the rm of the cleanup") + survived:close() end) it("stops the file and removes its folder under the old name when the rename of the folder fails", function() From 33bab6fad6ae9638567940d5ddb55b432344f752 Mon Sep 17 00:00:00 2001 From: Thomas Kosiewski Date: Fri, 13 Nov 2026 04:02:22 +0000 Subject: [PATCH 10/14] test(scripts): end the runner spec's stub waits after 20 seconds MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The stubs that hold a process wait for a release file that a test or a stub creates. Against the runner of main, which never calls mkdir, nothing created the file for rm, and the spec hung for 28 minutes instead of failing. Each wait now ends after 20 seconds at most. --- _Generated with [`xum`](https://github.com/coder/xum) • Model: `anthropic:claude-sonnet-5-5` • Thinking: `xhigh`_ --- tests/unit/integration_runner_spec.lua | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) diff --git a/tests/unit/integration_runner_spec.lua b/tests/unit/integration_runner_spec.lua index 1bff03da..a35bdb4b 100644 --- a/tests/unit/integration_runner_spec.lua +++ b/tests/unit/integration_runner_spec.lua @@ -157,10 +157,11 @@ local real_tee = assert(tee_pipe:read("*l"), "needs tee") tee_pipe:close() -- Copies its input like tee, then exits with the status in $STUB_DIR/tee.exit when that file exists, after the seconds -- in $STUB_DIR/tee.delay when that file exists. When $STUB_DIR/tee.hold exists, it notes in $STUB_DIR/tee.held and waits --- for $STUB_DIR/tee.go before it starts the real tee, which opens its output file. +-- for $STUB_DIR/tee.go before it starts the real tee, which opens its output file. The wait ends after 20 seconds at +-- most, so a runner that never releases it fails its test and does not hang it. local STUB_TEE = { "#!/bin/sh", - 'if [ -f "$STUB_DIR/tee.hold" ]; then : > "$STUB_DIR/tee.held"; while [ ! -e "$STUB_DIR/tee.go" ]; do sleep 0.05; done; fi', + 'if [ -f "$STUB_DIR/tee.hold" ]; then : > "$STUB_DIR/tee.held"; for tick in $(seq 400); do [ -e "$STUB_DIR/tee.go" ] && break; sleep 0.05; done; fi', quote(real_tee) .. ' "$@"', "status=$?", -- After the real tee, so that the output file is opened as early as it is for the real tee. @@ -245,18 +246,19 @@ local STUB_MV = { -- BASH_ENV files for the shell that execs timeout. Bash reads such a file first in each non-interactive shell, and only -- that shell has CLAUDECODE_MINIMAL_INIT. The first file stops the shell before its first command, as long as -- $STUB_DIR/a.release does not exist. The second file wraps echo, which the shell calls to write its process id: the --- redirection of the call has made the file by then, and the wrapper stops the shell before the write. +-- redirection of the call has made the file by then, and the wrapper stops the shell before the write. Both stop it for +-- 20 seconds at most. local HOLD_START = { 'if [ -n "${CLAUDECODE_MINIMAL_INIT:-}" ]; then', ' : > "$STUB_DIR/a.held"', - ' while [ ! -e "$STUB_DIR/a.release" ]; do sleep 0.05; done', + ' for tick in $(seq 400); do [ -e "$STUB_DIR/a.release" ] && break; sleep 0.05; done', "fi", } local HOLD_WRITE = { 'if [ -n "${CLAUDECODE_MINIMAL_INIT:-}" ]; then', " echo() {", ' : > "$STUB_DIR/a.held"', - ' while [ ! -e "$STUB_DIR/a.release" ]; do sleep 0.05; done', + ' for tick in $(seq 400); do [ -e "$STUB_DIR/a.release" ] && break; sleep 0.05; done', ' builtin echo "$@"', " }", "fi", @@ -273,7 +275,8 @@ rm_pipe:close() -- $STUB_DIR/rm.slow or $STUB_DIR/rm.hold exists: then it walks the way rm -rf does, but slowly. It lists the folders, -- deletes the files, waits, and only then removes the listed folders, so a file or a folder that something makes -- meanwhile stops the removal of its parent ("Directory not empty"), as it does for a slow rm -rf under strace. It waits --- the seconds in rm.slow, or with rm.hold it notes in $STUB_DIR/rm.held and waits for $STUB_DIR/rm.release. +-- the seconds in rm.slow, or with rm.hold it notes in $STUB_DIR/rm.held and waits for $STUB_DIR/rm.release, 20 seconds +-- at most. local STUB_RM = { "#!/bin/sh", "for fd in /proc/$$/fd/*; do", @@ -292,7 +295,7 @@ local STUB_RM = { 'for path in "$@"; do find "$path" -depth -type d >> "$STUB_DIR/rm.dirs"; find "$path" -type f -delete; done 2> /dev/null', 'if [ -f "$STUB_DIR/rm.hold" ]; then', ' : > "$STUB_DIR/rm.held"', - ' while [ ! -e "$STUB_DIR/rm.release" ]; do sleep 0.05; done', + ' for tick in $(seq 400); do [ -e "$STUB_DIR/rm.release" ] && break; sleep 0.05; done', "else", ' sleep "$(cat "$STUB_DIR/rm.slow")"', "fi", From a6fb25bad3d20d5fe909ea05f680efe4be9bbf57 Mon Sep 17 00:00:00 2001 From: Thomas Kosiewski Date: Fri, 13 Nov 2026 06:59:11 +0000 Subject: [PATCH 11/14] fix(scripts): move the runner's folder into a fresh grave and keep sockets short, and test the cleanup order MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Verifier findings F1, F2, F4 and F5 on 3ee7698a. F2: the cleanup renamed the folder to .gone. When a folder of that name existed, mv moved the work folder into it and rm then deleted the content of the user. The cleanup now makes a fresh folder next to the work folder with mktemp -d and moves the work folder into it. When either step fails, the folder keeps its old name and the cleanup works there. F4: a Neovim binds its socket in /tmp. With a TMPDIR of about 92 to 105 characters the path passed 107 bytes, libuv cut it, and a socket file named tmp.XXXXXXXX landed in the TMPDIR of the developer. A work folder with a name over 80 characters now goes to /tmp. F1: a mv stub waits while the shell of the file starts its Neovim. The test fails on a cleanup that reads the process id and kills before it renames, or that reads, renames and then kills. F5: the second signal of a cleanup is now INT, TERM or HUP after each of the three, nine tests in all. Two more tests check that a runner that gets SIGTERM or SIGHUP dies of it: the calling shell prints the report of a killed child, and it prints none for an exit with 143 or 129. --- _Generated with [`xum`](https://github.com/coder/xum) • Model: `anthropic:claude-sonnet-5-5` • Thinking: `xhigh`_ --- scripts/run_integration_tests_individually.sh | 33 ++-- tests/unit/integration_runner_spec.lua | 145 +++++++++++++++--- 2 files changed, 143 insertions(+), 35 deletions(-) diff --git a/scripts/run_integration_tests_individually.sh b/scripts/run_integration_tests_individually.sh index f2c3bfe8..5ee38c33 100755 --- a/scripts/run_integration_tests_individually.sh +++ b/scripts/run_integration_tests_individually.sh @@ -17,26 +17,29 @@ FILE_TIMEOUT=40s # state folder again. It kills `timeout` itself first: until timeout has made its own group, no Neovim runs yet. It # uses KILL for both, because with `timeout` dead its -k grace never starts, and a Neovim that ignores TERM would run # on. -# The cleanup renames the folder before it reads the process id file, so the old path is gone at once. It removes both -# names, because the rename can fail, and then the folder keeps the old name. The shell that starts the Neovim writes -# its process id to the file at the old path, then looks for the folder at the old path (the `bash -c` line in -# run_test_file). So one of them sees the other: a shell that wrote its id too late for the cleanup to read it finds no -# folder, and starts no Neovim. +# The cleanup moves the folder into a new folder next to it, the grave, before it reads the process id file, so the old +# path is gone at once. The grave comes from `mktemp -d`, so a name that exists already, as a folder of the user, is not +# the target of the move. When the grave or the move fails, the folder keeps its old name, and the cleanup works there. +# The shell that starts the Neovim writes its process id to the file at the old path, then looks for the folder at the +# old path (the `bash -c` line in run_test_file). So one of them sees the other: a shell that wrote its id too late for +# the cleanup to read it finds no folder, and starts no Neovim. CURRENT_WORK="" CURRENT_FILE_RUNS="" # shellcheck disable=SC2329 # The traps below run it. cleanup_current_file() { - local group timeout_pid + local grave moved timeout_pid [[ -n $CURRENT_WORK ]] || return 0 - mv "$CURRENT_WORK" "$CURRENT_WORK.gone" 2>/dev/null || true - group=$CURRENT_WORK.gone/group - [[ -s $group ]] || group=$CURRENT_WORK/group - if [[ -n $CURRENT_FILE_RUNS && -s $group ]]; then - timeout_pid=$(cat "$group") + moved=$CURRENT_WORK + grave=$(mktemp -d "$CURRENT_WORK.XXXXXX" 2>/dev/null) || grave="" + if [[ -n $grave ]] && mv "$CURRENT_WORK" "$grave/work" 2>/dev/null; then + moved=$grave/work + fi + if [[ -n $CURRENT_FILE_RUNS && -s $moved/group ]]; then + timeout_pid=$(cat "$moved/group") kill -KILL "$timeout_pid" 2>/dev/null || true kill -KILL -- "-$timeout_pid" 2>/dev/null || true fi - rm -rf "$CURRENT_WORK.gone" "$CURRENT_WORK" + rm -rf "$moved" ${grave:+"$grave"} } # A signal reaches the runner's own process group, and timeout leaves that group, so the file runs in the background and # `wait` below returns at once when a trap runs. Bash runs a trap after the foreground command that it waits for, so a @@ -90,6 +93,12 @@ run_test_file() { # comes before it drops the name can make it remove a folder that it did not make. local work_dir temp_output group_file state_dir config_dir tmp_dir status_file work_dir=$(mktemp -u) + # A Neovim binds its socket at $work_dir/tmp/nvim.., which is at most 20 characters more than the work folder. + # A socket path over 107 bytes is cut by libuv, and the cut name can be a file in the developer's TMPDIR, outside the + # work folder. So a work folder with a name over 80 characters goes to /tmp, where its name is short. + if [[ ${#work_dir} -gt 80 && -d /tmp && -w /tmp ]]; then + work_dir=$(mktemp -u /tmp/tmp.XXXXXXXXXX) + fi CURRENT_WORK=$work_dir mkdir -m 700 "$work_dir" || { CURRENT_WORK="" diff --git a/tests/unit/integration_runner_spec.lua b/tests/unit/integration_runner_spec.lua index a35bdb4b..308f8863 100644 --- a/tests/unit/integration_runner_spec.lua +++ b/tests/unit/integration_runner_spec.lua @@ -127,6 +127,12 @@ local has_proc_fd = (function() return status == 0 or status == true end)() +-- The runner moves the temp folder of a file to /tmp when TMPDIR is long, and it needs a /tmp that it can write to. +local tmp_is_writable = (function() + local status = os.execute("test -d /tmp && test -w /tmp") + return status == 0 or status == true +end)() + -- A process that nohup or a similar launcher started has SIGHUP ignored, and every process below it ignores it too. Bash -- cannot trap a signal that was ignored when it started, so a runner there ignores that signal. ---@param signal string @@ -177,11 +183,11 @@ local mktemp_pipe = assert(io.popen("command -v mktemp 2> /dev/null")) local real_mktemp = assert(mktemp_pipe:read("*l"), "needs mktemp") mktemp_pipe:close() -- Runs the real mktemp. When $STUB_DIR/mktemp.interrupt exists, its first call sends SIGINT to its process group, which --- has the runner in it, after the real mktemp ran and before it prints the name. When $STUB_DIR/mktemp.name exists, --- it makes nothing and prints the name in that file. +-- has the runner in it, after the real mktemp ran and before it prints the name. When $STUB_DIR/mktemp.name exists, a +-- call with -u makes nothing and prints the name in that file. local STUB_MKTEMP = { "#!/bin/sh", - 'if [ -f "$STUB_DIR/mktemp.name" ]; then cat "$STUB_DIR/mktemp.name"; exit 0; fi', + 'if [ -f "$STUB_DIR/mktemp.name" ] && [ "$1" = -u ]; then cat "$STUB_DIR/mktemp.name"; exit 0; fi', "path=$(" .. quote(real_mktemp) .. ' "$@") || exit $?', 'if [ -f "$STUB_DIR/mktemp.interrupt" ] && [ ! -f "$STUB_DIR/mktemp.interrupted" ]; then', ' : > "$STUB_DIR/mktemp.interrupted"', @@ -236,10 +242,12 @@ local STUB_MKDIR = { local mv_pipe = assert(io.popen("command -v mv 2> /dev/null")) local real_mv = assert(mv_pipe:read("*l"), "needs mv") mv_pipe:close() --- Runs the real mv, except when $STUB_DIR/mv.fail exists: then it fails and moves nothing. +-- Runs the real mv, except when $STUB_DIR/mv.fail exists: then it fails and moves nothing. When $STUB_DIR/mv.hold +-- exists, it notes in $STUB_DIR/mv.held and waits for $STUB_DIR/mv.release, 20 seconds at most, before the real mv. local STUB_MV = { "#!/bin/sh", 'if [ -f "$STUB_DIR/mv.fail" ]; then exit 1; fi', + 'if [ -f "$STUB_DIR/mv.hold" ]; then : > "$STUB_DIR/mv.held"; for tick in $(seq 400); do [ -e "$STUB_DIR/mv.release" ] && break; sleep 0.05; done; fi', "exec " .. quote(real_mv) .. ' "$@"', } @@ -267,10 +275,10 @@ local HOLD_WRITE = { local rm_pipe = assert(io.popen("command -v rm 2> /dev/null")) local real_rm = assert(rm_pipe:read("*l"), "needs rm") rm_pipe:close() --- Notes in $STUB_DIR/rm.holds_status_file that it holds the status file open. When $STUB_DIR/rm.second_sigint exists, --- its first call deletes the files and sends SIGINT to its process group, which has the runner in it, as a second --- Ctrl-C does. An rm that does not ignore SIGINT dies of it. A call that lives on notes it in $STUB_DIR/rm.survived and --- goes on as below. Runs the real rm and then exits with 1 +-- Notes in $STUB_DIR/rm.holds_status_file that it holds the status file open. When $STUB_DIR/rm.second_signal exists, +-- its first call deletes the files and sends the signal that the file names to its process group, which has the +-- runner in it, as a second Ctrl-C does. An rm that does not ignore the signal dies of it. A call that lives on notes +-- it in $STUB_DIR/rm.survived and goes on as below. Runs the real rm and then exits with 1 -- when $STUB_DIR/rm.fail exists, as rm does when a folder is not empty. Runs the real rm, except when -- $STUB_DIR/rm.slow or $STUB_DIR/rm.hold exists: then it walks the way rm -rf does, but slowly. It lists the folders, -- deletes the files, waits, and only then removes the listed folders, so a file or a folder that something makes @@ -283,10 +291,10 @@ local STUB_RM = { ' case "$(readlink "$fd" 2> /dev/null)" in */exit_statuses*) : > "$STUB_DIR/rm.holds_status_file" ;; esac', "done", 'if [ -f "$STUB_DIR/rm.fail" ]; then ' .. quote(real_rm) .. ' "$@"; exit 1; fi', - 'if [ -f "$STUB_DIR/rm.second_sigint" ] && [ ! -f "$STUB_DIR/rm.sent" ]; then', + 'if [ -f "$STUB_DIR/rm.second_signal" ] && [ ! -f "$STUB_DIR/rm.sent" ]; then', ' : > "$STUB_DIR/rm.sent"', ' (shift; for path in "$@"; do find "$path" -type f -delete; done) 2> /dev/null', - " kill -s INT 0", + ' kill -s "$(cat "$STUB_DIR/rm.second_signal")" 0', ' : > "$STUB_DIR/rm.survived"', "fi", 'if [ ! -f "$STUB_DIR/rm.slow" ] && [ ! -f "$STUB_DIR/rm.hold" ]; then exec ' .. quote(real_rm) .. ' "$@"; fi', @@ -596,6 +604,26 @@ describe("scripts/run_integration_tests_individually.sh", function() end end) + it("puts the temp folder of the file in /tmp when TMPDIR is long, so a socket path stays short", function() + if not tmp_is_writable then + pending("needs a /tmp that the test can write to") + end + local long = dir .. "/" .. string.rep("t", math.max(1, 100 - #dir - 1)) + shell("mkdir -p " .. quote(long)) + local status, output = run_script({ + { name = "a_spec.lua", exit = 0, output = summary(1, 0, 0) }, + }, nil, long) + + assert.are.equal("0", status, output) + local seen = env_seen() + assert.are.equal(1, #seen) + -- The longest socket name that a Neovim makes in XDG_RUNTIME_DIR is "/nvim.<7 digits>.<2 digits>". + assert.is_true(#seen[1].runtime_dir + 16 <= 107, "a socket path in " .. seen[1].runtime_dir .. " passes 107 bytes") + assert.are.equal(seen[1].tmpdir, seen[1].runtime_dir) + shell("test ! -e " .. quote(seen[1].tmpdir)) + shell('test -z "$(ls -A ' .. quote(long) .. ')"') + end) + it("starts its own Neovim without a ShaDa file, because the Neovim of plenary shares its folders", function() local status, output = run_script({ { name = "a_spec.lua", exit = 0, output = summary(1, 0, 0) }, @@ -641,15 +669,21 @@ describe("scripts/run_integration_tests_individually.sh", function() --- sends only SIGINT ---@field interrupt_at integer|nil The number of the mkdir call that sends the signal, 1 by default ---@field rm_holds boolean|nil The runner's rm stops in the middle, as with `during_rm`, until that mkdir call ends - ---@field second_sigint boolean|nil The first rm of the runner deletes the files and then gets a second SIGINT + ---@field second_signal "INT"|"TERM"|"HUP"|nil The first rm of the runner deletes the files, and then a second + --- signal of that name reaches its process group + ---@field during_mv boolean|nil The runner's mv, which renames the folder, waits until the shell that execs timeout has + --- started the file's Neovim + ---@field caller_reports boolean|nil Run the runner once in a calling shell, which prints the status of the runner and + --- the report that bash prints for a child that died of a signal. Send the signal to the runner alone, with `interrupt` ---@field mv_fails boolean|nil The runner's mv, which renames the folder, fails ---@param signal string The signal to send ---@param run SignalRun ---@return table result `status` of the runner (of the calling shell with `caller_loop`), `left` in its TMPDIR once --- its Neovim's grandchild would have written, `ended` when the file's Neovim got to the end of its 2 second run, - --- `runner_output` (stdout and stderr of the runner), `caller_output` of the calling shell, and `report`, one line - --- with all of these but `ended` for the message of an assertion + --- `runner_output` (stdout and stderr of the runner), `caller_output` of the calling shell, `caller_status` (the status + --- of the runner that the calling shell printed with `caller_reports`), and `report`, one line with all of these but + --- `ended` and `caller_status` for the message of an assertion local function signal_runner(signal, run) write(dir .. "/tests/integration/slow_spec.lua", {}) write(dir .. "/slow_spec.lua.out", summary(1, 0, 0)) @@ -670,7 +704,7 @@ describe("scripts/run_integration_tests_individually.sh", function() if run.rm_fails then write(dir .. "/rm.fail", {}) end - local hold = run.hold or run.during_rm == "launcher" and "start" or nil + local hold = run.hold or (run.during_rm == "launcher" or run.during_mv) and "start" or nil if hold then write(dir .. "/hold.sh", hold == "write" and HOLD_WRITE or HOLD_START) end @@ -680,12 +714,15 @@ describe("scripts/run_integration_tests_individually.sh", function() if run.during_rm or run.rm_holds then write(dir .. "/rm.hold", {}) end - if run.second_sigint then - write(dir .. "/rm.second_sigint", {}) + if run.second_signal then + write(dir .. "/rm.second_signal", { run.second_signal }) end if run.mv_fails then write(dir .. "/mv.fail", {}) end + if run.during_mv then + write(dir .. "/mv.hold", {}) + end if run.interrupt then write(dir .. "/" .. run.interrupt .. ".interrupt", { signal }) end @@ -701,6 +738,9 @@ describe("scripts/run_integration_tests_individually.sh", function() job = "bash -c " .. quote('for i in 1 2; do "$0" > ' .. quote(dir .. "/runner.out") .. " 2>&1; echo ran=$i; done") job = job .. " " .. quote(script) .. " > " .. quote(dir .. "/caller.out") .. " 2>&1" target = "-- -$pid" + elseif run.caller_reports then + job = "bash -c " .. quote('"$0" > ' .. quote(dir .. "/runner.out") .. " 2>&1; echo runner_status=$?") + job = job .. " " .. quote(script) .. " > " .. quote(dir .. "/caller.out") .. " 2>&1" end local gate = run.window and "/slow_spec.lua.timeout_started" or hold and "/a.held" @@ -711,6 +751,15 @@ describe("scripts/run_integration_tests_individually.sh", function() send = "for _ in $(seq 100); do [ -e " .. quote(dir .. gate) .. " ] && break; sleep 0.1; done; " send = send .. "kill -" .. signal .. " " .. target .. "; " end + if run.during_mv then + send = send .. "for _ in $(seq 100); do [ -e " .. quote(dir .. "/mv.held") .. " ] && break; sleep 0.1; done; " + send = send .. ": > " .. quote(dir .. "/a.release") .. "; " + send = send + .. "for _ in $(seq 100); do [ -e " + .. quote(dir .. "/slow_spec.lua.started") + .. " ] && break; sleep 0.1; done; " + send = send .. ": > " .. quote(dir .. "/mv.release") .. "; " + end if run.during_rm then local release = run.during_rm == "tee" and "/tee.go" or "/a.release" send = send .. "for _ in $(seq 100); do [ -e " .. quote(dir .. "/rm.held") .. " ] && break; sleep 0.1; done; " @@ -733,7 +782,7 @@ describe("scripts/run_integration_tests_individually.sh", function() .. " & pid=$!; " .. send .. "wait $pid 2> /dev/null; echo status=$?; " - .. (hold and not run.during_rm and ": > " .. quote(dir .. "/a.release") .. "; " or "") + .. (hold and not run.during_rm and not run.during_mv and ": > " .. quote(dir .. "/a.release") .. "; " or "") .. "sleep 3" local pipe = assert(io.popen("bash -c " .. quote(inner))) local output = pipe:read("*a") @@ -761,6 +810,7 @@ describe("scripts/run_integration_tests_individually.sh", function() left = left, ended = ended ~= nil, caller_output = caller_output, + caller_status = caller_output:match("runner_status=(%d+)"), runner_output = runner_output, report = string.format( "status %s, left %q, caller output %q, runner output %q", @@ -888,14 +938,51 @@ describe("scripts/run_integration_tests_individually.sh", function() assert.is_false(result.ended) end) - it("finishes its cleanup when a second SIGINT comes while its rm runs", function() - local result = signal_runner("INT", { second_sigint = true }) + -- A second signal reaches the rm of the cleanup as well. The runner ignores the signals from the first line of its + -- handler on, and its rm ignores them too, so the cleanup ends and the result is that of the first signal. + for _, first in ipairs({ { "INT", "130" }, { "TERM", "143" }, { "HUP", "129" } }) do + for _, second in ipairs({ "INT", "TERM", "HUP" }) do + local signal, expected_status = first[1], first[2] + it("finishes its cleanup and exits " .. expected_status .. " when " .. second .. " follows " .. signal, function() + if signal_is_ignored(signal) then + pending(signal .. " is ignored in this environment, so the runner ignores it too") + end + local result = signal_runner(signal, { second_signal = second }) + + assert.are.equal(expected_status, result.status, result.report) + assert.are.equal("", result.left, "the runner left the folder that its cleanup moved") + local survived = io.open(dir .. "/rm.survived") + assert.is_not_nil(survived, "the second " .. second .. " stopped the rm of the cleanup") + survived:close() + end) + end + end + + -- bash stops its own loop only for a child that died of SIGINT, so for SIGTERM and SIGHUP the sign of a runner that + -- died of the signal, and did not exit with 143 or 129, is the report that bash prints for the child. + for _, case in ipairs({ { "TERM", "143", "Terminated" }, { "HUP", "129", "Hangup" } }) do + local signal, expected_status, report = case[1], case[2], case[3] + it("dies of " .. signal .. " that it gets, so that the calling shell sees a child that was killed", function() + if signal_is_ignored(signal) then + pending(signal .. " is ignored in this environment, so the runner ignores it too") + end + local result = signal_runner(signal, { interrupt = "mkdir", caller_reports = true }) + + assert.are.equal(expected_status, result.caller_status, result.report) + assert.is_truthy( + result.caller_output:find(report, 1, true), + "the runner exited with " .. expected_status .. " and did not die of " .. signal .. ": " .. result.report + ) + assert.are.equal("", result.left, "the runner left temp files or folders behind") + end) + end + + it("stops the Neovim that the shell of the file started while the cleanup renamed its folder", function() + local result = signal_runner("INT", { during_mv = true }) assert.are.equal("130", result.status, result.report) - assert.are.equal("", result.left, "the runner left the folder that its cleanup renamed") - local survived = io.open(dir .. "/rm.survived") - assert.is_not_nil(survived, "the second SIGINT stopped the rm of the cleanup") - survived:close() + assert.are.equal("", result.left, "the file's Neovim ran on and wrote into the folder after the runner died") + assert.is_false(result.ended, "the file's Neovim ran on after the runner died") end) it("stops the file and removes its folder under the old name when the rename of the folder fails", function() @@ -906,6 +993,18 @@ describe("scripts/run_integration_tests_individually.sh", function() assert.is_false(result.ended) end) + it("leaves a folder alone that is named like its temp folder with .gone added", function() + local work = dir .. "/tmp/fixed" + shell("mkdir -p " .. quote(work .. ".gone") .. " && : > " .. quote(work .. ".gone/keep")) + write(dir .. "/mktemp.name", { work }) + local result = signal_runner("INT", {}) + + assert.are.equal("130", result.status, result.report) + assert.are.equal("fixed.gone\n", result.left, "the runner left its folder, or removed the one that it did not make") + assert.is_not_nil(io.open(work .. ".gone/keep"), "the cleanup removed a folder that it did not make") + assert.is_false(result.ended) + end) + for _, case in ipairs({ { "mktemp", 1 }, { "mkdir", 1 }, { "mkdir", 2 } }) do local command, at = case[1], case[2] it( From a395873203622b58b633c9fb12cb7b64e819b53e Mon Sep 17 00:00:00 2001 From: Thomas Kosiewski Date: Fri, 13 Nov 2026 07:21:02 +0000 Subject: [PATCH 12/14] docs(scripts): describe the grave folder and the /tmp work folder of the runner MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The cleanup moves the work folder into a new folder next to it. A work folder with a name over 80 characters goes to /tmp. The README states both, and that a failed move leaves the race that main has. --- _Generated with [`xum`](https://github.com/coder/xum) • Model: `anthropic:claude-sonnet-5-5` • Thinking: `xhigh`_ --- scripts/README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scripts/README.md b/scripts/README.md index 519cdcf4..261882b6 100644 --- a/scripts/README.md +++ b/scripts/README.md @@ -84,7 +84,7 @@ The Lua repros run from the repository root with `nvim --headless -u NONE -l scr - CAUTION: Do not run the token-sending clients (`repro_issue_238.sh`, `repro_issue_248.sh`, and `repro_issue_277.sh`) on a shared machine. They pass the auth token on the `websocat` command line (`--header`), where other users can read it with `ps` [repro_issue_238.sh:WEBSOCAT] [repro_issue_248.sh:TOKEN] [repro_issue_277.sh:TOKEN]. - `repro_issue_238.sh`, `repro_issue_248.sh`, and `repro_issue_277.sh` connect to the plugin server on `127.0.0.1` [repro_issue_238.sh:PORT] [repro_issue_248.sh:PORT] [repro_issue_277.sh:PORT]. -- `run_integration_tests_individually.sh` creates one temp folder per test, and puts the output, the file that holds the process id of the file's `timeout`, and the `XDG_STATE_HOME`, `XDG_CONFIG_HOME` and `TMPDIR` that the file's two Neovims share in it, and deletes the folder after the file ran. The runner stores the name of the folder (from `mktemp -u`) before it makes the folder, so a signal never finds a folder that the runner does not know, and `mkdir` fails when the name exists, and the runner then drops the name. Only a signal that comes before it drops the name can make it remove a folder that it did not make. The runner opens a status file in the state folder before it starts the file, and the file's subshell writes the exit statuses of nvim and of `tee` to that open file when the file ends. A subshell that ends while the cleanup removes the folder opens no path, so it leaves nothing behind. When the runner cannot read the file back, for example because the Neovim deleted the state folder, it uses the exit status of the subshell, which is the status of nvim, or of `tee` when nvim exited 0, and names it as the status of nvim. When it gets SIGTERM, SIGHUP or SIGINT (Ctrl-C) while a file runs, it renames the folder, kills that `timeout`, then the process group of the file, deletes the renamed folder and dies by that signal as soon as the command that it waits for ends, even when a delete fails, so a calling shell sees status 143, 129 or 130, and a shell loop around the runner stops after Ctrl-C. It ignores INT, TERM and HUP while it cleans up, so a second Ctrl-C does not stop the delete half way (with bash 3.2 it would leave the renamed folder). Killing the `timeout` first covers the moments before it has made its own group, when no Neovim runs yet. Both kills use KILL, so a Neovim that ignores SIGTERM stops too. The shell of the file writes its process id to the file in the folder, then looks for the folder, and it starts no Neovim when it finds none. The runner renames the folder before it reads the process id, so one of the two sees the other. SIGKILL gives the runner no time to clean up, so the folder stays. A runner that leads its own session on a terminal, for example one that a terminal emulator starts directly, hangs up the foreground process group of the terminal when it dies, so the shell of the file dies too and starts no Neovim. Without a terminal that shell goes on, also when it has not written its process id yet, and its Neovim runs until its own timeout. Bash ignores SIGQUIT, so the file runs to its end. It sets `VIMINIT`, `CLAUDECODE_MINIMAL_INIT`, `XDG_STATE_HOME`, `XDG_CONFIG_HOME`, `TMPDIR` and `XDG_RUNTIME_DIR` for that file's Neovims and the processes they start only, as a prefix of one command, and leaves `XDG_DATA_HOME` alone because plenary lives there. A run writes nothing under `~/.local/state/nvim`, leaves no `nvim.` socket folder in the developer's `TMPDIR`, even when it kills a Neovim, and the developer's `ftplugin` and `after` files under `~/.config/nvim` do not run [run_integration_tests_individually.sh:work_dir, temp_output, group_file, state_dir, config_dir, tmp_dir]. +- `run_integration_tests_individually.sh` creates one temp folder per test, and puts the output, the file that holds the process id of the file's `timeout`, and the `XDG_STATE_HOME`, `XDG_CONFIG_HOME` and `TMPDIR` that the file's two Neovims share in it, and deletes the folder after the file ran. The runner stores the name of the folder (from `mktemp -u`) before it makes the folder, so a signal never finds a folder that the runner does not know, and `mkdir` fails when the name exists, and the runner then drops the name. Only a signal that comes before it drops the name can make it remove a folder that it did not make. The runner opens a status file in the state folder before it starts the file, and the file's subshell writes the exit statuses of nvim and of `tee` to that open file when the file ends. A subshell that ends while the cleanup removes the folder opens no path, so it leaves nothing behind. When the runner cannot read the file back, for example because the Neovim deleted the state folder, it uses the exit status of the subshell, which is the status of nvim, or of `tee` when nvim exited 0, and names it as the status of nvim. When it gets SIGTERM, SIGHUP or SIGINT (Ctrl-C) while a file runs, it moves the folder into a new folder next to it (made with `mktemp -d`, so no name that exists is touched), kills that `timeout`, then the process group of the file, deletes the new folder and dies by that signal as soon as the command that it waits for ends, even when a delete fails, so a calling shell sees status 143, 129 or 130, and a shell loop around the runner stops after Ctrl-C. It ignores INT, TERM and HUP while it cleans up, so a second Ctrl-C does not stop the delete half way (with bash 3.2 it would leave the renamed folder). Killing the `timeout` first covers the moments before it has made its own group, when no Neovim runs yet. Both kills use KILL, so a Neovim that ignores SIGTERM stops too. The shell of the file writes its process id to the file in the folder, then looks for the folder, and it starts no Neovim when it finds none. The runner moves the folder before it reads the process id, so one of the two sees the other. When the move fails, the folder keeps its name, and a shell that acts while the delete runs can still start a Neovim, as on main. SIGKILL gives the runner no time to clean up, so the folder stays. A runner that leads its own session on a terminal, for example one that a terminal emulator starts directly, hangs up the foreground process group of the terminal when it dies, so the shell of the file dies too and starts no Neovim. Without a terminal that shell goes on, also when it has not written its process id yet, and its Neovim runs until its own timeout. Bash ignores SIGQUIT, so the file runs to its end. It sets `VIMINIT`, `CLAUDECODE_MINIMAL_INIT`, `XDG_STATE_HOME`, `XDG_CONFIG_HOME`, `TMPDIR` and `XDG_RUNTIME_DIR` for that file's Neovims and the processes they start only, as a prefix of one command, and leaves `XDG_DATA_HOME` alone because plenary lives there. A run writes nothing under `~/.local/state/nvim`, leaves no `nvim.` socket folder in the developer's `TMPDIR`, even when it kills a Neovim, and puts a work folder with a name over 80 characters in `/tmp`, so that a Neovim socket path stays under the 107 bytes that libuv allows (without a writable `/tmp` the folder stays in `TMPDIR`, and a longer path is cut and can name a file there), and the developer's `ftplugin` and `after` files under `~/.config/nvim` do not run [run_integration_tests_individually.sh:work_dir, temp_output, group_file, state_dir, config_dir, tmp_dir]. - `repro_issue_231.lua` deletes `/__issue_231_repro__.md` [repro_issue_231.lua:try_open_diff]. - `repro_issue_70.sh` and `repro_issue_277.sh` make a work dir with `mktemp -d` (`${TMPDIR:-/tmp}/issue70.XXXXXX` or `issue277.XXXXXX`) and an `att-home` folder in it before they check their tools. They install `trap cleanup EXIT` after that check, so an exit 3 for a missing tool leaves the work dir behind. The trap also runs on SIGTERM, SIGINT and SIGHUP. It destroys the `agent-tty` sessions and removes the work dir. agent-tty still leaves an empty socket folder `/tmp/agent-tty/<8 hex digits>` (it ignores `TMPDIR`) and an entry for the deleted home in `${XDG_STATE_HOME:-$HOME/.local/state}/agent-tty/homes.json`, one of each per run. The sessions inherit the caller's environment, so `CLAUDE_CONFIG_DIR` and the `XDG_*` variables reach the Neovim and the `claude` CLI that they run. `repro_issue_70.sh` launches the real `claude` CLI, and it also runs `claude --version` once outside agent-tty. Its cleanup runs `pkill -f "repro_issue_70_probe.lua $REPO_ROOT"`, which also kills the probe of another run in the same checkout [repro_issue_70.sh:WORK, cleanup] [repro_issue_277.sh:WORK, cleanup]. - `repro_issue_70_probe.lua` starts the real server with `auto_start = true`, so the plugin writes a real `PORT.lock` into the user's lock directory. Other Claude CLIs can see that lock file until `claudecode.stop()` removes it at the end. When the removal fails, `stop` only logs a warning, so a lock file that the removal failed to delete stays [repro_issue_70_probe.lua:claudecode.setup, claudecode.stop] [lua/claudecode/init.lua:M.stop]. From fb426bb4fa806bf683c0dc05c7a7c5e8808568d1 Mon Sep 17 00:00:00 2001 From: Thomas Kosiewski Date: Fri, 13 Nov 2026 08:53:32 +0000 Subject: [PATCH 13/14] fix(scripts): remove the old path of the work folder after the kill MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The move into a fresh grave left the old path unremoved. A Neovim that still runs between the move and the kill makes its state folder there again (state/nvim/swap, shada), and nothing removed it. A real Ctrl-C in the whole runner with a real Neovim left such a folder in TMPDIR in 7 of 15 runs. The last rm now removes the old path too: the Neovim is dead by then. A cat stub holds the cleanup after the move and before the kill, and the stub Neovim writes into the old path then. The test fails without the fix. --- _Generated with [`xum`](https://github.com/coder/xum) • Model: `anthropic:claude-sonnet-5-5` • Thinking: `xhigh`_ --- scripts/README.md | 2 +- scripts/run_integration_tests_individually.sh | 5 +- tests/unit/integration_runner_spec.lua | 49 ++++++++++++++++++- 3 files changed, 52 insertions(+), 4 deletions(-) diff --git a/scripts/README.md b/scripts/README.md index 261882b6..2c52cbf9 100644 --- a/scripts/README.md +++ b/scripts/README.md @@ -84,7 +84,7 @@ The Lua repros run from the repository root with `nvim --headless -u NONE -l scr - CAUTION: Do not run the token-sending clients (`repro_issue_238.sh`, `repro_issue_248.sh`, and `repro_issue_277.sh`) on a shared machine. They pass the auth token on the `websocat` command line (`--header`), where other users can read it with `ps` [repro_issue_238.sh:WEBSOCAT] [repro_issue_248.sh:TOKEN] [repro_issue_277.sh:TOKEN]. - `repro_issue_238.sh`, `repro_issue_248.sh`, and `repro_issue_277.sh` connect to the plugin server on `127.0.0.1` [repro_issue_238.sh:PORT] [repro_issue_248.sh:PORT] [repro_issue_277.sh:PORT]. -- `run_integration_tests_individually.sh` creates one temp folder per test, and puts the output, the file that holds the process id of the file's `timeout`, and the `XDG_STATE_HOME`, `XDG_CONFIG_HOME` and `TMPDIR` that the file's two Neovims share in it, and deletes the folder after the file ran. The runner stores the name of the folder (from `mktemp -u`) before it makes the folder, so a signal never finds a folder that the runner does not know, and `mkdir` fails when the name exists, and the runner then drops the name. Only a signal that comes before it drops the name can make it remove a folder that it did not make. The runner opens a status file in the state folder before it starts the file, and the file's subshell writes the exit statuses of nvim and of `tee` to that open file when the file ends. A subshell that ends while the cleanup removes the folder opens no path, so it leaves nothing behind. When the runner cannot read the file back, for example because the Neovim deleted the state folder, it uses the exit status of the subshell, which is the status of nvim, or of `tee` when nvim exited 0, and names it as the status of nvim. When it gets SIGTERM, SIGHUP or SIGINT (Ctrl-C) while a file runs, it moves the folder into a new folder next to it (made with `mktemp -d`, so no name that exists is touched), kills that `timeout`, then the process group of the file, deletes the new folder and dies by that signal as soon as the command that it waits for ends, even when a delete fails, so a calling shell sees status 143, 129 or 130, and a shell loop around the runner stops after Ctrl-C. It ignores INT, TERM and HUP while it cleans up, so a second Ctrl-C does not stop the delete half way (with bash 3.2 it would leave the renamed folder). Killing the `timeout` first covers the moments before it has made its own group, when no Neovim runs yet. Both kills use KILL, so a Neovim that ignores SIGTERM stops too. The shell of the file writes its process id to the file in the folder, then looks for the folder, and it starts no Neovim when it finds none. The runner moves the folder before it reads the process id, so one of the two sees the other. When the move fails, the folder keeps its name, and a shell that acts while the delete runs can still start a Neovim, as on main. SIGKILL gives the runner no time to clean up, so the folder stays. A runner that leads its own session on a terminal, for example one that a terminal emulator starts directly, hangs up the foreground process group of the terminal when it dies, so the shell of the file dies too and starts no Neovim. Without a terminal that shell goes on, also when it has not written its process id yet, and its Neovim runs until its own timeout. Bash ignores SIGQUIT, so the file runs to its end. It sets `VIMINIT`, `CLAUDECODE_MINIMAL_INIT`, `XDG_STATE_HOME`, `XDG_CONFIG_HOME`, `TMPDIR` and `XDG_RUNTIME_DIR` for that file's Neovims and the processes they start only, as a prefix of one command, and leaves `XDG_DATA_HOME` alone because plenary lives there. A run writes nothing under `~/.local/state/nvim`, leaves no `nvim.` socket folder in the developer's `TMPDIR`, even when it kills a Neovim, and puts a work folder with a name over 80 characters in `/tmp`, so that a Neovim socket path stays under the 107 bytes that libuv allows (without a writable `/tmp` the folder stays in `TMPDIR`, and a longer path is cut and can name a file there), and the developer's `ftplugin` and `after` files under `~/.config/nvim` do not run [run_integration_tests_individually.sh:work_dir, temp_output, group_file, state_dir, config_dir, tmp_dir]. +- `run_integration_tests_individually.sh` creates one temp folder per test, and puts the output, the file that holds the process id of the file's `timeout`, and the `XDG_STATE_HOME`, `XDG_CONFIG_HOME` and `TMPDIR` that the file's two Neovims share in it, and deletes the folder after the file ran. The runner stores the name of the folder (from `mktemp -u`) before it makes the folder, so a signal never finds a folder that the runner does not know, and `mkdir` fails when the name exists, and the runner then drops the name. Only a signal that comes before it drops the name can make it remove a folder that it did not make. The runner opens a status file in the state folder before it starts the file, and the file's subshell writes the exit statuses of nvim and of `tee` to that open file when the file ends. A subshell that ends while the cleanup removes the folder opens no path, so it leaves nothing behind. When the runner cannot read the file back, for example because the Neovim deleted the state folder, it uses the exit status of the subshell, which is the status of nvim, or of `tee` when nvim exited 0, and names it as the status of nvim. When it gets SIGTERM, SIGHUP or SIGINT (Ctrl-C) while a file runs, it moves the folder into a new folder next to it (made with `mktemp -d`, so no name that exists is touched), kills that `timeout`, then the process group of the file, deletes the new folder and the old path (a Neovim that still runs before the kill can make folders there again) and dies by that signal as soon as the command that it waits for ends, even when a delete fails, so a calling shell sees status 143, 129 or 130, and a shell loop around the runner stops after Ctrl-C. It ignores INT, TERM and HUP while it cleans up, so a second Ctrl-C does not stop the delete half way (with bash 3.2 it would leave the renamed folder). Killing the `timeout` first covers the moments before it has made its own group, when no Neovim runs yet. Both kills use KILL, so a Neovim that ignores SIGTERM stops too. The shell of the file writes its process id to the file in the folder, then looks for the folder, and it starts no Neovim when it finds none. The runner moves the folder before it reads the process id, so one of the two sees the other. When the move fails, the folder keeps its name, and a shell that acts while the delete runs can still start a Neovim, as on main. SIGKILL gives the runner no time to clean up, so the folder stays. A runner that leads its own session on a terminal, for example one that a terminal emulator starts directly, hangs up the foreground process group of the terminal when it dies, so the shell of the file dies too and starts no Neovim. Without a terminal that shell goes on, also when it has not written its process id yet, and its Neovim runs until its own timeout. Bash ignores SIGQUIT, so the file runs to its end. It sets `VIMINIT`, `CLAUDECODE_MINIMAL_INIT`, `XDG_STATE_HOME`, `XDG_CONFIG_HOME`, `TMPDIR` and `XDG_RUNTIME_DIR` for that file's Neovims and the processes they start only, as a prefix of one command, and leaves `XDG_DATA_HOME` alone because plenary lives there. A run writes nothing under `~/.local/state/nvim`, leaves no `nvim.` socket folder in the developer's `TMPDIR`, even when it kills a Neovim, and puts a work folder with a name over 80 characters in `/tmp`, so that a Neovim socket path stays under the 107 bytes that libuv allows (without a writable `/tmp` the folder stays in `TMPDIR`, and a longer path is cut and can name a file there), and the developer's `ftplugin` and `after` files under `~/.config/nvim` do not run [run_integration_tests_individually.sh:work_dir, temp_output, group_file, state_dir, config_dir, tmp_dir]. - `repro_issue_231.lua` deletes `/__issue_231_repro__.md` [repro_issue_231.lua:try_open_diff]. - `repro_issue_70.sh` and `repro_issue_277.sh` make a work dir with `mktemp -d` (`${TMPDIR:-/tmp}/issue70.XXXXXX` or `issue277.XXXXXX`) and an `att-home` folder in it before they check their tools. They install `trap cleanup EXIT` after that check, so an exit 3 for a missing tool leaves the work dir behind. The trap also runs on SIGTERM, SIGINT and SIGHUP. It destroys the `agent-tty` sessions and removes the work dir. agent-tty still leaves an empty socket folder `/tmp/agent-tty/<8 hex digits>` (it ignores `TMPDIR`) and an entry for the deleted home in `${XDG_STATE_HOME:-$HOME/.local/state}/agent-tty/homes.json`, one of each per run. The sessions inherit the caller's environment, so `CLAUDE_CONFIG_DIR` and the `XDG_*` variables reach the Neovim and the `claude` CLI that they run. `repro_issue_70.sh` launches the real `claude` CLI, and it also runs `claude --version` once outside agent-tty. Its cleanup runs `pkill -f "repro_issue_70_probe.lua $REPO_ROOT"`, which also kills the probe of another run in the same checkout [repro_issue_70.sh:WORK, cleanup] [repro_issue_277.sh:WORK, cleanup]. - `repro_issue_70_probe.lua` starts the real server with `auto_start = true`, so the plugin writes a real `PORT.lock` into the user's lock directory. Other Claude CLIs can see that lock file until `claudecode.stop()` removes it at the end. When the removal fails, `stop` only logs a warning, so a lock file that the removal failed to delete stays [repro_issue_70_probe.lua:claudecode.setup, claudecode.stop] [lua/claudecode/init.lua:M.stop]. diff --git a/scripts/run_integration_tests_individually.sh b/scripts/run_integration_tests_individually.sh index 5ee38c33..de5f8383 100755 --- a/scripts/run_integration_tests_individually.sh +++ b/scripts/run_integration_tests_individually.sh @@ -22,7 +22,8 @@ FILE_TIMEOUT=40s # the target of the move. When the grave or the move fails, the folder keeps its old name, and the cleanup works there. # The shell that starts the Neovim writes its process id to the file at the old path, then looks for the folder at the # old path (the `bash -c` line in run_test_file). So one of them sees the other: a shell that wrote its id too late for -# the cleanup to read it finds no folder, and starts no Neovim. +# the cleanup to read it finds no folder, and starts no Neovim. A Neovim that still runs between the move and the kill +# can make its state folder at the old path again, so the last rm removes the old path as well. CURRENT_WORK="" CURRENT_FILE_RUNS="" # shellcheck disable=SC2329 # The traps below run it. @@ -39,7 +40,7 @@ cleanup_current_file() { kill -KILL "$timeout_pid" 2>/dev/null || true kill -KILL -- "-$timeout_pid" 2>/dev/null || true fi - rm -rf "$moved" ${grave:+"$grave"} + rm -rf "$moved" ${grave:+"$grave"} "$CURRENT_WORK" } # A signal reaches the runner's own process group, and timeout leaves that group, so the file runs in the background and # `wait` below returns at once when a trap runs. Bash runs a trap after the foreground command that it waits for, so a diff --git a/tests/unit/integration_runner_spec.lua b/tests/unit/integration_runner_spec.lua index 308f8863..cbe03018 100644 --- a/tests/unit/integration_runner_spec.lua +++ b/tests/unit/integration_runner_spec.lua @@ -50,7 +50,12 @@ local STUB_NVIM = concat({ "#!/bin/sh" }, SPEC_NAME, { 'if [ -f "$STUB_DIR/$name.sleep" ]; then', ' if [ -f "$STUB_DIR/$name.deaf_to_term" ]; then trap "" TERM; fi', ' : > "$STUB_DIR/$name.started"', - ' (sleep 2; mkdir -p "$XDG_STATE_HOME/nvim" && : > "$XDG_STATE_HOME/nvim/shada") &', + ' if [ -f "$STUB_DIR/$name.write_on_cue" ]; then', + ' (for tick in $(seq 400); do [ -e "$STUB_DIR/$name.cue" ] && break; sleep 0.05; done;', + ' mkdir -p "$XDG_STATE_HOME/nvim" && : > "$XDG_STATE_HOME/nvim/shada"; : > "$STUB_DIR/$name.written"; sleep 20) &', + " else", + ' (sleep 2; mkdir -p "$XDG_STATE_HOME/nvim" && : > "$XDG_STATE_HOME/nvim/shada") &', + " fi", " wait", ' : > "$STUB_DIR/$name.ended"', "fi", @@ -251,6 +256,24 @@ local STUB_MV = { "exec " .. quote(real_mv) .. ' "$@"', } +local cat_pipe = assert(io.popen("command -v cat 2> /dev/null")) +local real_cat = assert(cat_pipe:read("*l"), "needs cat") +cat_pipe:close() +-- Runs the real cat, except when $STUB_DIR/cat.hold exists: then the call that reads a process id file in a folder that +-- the cleanup moved (a path that ends in /work/group) notes in $STUB_DIR/cat.held and waits for $STUB_DIR/cat.release, +-- 20 seconds at most, before the real cat. +local STUB_CAT = { + "#!/bin/sh", + 'if [ -f "$STUB_DIR/cat.hold" ]; then', + ' case "$1" in */work/group)', + ' : > "$STUB_DIR/cat.held"', + ' for tick in $(seq 400); do [ -e "$STUB_DIR/cat.release" ] && break; sleep 0.05; done', + " ;;", + " esac", + "fi", + "exec " .. quote(real_cat) .. ' "$@"', +} + -- BASH_ENV files for the shell that execs timeout. Bash reads such a file first in each non-interactive shell, and only -- that shell has CLAUDECODE_MINIMAL_INIT. The first file stops the shell before its first command, as long as -- $STUB_DIR/a.release does not exist. The second file wraps echo, which the shell calls to write its process id: the @@ -370,6 +393,7 @@ describe("scripts/run_integration_tests_individually.sh", function() write(dir .. "/bin/mktemp", STUB_MKTEMP, true) write(dir .. "/bin/mkdir", STUB_MKDIR, true) write(dir .. "/bin/mv", STUB_MV, true) + write(dir .. "/bin/cat", STUB_CAT, true) end) after_each(function() @@ -673,6 +697,8 @@ describe("scripts/run_integration_tests_individually.sh", function() --- signal of that name reaches its process group ---@field during_mv boolean|nil The runner's mv, which renames the folder, waits until the shell that execs timeout has --- started the file's Neovim + ---@field write_before_kill boolean|nil The cleanup waits after it moved the folder and before it kills, and the file's + --- Neovim writes into the old folder path then, as a live Neovim does ---@field caller_reports boolean|nil Run the runner once in a calling shell, which prints the status of the runner and --- the report that bash prints for a child that died of a signal. Send the signal to the runner alone, with `interrupt` ---@field mv_fails boolean|nil The runner's mv, which renames the folder, fails @@ -723,6 +749,10 @@ describe("scripts/run_integration_tests_individually.sh", function() if run.during_mv then write(dir .. "/mv.hold", {}) end + if run.write_before_kill then + write(dir .. "/cat.hold", {}) + write(dir .. "/slow_spec.lua.write_on_cue", {}) + end if run.interrupt then write(dir .. "/" .. run.interrupt .. ".interrupt", { signal }) end @@ -760,6 +790,15 @@ describe("scripts/run_integration_tests_individually.sh", function() .. " ] && break; sleep 0.1; done; " send = send .. ": > " .. quote(dir .. "/mv.release") .. "; " end + if run.write_before_kill then + send = send .. "for _ in $(seq 100); do [ -e " .. quote(dir .. "/cat.held") .. " ] && break; sleep 0.1; done; " + send = send .. ": > " .. quote(dir .. "/slow_spec.lua.cue") .. "; " + send = send + .. "for _ in $(seq 100); do [ -e " + .. quote(dir .. "/slow_spec.lua.written") + .. " ] && break; sleep 0.1; done; " + send = send .. ": > " .. quote(dir .. "/cat.release") .. "; " + end if run.during_rm then local release = run.during_rm == "tee" and "/tee.go" or "/a.release" send = send .. "for _ in $(seq 100); do [ -e " .. quote(dir .. "/rm.held") .. " ] && break; sleep 0.1; done; " @@ -985,6 +1024,14 @@ describe("scripts/run_integration_tests_individually.sh", function() assert.is_false(result.ended, "the file's Neovim ran on after the runner died") end) + it("removes what a live Neovim wrote into the old folder path after the cleanup moved the folder", function() + local result = signal_runner("INT", { write_before_kill = true }) + + assert.are.equal("130", result.status, result.report) + assert.are.equal("", result.left, "the file's Neovim made a folder at the old path, and the cleanup left it") + assert.is_false(result.ended) + end) + it("stops the file and removes its folder under the old name when the rename of the folder fails", function() local result = signal_runner("INT", { mv_fails = true }) From b2ced6fdcdbf38da99a1a47f8fcb882f5edf17de Mon Sep 17 00:00:00 2001 From: Thomas Kosiewski Date: Fri, 13 Nov 2026 08:54:33 +0000 Subject: [PATCH 14/14] refactor(scripts): drop the redundant rm of the moved folder in the cleanup MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The moved folder lies in the grave when the move worked, and it is the old path when the move failed. The last rm removes the grave and the old path, which covers both. --- _Generated with [`xum`](https://github.com/coder/xum) • Model: `anthropic:claude-sonnet-5-5` • Thinking: `xhigh`_ --- scripts/run_integration_tests_individually.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scripts/run_integration_tests_individually.sh b/scripts/run_integration_tests_individually.sh index de5f8383..fe530e8a 100755 --- a/scripts/run_integration_tests_individually.sh +++ b/scripts/run_integration_tests_individually.sh @@ -40,7 +40,7 @@ cleanup_current_file() { kill -KILL "$timeout_pid" 2>/dev/null || true kill -KILL -- "-$timeout_pid" 2>/dev/null || true fi - rm -rf "$moved" ${grave:+"$grave"} "$CURRENT_WORK" + rm -rf ${grave:+"$grave"} "$CURRENT_WORK" } # A signal reaches the runner's own process group, and timeout leaves that group, so the file runs in the background and # `wait` below returns at once when a trap runs. Bash runs a trap after the foreground command that it waits for, so a