fix(scripts): make the integration runner die by SIGINT and leave nothing after a signal by ThomasK33 · Pull Request #592 · coder/claudecode.nvim

ThomasK33 · 2026-11-12T23:45:15Z

Why

scripts/run_integration_tests_individually.sh runs each integration spec in its own Neovim. When a developer presses Ctrl-C, or CI sends SIGTERM or SIGHUP, the runner must stop at once, remove what it made, and leave no Neovim running. A calling shell loop must also stop. This PR fixes four defects in that path and updates the README. The first defect made a test fail on main. The others were gaps that scripts/README.md listed, or that review of earlier heads of this PR found.

What changed

1. A failing cleanup no longer swallows Ctrl-C

The INT trap ran cleanup_current_file under set -e. When its rm -rf exited non-zero, bash ended the trap there. The runner then exited with status 1 and never re-raised SIGINT. A calling shell goes on after a child that exited, so its loop ran the runner again and an interrupted run looked green.

This is the failure of main's post-merge Tests run 43299040669, where "dies by SIGINT on Ctrl-C, so a shell that calls it in a loop stops the loop" got status 0 instead of 130. The handler now runs cleanup_current_file || true.

Before #568 the file's subshell could create exit_statuses again while the cleanup removed the state folder. That made rm -rf fail with "Directory not empty", and it is the most likely trigger of the CI failure. #568 closed that trigger. This change makes the handler safe for any other rm failure too.

2. No temp item is left when a signal comes while the runner makes them

The runner made four items with four mktemp calls and stored their names afterwards. A signal in between left up to four items that no cleanup knew. Now the runner picks the name of one folder with mktemp -u, stores it, and only then makes the folder with mkdir. All items live in that folder. mkdir fails when the name exists. The runner then drops the name again.

INT, TERM and HUP now share one handler. 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. Before, bash ended on SIGTERM and SIGHUP at once, ran the EXIT cleanup, and a mkdir that was still running made the folder afterwards.

3. The cleanup renames the folder first, so the shell that starts the Neovim sees it

The shell that execs timeout writes its process id to a file that the cleanup reads. A signal before that write left the cleanup with nothing to kill, and the shell then started a Neovim that ran on after the runner died.

Now the cleanup moves the work folder into a fresh folder next to it, made with mktemp -d, so the old path is gone at once. It then reads the process id file in the moved folder, kills timeout and its process group, and removes the fresh folder and the old path. A Neovim that still runs between the move and the kill can make its state folder at the old path again, so the old path goes last, after the kill. When the grave or the move fails, the folder keeps its old name and the cleanup works there. The shell writes its id, then looks for the folder at the old path, and it exits when it finds none. So one of the two sees the other. An earlier version of this PR used a cancel file inside the folder. Codex showed that rm -rf removes that file before the folder, which left a gap. The move has no such gap. The previous head of this PR renamed the folder to <work>.gone. When a folder of that name existed, mv moved the work folder into it and rm then deleted the content of that folder. A name from mktemp -d cannot exist yet. The move also closes a second gap: tee opened its output file inside the folder while rm removed it, and rmdir then failed.

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. With bash 3.2 (the macOS /bin/bash) it stopped the rm half way, and the runner died with 130 and left the renamed folder.

4. A Neovim that the cleanup kills no longer leaves its socket folder in TMPDIR

Neovim makes $TMPDIR/nvim.<user>/XXXXXX for its server socket and removes it when it exits by itself. The cleanup kills it with SIGKILL, so every Ctrl-C left one. Main does the same. I ran the real runner with a real Neovim and pressed Ctrl-C: main left nvim.<user>/… in TMPDIR, and this head leaves nothing in TMPDIR. Each file now gets TMPDIR and XDG_RUNTIME_DIR that point at a tmp/ folder in its work folder, so the socket folder goes away with the work folder. A work folder with a name over 80 characters goes to /tmp. The runner counts characters, not bytes, so a multibyte TMPDIR can still pass the limit (see "Not proven"). Without that, a TMPDIR of about 92 to 105 characters made the socket path pass the 107 bytes that libuv allows. libuv cut the path, and a socket file named tmp.XXXXXXXX landed in the TMPDIR of the developer (v0.8.0 left 38 in one run). This needs a writable /tmp. Without one the folder stays in TMPDIR.

