fix(scripts): make the integration runner die by SIGINT and leave nothing after a signal

scripts/README.md

@@ -90,7 +90,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 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.<user>` 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 `<repo_root>/__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].

scripts/run_integration_tests_individually.sh

@@ -12,29 +12,56 @@ 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=()
-CURRENT_GROUP_FILE=""
-# shellcheck disable=SC2329 # The two traps below run it.
+# 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.
+# 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. 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.
 cleanup_current_file() {
-  local timeout_pid
-  if [[ -s $CURRENT_GROUP_FILE ]]; then
-    timeout_pid=$(cat "$CURRENT_GROUP_FILE")
+  local grave moved timeout_pid
+  [[ -n $CURRENT_WORK ]] || return 0
+  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_TEMP[@]}"
+  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
+# 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. 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" $$
+  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.
-trap 'cleanup_current_file; 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 ==="
 
@@ -61,14 +88,31 @@ 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 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 tmp_dir status_file
+  work_dir=$(mktemp -u)
+  # A Neovim binds its socket at $work_dir/tmp/nvim.<pid>.<n>, 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=""
+    false
+  }
+  temp_output=$work_dir/output
+  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
-  CURRENT_TEMP=("$temp_output" "$group_file" "$state_dir" "$config_dir")
-  CURRENT_GROUP_FILE=$group_file
+  mkdir -m 700 "$state_dir" "$config_dir" "$tmp_dir"
+  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.
@@ -79,6 +123,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
@@ -93,8 +139,9 @@ 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" \
-      bash -c 'echo $$ > "$1"; shift; exec timeout -k 5s "$@"' _ "$group_file" "$FILE_TIMEOUT" \
+      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 \
       -c "lua require('plenary.test_harness').test_file('$test_file')" \
       9>&- 2>&1 | tee "$temp_output" 9>&-
@@ -107,7 +154,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
@@ -158,7 +205,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
@@ -176,8 +223,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