README

scripts/README.md no longer lists the closed windows as open. It lists the ones that remain (see "Not proven").

Tests

Each new test fails on a runner without its change. I checked that with the runner of main, with the runner of the earlier head of this PR (c4e00f8), and with copies of the final runner that each lack one change. A stub rm, mv, cat, mktemp, mkdir, tee or a BASH_ENV file creates each moment, and no test depends on sleeps for its verdict. One exception, the timing of the during_rm tests, is under "Not proven".

Defect 1: a stub rm removes everything and then exits with 1. The calling shell's loop must stop with status 130.
Defect 2: a mktemp stub and a mkdir stub send SIGINT right after they made something. A mkdir stub also sends SIGTERM or SIGHUP to the runner alone. In the test for the second mkdir, the stub rm of the cleanup waits half way until that mkdir ends, so mkdir makes its folder while rm runs. The runner must leave nothing in TMPDIR. Another test checks that the runner removes no folder that it did not make.
Defect 3: a BASH_ENV file stops the shell that execs timeout before its first command, or between the redirection that makes the process id file and the write, until the test lets it go. The tests let it go after the runner died, and while the stub rm has deleted the files and not yet the folders. Two more tests let tee open its output file at that moment. A test makes a folder where the process id file belongs, so the write fails, and expects no Neovim. A test makes the rename fail and expects the folder removed under its old name. A stub mv waits while the shell of the file starts its Neovim, and the test expects that Neovim to die: a cleanup that reads the process id and kills before it moves the folder, or that reads, moves and then kills, leaves it running. Nine tests send a second signal, INT, TERM or HUP after each of the three, while the rm of the cleanup runs, and expect that rm to live on and the first signal's status. Two tests check that a runner that gets SIGTERM or SIGHUP dies of it. Bash stops a calling loop only after a child that died of SIGINT, so for SIGTERM and SIGHUP the calling shell prints the report that bash prints for a killed child ("Terminated", "Hangup") and the status, and it prints no report for an exit with 143 or 129. A test puts a folder named <work>.gone next to the work folder and expects the cleanup to leave it alone. A cat stub holds the cleanup after the move and before the kill while the stub Neovim writes into the old path, and the test expects nothing left.
Defect 4: one test checks that TMPDIR and XDG_RUNTIME_DIR of the file's Neovims are the tmp/ folder of the work folder. A second test runs the runner under a TMPDIR of 100 characters and checks that the work folder is in /tmp, that a socket path in it stays under 107 bytes, that the folder is gone afterwards, and that nothing stays in the long TMPDIR.
Each signal test puts the status, the leftovers and the output of the runner and of the calling shell into its failure message. The CI failure would have explained itself.
The stubs that hold a process wait 20 seconds at most. Against the runner of main, which never calls mkdir, a wait without a limit hung the spec for 28 minutes.
The SIGHUP tests report pending where SIGHUP is ignored when the test process starts, as under nohup. The test that reads which files the stubs hold open reports pending on a host without /proc/<pid>/fd.

Failures of the new spec (105 tests) against other runners:

Runner	Failing tests of 105	Which
main (`bcf1f08`)	31	The CI test (cleanup fails), the status-file test, the `TMPDIR` and long-`TMPDIR` tests, the nine second-signal tests, the two death tests, the `mv` test, and the 15 signal-timing tests
`c4e00f8`, the earlier head with the `cancel` file	20	The `TMPDIR` test, the long-`TMPDIR` test, all nine second-signal tests, the two death tests, SIGTERM and SIGHUP while `mkdir` 1 and 2 runs (4 tests), SIGINT while `rm` is half way, SIGTERM and SIGINT with `tee` during `rm` (2 tests)
`3ee7698`, the previous head	2	The long-`TMPDIR` test and the `<work>.gone` test
cleanup reads the process id and kills, then moves the folder	1	The Neovim started while `mv` waits
cleanup reads the process id, moves the folder, then kills	2	The Neovim started while `mv` waits, and SIGINT with `tee` during `rm`
handler ignores INT only	6	TERM and HUP as the second signal, after each of the three
handler resets only EXIT and INT	2	The SIGTERM and SIGHUP death tests
cleanup moves the folder to `<work>.gone`	1	The `<work>.gone` test
no move to /tmp for a long `TMPDIR`	1	The long-`TMPDIR` test
cleanup removes only the old path, not the grave	35	Every test that finds the grave left in `TMPDIR`
cleanup removes only the grave, not the old path	2	The live-write test and the move fails
no move (cleanup works on the old name)	3	SIGINT while `rm` is half way, SIGTERM and SIGINT with `tee` during `rm`
no `[ -d ]` check of the launcher	1	SIGINT between the redirection and the write of the process id
`;` instead of `&&` in front of that check	1	The write of the process id fails
no `\|\| true` on the cleanup in the handler	1	The cleanup fails
no TERM and HUP traps	10	SIGTERM and SIGHUP while `mkdir` 1 runs, six second-signal tests, two death tests
name stored after `mkdir`	5	SIGINT right after `mkdir` 1, SIGTERM and SIGHUP while `mkdir` 1 runs, two death tests
no guard on a failing `mkdir`	1	The name exists
no private `TMPDIR`	2	The `TMPDIR` test and the long-`TMPDIR` test
no `trap '' INT TERM HUP` in the handler	9	All nine second-signal tests

Each row is one run of the whole spec, started in a session where SIGHUP is not ignored, so every SIGHUP test ran.

Evidence

Final head cde2e40: MISE_LOCKFILE=false mise run all with GNU timeout first on PATH exits 0 with 1936 successes, 0 failures, 0 errors, 36 pending, luacheck 0 warnings and 0 errors in 200 files, and 0 files changed by format. mise run format-check, shellcheck and shfmt on the runner exit 0.
The runner spec (105 tests) passed 20 of 20 times on the final head, two runs at a time under the host's load: 15 with timeout from uutils coreutils, 3 with GNU bash 3.2.57 as bash on PATH and as the interpreter, and 2 with GNU timeout. These runs did not ignore SIGHUP, so no test was pending.
The whole runner with real Neovim gives the same counts as main (f28eba7): 856 successes on 0.12.3 and 852 on v0.8.0. That holds with TMPDIR unset, with a space and a % in it, and on v0.8.0 with a TMPDIR of 92, 100 and 105 characters (0.12.3: 100). After each run TMPDIR is empty and has no socket, and /tmp has no new folder of the runner. No file got a TIMEOUT line. The previous head left 38 socket files in a TMPDIR of 100 characters on v0.8.0.
A real Ctrl-C after 7 to 10 seconds of the real runner and a real Neovim, 15 times (5 of them with a TMPDIR of 100 characters): every run exits 130, and nothing is left in TMPDIR after 1 second or after 5 seconds. Main leaves nvim.<user>/… socket folders in TMPDIR. A first version of the move, which did not remove the old path, left tmp.X/state/nvim/… in 7 of 15 such runs on my host (an independent verifier saw 1 of 10). A live Neovim made that folder again between the move and the kill. The cat stub test covers it.

Not proven

I did not prove that the status-file race caused the CI failure. The deterministic repro uses a slow rm that reports failure. Main had 0 failures in 440 runs of the single test, in 2300 runs of the runner under a calling shell, and in 3200 runs of the runner alone, so the natural race did not reproduce.
Windows that remain: SIGKILL gives the runner no time to clean up, so the folder stays. A runner that leads its own session on a terminal 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. If the name from mktemp -u already exists and a signal comes before the runner drops the name, the cleanup can remove that folder. A second signal that comes before the first line of the handler runs is not ignored.
When the grave or the move fails, the folder keeps its old name. A shell that acts while the delete runs can then still start a Neovim, and the folder stays. Main has the same race. One test covers a failing move, and it has no shell that acts at that moment.
A work folder moves to /tmp only when /tmp is a writable folder. Without one it stays in TMPDIR, and a socket path over 107 bytes is cut by libuv and can name a file there. The limit of 80 characters leaves room for a socket name of 20 characters in the folder (/tmp/nvim.<7 digits>.<n>).
The limit counts characters, not bytes. A TMPDIR with multibyte characters can pass the 107 bytes without passing 80 characters, and then leaves socket files in TMPDIR (a verifier saw 38 with a 48-character, 94-byte TMPDIR on v0.8.0, where main left 54).
The death tests for TERM and HUP read bash's English "Terminated" and "Hangup". In a translated locale they fail on a correct runner.
An error exit (for example a failing exec 9>) followed by a signal during its cleanup can leave the first cleanup's grave folder, with empty work/config and work/tmp, in TMPDIR. The exit status stays right and a calling loop stops. A verifier found this in 4 of 6 such orders.
A follow-up PR fixes the three items above.
The during_rm tests let the held process go, wait one second, and then let the rm go on. On a host so slow that the held process needs more than one second for its next command, such a test can pass on a runner that has the defect. A slow host cannot make it fail on a correct runner.
I tested bash 3.2 with a GNU build of 3.2.57, not with Apple's /bin/bash.

Generated with xum • Model: anthropic:claude-sonnet-5-5 • Thinking: xhigh

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/<pid>/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`_

…n the runner's start-up windows 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 2a4c214 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`_

…tubs 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.

ThomasK33 · 2026-11-12T23:45:29Z

@codex review

ThomasK33 · 2026-11-12T23:45:39Z

@codex security review

chatgpt-codex-connector · 2026-11-12T23:45:43Z

Codex Review Summary: ✅ Completed

This comment shows the latest Codex review activity on this pull request.

Review	Status	Commit	Review trigger
📝 Code Review	✅ Completed 2026-11-13T10:34:48.220503Z	`cde2e40`	Manual request
🔒 Security Review	✅ Completed 2026-11-13T10:32:52.888756Z	`cde2e40`	Manual request

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

chatgpt-codex-connector · 2026-11-12T23:49:33Z

🛡️ Codex Security Review

Security review completed. No security issues were found in this pull request.

Reviewed commit: c4e00f8181

View security finding report

Only the user who started this review can view the report in Codex.

ℹ️ About Codex security reviews in GitHub

This is an experimental Codex feature. Security reviews are triggered when:

You comment "@codex security review"
A regular code review gets triggered (for example, "@codex review" or when a PR is opened), and you’re opted in so security review runs alongside code review

Once complete, Codex will leave suggestions, or a comment if no findings are found.

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: c4e00f8181

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

…ne and trap TERM and HUP 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`_

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`_

… Neovim a temp folder of its own 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`_

…s the first cleanup 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`_

…er fails 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`_

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`_

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`_

ThomasK33 · 2026-11-13T05:35:30Z

@codex review

ThomasK33 · 2026-11-13T05:35:32Z

@codex security review

chatgpt-codex-connector · 2026-11-13T05:37:47Z

Codex Review: Didn't find any major issues. More of your lovely PRs please.

Reviewed commit: 3ee7698a31

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

chatgpt-codex-connector · 2026-11-13T05:38:50Z

🛡️ Codex Security Review

Security review completed. No security issues were found in this pull request.

Reviewed commit: 3ee7698a31

View security finding report

Only the user who started this review can view the report in Codex.

ℹ️ About Codex security reviews in GitHub

This is an experimental Codex feature. Security reviews are triggered when:

You comment "@codex security review"
A regular code review gets triggered (for example, "@codex review" or when a PR is opened), and you’re opted in so security review runs alongside code review

Once complete, Codex will leave suggestions, or a comment if no findings are found.

…ckets short, and test the cleanup order Verifier findings F1, F2, F4 and F5 on 3ee7698. F2: the cleanup renamed the folder to <work>.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 <work>/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`_

…the runner 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`_

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`_

…leanup 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`_

ThomasK33 · 2026-11-13T10:29:11Z

@codex review

ThomasK33 · 2026-11-13T10:29:20Z

@codex security review

chatgpt-codex-connector · 2026-11-13T10:32:51Z

🛡️ Codex Security Review

Security review completed. No security issues were found in this pull request.

Reviewed commit: cde2e4047c

View security finding report

Only the user who started this review can view the report in Codex.

ℹ️ About Codex security reviews in GitHub

This is an experimental Codex feature. Security reviews are triggered when:

You comment "@codex security review"
A regular code review gets triggered (for example, "@codex review" or when a PR is opened), and you’re opted in so security review runs alongside code review

Once complete, Codex will leave suggestions, or a comment if no findings are found.

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: cde2e4047c

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

chatgpt-codex-connector · 2026-11-13T10:34:45Z

+  # 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


Measure the temp path in bytes before accepting it

When TMPDIR contains multibyte characters under a UTF-8 locale, Bash's ${#work_dir} counts characters while the Unix-socket limit described here is measured in bytes. For example, a path containing fifty é characters can remain below 80 characters while already exceeding 100 bytes, so it bypasses the /tmp fallback and the appended Neovim socket name exceeds the limit. This leaves the exact socket-truncation and cleanup leak that the fallback is intended to prevent; base the threshold on the encoded byte length instead.

Useful? React with 👍 / 👎.

chatgpt-codex-connector · 2026-11-13T10:34:45Z

+
+  -- 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


Make the signal-report assertions locale-independent

When the suite runs under a non-English locale, Bash/libc can localize the diagnostic printed for a child terminated by SIGTERM or SIGHUP, so caller_output need not contain the literal English strings Terminated or Hangup even though the runner correctly died from the signal. These new tests will therefore fail on otherwise supported developer environments; run the nested shell with LC_ALL=C or verify signal termination without matching localized prose.

Useful? React with 👍 / 👎.

ThomasK33 · 2026-11-13T12:21:48Z

Maintainer decision (ThomasK33, 2026-11-13, in the agent chat): option B for this PR.

Waiver recorded before the merge: merge with the two open Codex findings (a multibyte TMPDIR skips the move to /tmp, and the death tests match English signal names), and ship the fix in a follow-up PR. Neither finding is a security flaw, data loss or broken core behavior. The merge still waits for the verifier's verdict on this head and the readiness check.

Generated with xum • Model: anthropic:claude-opus-5-5 • Thinking: high

ThomasK33 · 2026-11-13T13:13:03Z

Maintainer decision (ThomasK33, 2026-11-13, in the agent chat): option A for this PR, after the last verifier's verdict.

Record before the merge of cde2e404:

The last verifier found one new problem, N1. An error exit (for example a failing exec 9>) followed by a signal during its cleanup can leave the first cleanup's grave folder, with empty work/config and work/tmp, in TMPDIR. The exit status stays right (130, 143 or 129) and a calling loop stops. It needs two faults at once and leaves only empty folders, so it is not a security flaw, data loss or broken core behavior.
It also confirmed the two open Codex findings (a multibyte TMPDIR and the English signal names in the death tests).
A clean-context readiness check recommended merging now and fixing all three in a follow-up PR.
Final commit cde2e404. CI passes on it. All six Codex review requests are used.
Follow-up PR in this run: the byte count with its test, LC_ALL=C in the death tests, the README wording, an N1 fix that keeps the grave name where a second cleanup can find it, a test for N1 that fails on this head, and boundary tests for the 80 limit, for leftovers in /tmp and for kill before delete.

Generated with xum • Model: anthropic:claude-opus-5-5 • Thinking: high

…runner's limits (coder#605) ## Why coder#592 makes `scripts/run_integration_tests_individually.sh` stop on Ctrl-C, SIGTERM and SIGHUP and leave nothing behind. A verifier and two Codex threads found six gaps on its head cde2e40. None of them changes the status that the runner returns for a signal. This PR closes them and adds the tests that were missing. It follows coder#592, which merged as 0e10d2a. The record comment of coder#592 is coder#592 (comment). ## What changed ### Fixes 1. **A second cleanup removes the grave of the first (verifier finding N1).** After an error exit, the cleanup runs as the EXIT trap and ignores no signal. A signal that came while it ran started a second cleanup in the signal handler. The second cleanup made its own grave, found the old path gone, and removed only its own grave. The first grave stayed in `TMPDIR` with `config/` and `tmp/` in it. The cleanup now stores the name of its grave in a global before it makes the folder (`mktemp -u`, then `mkdir -m 700`). A second cleanup finds that name and removes the grave. The EXIT path still ignores no signal, so under bash 5 Ctrl-C ends with 130 and stops a calling loop (for bash 3.2 see "Not proven"). 2. **The 80 limit counts bytes (Codex thread, finding K1).** Bash counts characters with `${#var}` in a UTF-8 locale, but libuv cuts a socket path at 107 bytes. A `TMPDIR` of 64 characters and 99 bytes kept its work folder, and v0.8.0 then left 38 socket files in `TMPDIR`. The runner now counts bytes with `LC_ALL=C`. `scripts/README.md` says bytes. 3. **The death tests run in the C locale (Codex thread, finding K2).** The two tests that read the report that bash prints for a child killed by SIGTERM or SIGHUP matched the English words. A translated libc prints other words, and the tests failed on a correct runner. The calling shell of those tests now runs with `LC_ALL=C`. ### Tests - Six tests send INT, TERM and HUP while the cleanup of an error exit waits at its `mv` and at its `rm`. The INT tests run in a calling loop and expect it to stop. A seventh test sends SIGINT right after the `mkdir` that makes the grave. - Two tests give the runner a work folder name of exactly 80 and 81 bytes through the `mktemp` stub and expect the first to stay and the second to move to `/tmp` (finding M1). - A test makes the `rm` stub wait half way and checks then that the file's Neovim is already dead, so the cleanup kills before it deletes (finding M3). - The signal tests keep their `TMPDIR` short through `short_temp()` (finding M2). Before, a test process with a `TMPDIR` of about 42 characters or more sent every work folder to `/tmp`, where the leftover checks did not look. Writing the helper showed that four more tests had the same flaw: the test of a name that exists, the `.gone` test and the two boundary tests built a folder under the test folder. The first of them failed on the unchanged runner under a long `TMPDIR`. The multibyte test needed a short parent too. ## Evidence All runs use GNU timeout first on `PATH` and a stub `claude`. Every run below passed on its first try. No test passed only on a retry. - **Gate on the head of this PR.** `mise run clean && mise run all` exits 0: 1970 successes, 0 failures, 0 errors, 38 pending, luacheck 0 warnings and 0 errors in 206 files, and 0 files changed by format. - **Runner spec on the head of this PR (116 tests, no skips).** Bash 5 with a long `TMPDIR` of 85 characters and with a multibyte `TMPDIR`, and bash 3.2.57 (GNU build, as `bash` on `PATH` and as interpreter) with the same two folders: all four runs pass 116 of 116. - **Earlier runs of the same three files on top of cde2e40 (before they moved onto main).** The gate gave 1947 successes, 0 failures, 0 errors, 36 pending. The spec passed 116 of 116 with bash 5 under a short, a long and a multibyte `TMPDIR` (3 baseline runs plus 3 plain runs), and with bash 3.2.57 under all three. In a German libc sandbox (a compiled de_DE.UTF-8 locale and a hand-made `libc.mo`, run with `bwrap`, no root) it gave 116 ok, 0 failures, 6 pending. The 6 are the wide-digit tests, which need other digits in `[0-9]`. The live runs and the mutants below come from that build. - **K1 live, v0.8.0, whole runner, `TMPDIR` of 64 characters and 99 bytes.** On the build of these commits on cde2e40, cde2e40 left 38 socket files and the build left none, with 852 successes each (0.12.3: nothing left, 856). On this head an independent verifier measured main `d21f310a` at 40 socket files and this head at 0, with 865 successes on v0.8.0 and 869 on 0.12.3. - **K2.** In the German sandbox the two death tests fail 2 of 2 without the `LC_ALL=C` prefix and pass 2 of 2 with it. - **N1.** The runner of cde2e40 fails 6 tests of the new spec: the multibyte test, INT, TERM and HUP at `mv`, INT at `rm`, and the test right after the grave. TERM and HUP at `rm` pass on both runners, because the interrupted `rm` had finished by then. Mutants of the final runner, each run against the whole spec: | Mutant | Failing tests | Which | |---|---|---| | characters instead of bytes | 1 | The multibyte test (also under a long `TMPDIR`) | | runner of 7b820d4d (byte count, old grave) | 5 | The four error-exit tests and the grave test | | grave name stored after `mkdir` | 1 | The grave test | | grave name in a local variable | 1 (an independent verifier's version of this mutant failed 5) | The grave test | | limit 79 | 1 | The 80-byte test | | limit 92 | 2 | The 81-byte test and the multibyte test | | limit 100 | 2 | The 81-byte test and the multibyte test | | rm, then kill | 1 | The kill-before-rm test | | no move into the grave | 6 | INT, TERM and HUP at `mv`, and three tests where `rm` is half way | | rm only the grave, not the old path | 2 | The live-write test and the failed-move test | Reverting the M2 change shows what it fixes. Under a `TMPDIR` of 85 characters, the spec before this change failed the mutant "rm only the grave" only in the test of a name that exists, which fails on the unchanged runner too. The live-write test and the failed-move test passed, because the folders were in `/tmp`. With this change the same mutant fails those two tests under a long and under a short `TMPDIR`. ## Not proven - The two death tests for TERM and HUP check the status and the bash kill report, and the four new error-exit tests for TERM and HUP check the status and the leftovers. None checks that a calling loop stops. They cannot, because bash stops a loop only after a child that died of SIGINT. - No test holds the cleanup between `mktemp -u` and `mkdir`. In that state the grave name is stored and the folder does not exist, so the move fails and the cleanup works on the old path, as in the failed-move test. An independent verifier sent INT, TERM and HUP there under bash 5 and bash 3.2: nothing was left and the status was right. - Storing the grave name before `mkdir` opens a name race that `mktemp -d` did not have: if another process makes a folder at that exact name and a signal comes before the runner drops the name, the cleanup removes that folder. In the same forced sequence, a nested cleanup could also read a process id from that folder and kill that process group. No process can know the name in advance, the window is microseconds, and main has the same race for its work folder. The README line ("no name that exists is touched") and the runner comment overstate this. - Under bash 3.2, a Ctrl-C during the cleanup's `$(mktemp ...)` after an error exit makes the runner exit with status 130 instead of dying from the signal, so a calling loop goes on. Main does the same. - The kill-before-delete test checks nothing on a host without `ps`. - I tested bash 3.2 with a GNU build of 3.2.57, not with Apple's `/bin/bash`. - A work folder moves to `/tmp` only when `/tmp` is a writable folder. Without one it stays in `TMPDIR`, as in coder#592. --- _Generated with [`xum`](https://github.com/coder/xum) • Model: `anthropic:claude-sonnet-5-5` • Thinking: `xhigh`_

ThomasK33 added 4 commits November 12, 2026 22:30

chore: merge origin/main into fix/runner-sigint-race

ae9f102

docs(scripts): correct runner comments on temp folder name and test s…

c4e00f8

…tubs 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.

ThomasK33 marked this pull request as ready for review November 12, 2026 23:45

chatgpt-codex-connector Bot reviewed Nov 12, 2026

View reviewed changes

Comment thread scripts/run_integration_tests_individually.sh Resolved

Comment thread scripts/run_integration_tests_individually.sh Resolved

ThomasK33 added 12 commits November 13, 2026 00:07

chore: merge origin/main into fix/runner-sigint-race

dbefc8b

chore: merge origin/main into fix/runner-sigint-race

7abe910

Merge remote-tracking branch 'origin/main' into fix/runner-sigint-race

ae30aa7

Merge remote-tracking branch 'origin/main' into fix/runner-sigint-race

1baeed4

Merge remote-tracking branch 'origin/main' into fix/runner-sigint-race

3ee7698

ThomasK33 added 4 commits November 13, 2026 06:59

Merge remote-tracking branch 'origin/main' into fix/runner-sigint-race

cde2e40

chatgpt-codex-connector Bot reviewed Nov 13, 2026

View reviewed changes

ThomasK33 mentioned this pull request Nov 13, 2026

Deferred follow-ups from the 2026-11 fix run #603

Open

47 tasks

ThomasK33 added this pull request to the merge queue Nov 13, 2026

Merged via the queue into main with commit 0e10d2a Nov 13, 2026
8 checks passed

ThomasK33 deleted the fix/runner-sigint-race branch November 13, 2026 13:30

ThomasK33 mentioned this pull request Nov 13, 2026

fix(scripts): remove the grave of an interrupted cleanup and pin the runner's limits #605

Merged

Conversation

ThomasK33 commented Nov 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Why

What changed

1. A failing cleanup no longer swallows Ctrl-C

2. No temp item is left when a signal comes while the runner makes them

3. The cleanup renames the folder first, so the shell that starts the Neovim sees it

4. A Neovim that the cleanup kills no longer leaves its socket folder in TMPDIR

README

Tests

Evidence

Not proven

Uh oh!

ThomasK33 commented Nov 12, 2026

Uh oh!

ThomasK33 commented Nov 12, 2026

Uh oh!

chatgpt-codex-connector Bot commented Nov 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

chatgpt-codex-connector Bot commented Nov 12, 2026

🛡️ Codex Security Review

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

Uh oh!

Uh oh!

ThomasK33 commented Nov 13, 2026

Uh oh!

ThomasK33 commented Nov 13, 2026

Uh oh!

chatgpt-codex-connector Bot commented Nov 13, 2026

Uh oh!

chatgpt-codex-connector Bot commented Nov 13, 2026

🛡️ Codex Security Review

Uh oh!

ThomasK33 commented Nov 13, 2026

Uh oh!

ThomasK33 commented Nov 13, 2026

Uh oh!

chatgpt-codex-connector Bot commented Nov 13, 2026

🛡️ Codex Security Review

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

chatgpt-codex-connector Bot Nov 13, 2026

Choose a reason for hiding this comment

Uh oh!

chatgpt-codex-connector Bot Nov 13, 2026

Choose a reason for hiding this comment

Uh oh!

ThomasK33 commented Nov 13, 2026

Uh oh!

ThomasK33 commented Nov 13, 2026

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

ThomasK33 commented Nov 12, 2026 •

edited

Loading

chatgpt-codex-connector Bot commented Nov 12, 2026 •

edited

Loading