fix(runner): name why a file failed when nvim exits non-zero without a reason by ThomasK33 · Pull Request #554 · coder/claudecode.nvim

ThomasK33 · 2026-11-12T07:26:58Z

Why

#532 left four follow-ups from its last review and the readiness advisor. This PR makes them, and it fixes what the review of this PR found.

What changed

A line with a reason for every non-zero exit, by stage. A file whose Neovim exits non-zero gets at least one line: FAILED: <file>: no plenary summary (nvim exit N), the TIMEOUT: line, or a new FAILED: <file>: nvim exit N. The new line prints when neither of the other two did, for example when Neovim crashes or exits 1 after a clean summary. A file whose specs fail (plenary exits 1) now also gets FAILED: <file>: nvim exit 1. A file with no summary whose status is 124, 137 or 143 gets two lines, the no-summary line and the TIMEOUT: line, as it does on main. A crash before the summary (for example 139) gets one line, the no-summary line. An exit status above 128 names its signal: nvim exit 139, SIGSEGV, also in the no-summary line.
A failing tee gets its own line, FAILED: <file>: tee exit N, with the signal name above 128 (tee exit 141, SIGPIPE). On main, a tee failure after a clean summary printed no reason line, and the no-summary line showed the status of tee as the status of Neovim (a tee that exited 3 gave nvim exit 3). The file's subshell writes the exit statuses of Neovim and of tee to a file in the state folder, so the runner's cleanup deletes it with the folder. A subshell that ends after the cleanup (a signal stop) usually finds the folder gone, and its write fails without a message. When both fail, both lines print.
Known limit: the subshell can still open its file while the cleanup runs. Under strace -f, this left the state folder with exit_statuses in TMPDIR and made rm print Directory not empty in 3 of 3 runs (2 scripted runs and 1 manual run), and main was clean in the 1 run under the same setup. At normal speed it happened in 0 of 150 runs on this PR, and in 0 of 60 runs on main. A follow-up PR in this run fixes it.
The TIMEOUT: lines are true. Exit 137 comes from timeout -k and also from an out-of-memory kill: TIMEOUT: <file> was stopped (exit 137, SIGKILL): nvim did not exit within 40s, or another process killed it. An outside SIGTERM that reaches timeout gives 124 with uutils timeout and 143 with GNU timeout (a child that dies of the signal), so the lines for 124 and 143 now end with or an outside signal stopped it.
The file's process group is killed after any non-zero exit of Neovim (a tee failure alone does not kill it). Before, only a timeout status killed it. When the Neovim that the runner starts died by a signal before its summary, the Neovim that plenary started for the spec ran on. This is on main too. The cleanup after a signal to the runner is unchanged.
Known limit, not changed: the kill uses the process group id that timeout had. A process group id that another process reused in the few milliseconds after timeout ended would get SIGKILL. pid_max is 4194304 on my host, so a reuse needs a wrap-around in those milliseconds. Main has had the same risk after a timeout status since fix(scripts): fail the integration runner when a spec hangs and run command_args_spec again #445. I declined to change it, because the chance is very small (a wrap-around of 4194304 ids inside those milliseconds) and main already takes the same risk after a timeout status. The follow-up PR records this reason.
README. scripts/README.md says what the early window does for a runner that leads a pty session: SIGTERM, SIGHUP, SIGINT and SIGUSR1 leave nothing and start no Neovim, SIGKILL starts no Neovim but leaves all four temp items (without a terminal the Neovim starts as well), and bash ignores SIGQUIT so the file runs to its end. It also describes the new output lines and where the statuses go.
Comments and tests. The comment on the reason lines says what is true (see 1). The -i NONE test also checks --headless and -u followed by tests/minimal_init.lua.

Tests

New or changed runner spec tests (the names are the columns of the table below):

clean_after: five tests. A clean summary followed by exit 139, 134, 130, 129 or 1 prints the FAILED: line (with , SIGSEGV, , SIGABRT, , SIGINT, , SIGHUP or no name), exactly one FAILED: line, the counts in the totals, and the file in the final list.
crash_before: no summary and exit 139 names SIGSEGV in the no-summary line, with one FAILED: line and no TIMEOUT line.
sigkill_timeout and kill_after: a stub Neovim that kills itself with SIGKILL gets the TIMEOUT: text, exactly once, and no extra FAILED: line after a clean summary.
stuck_137: the test of a Neovim that ignores SIGTERM expects (exit 137, SIGKILL).
tee_stage: tee exits 3 after a clean summary: FAILED: <file>: tee exit 3, no nvim exit text.
both_stages: Neovim exits 1 and tee exits 141: both lines, tee exit 141, SIGPIPE.
outside_term: an outside SIGTERM reaches timeout while the stub Neovim sleeps. The TIMEOUT: line has the new text with exit 124 (uutils) or 143 (GNU), exactly once.
p3_orphan: exit 139 before any summary with a process left in the group: the process is killed.
cleanup: after a file that failed, with a failing tee, nothing is left in TMPDIR, the status file included.
late_tee: a tee that ends a second after the SIGTERM, long after the runner cleaned up: nothing is left in TMPDIR, and the runner prints no No such file message.

Each test ran against each mutant of the runner, with uutils timeout 0.10.0 and with GNU timeout 9.7. The two builds gave the same table. Red means the test fails.

Variant	clean_after	crash_before	sigkill_timeout	kill_after	stuck_137	tee_stage	both_stages	outside_term	p3_orphan	cleanup	late_tee
this PR	green	green	green	green	green	green	green	green	green	green	green
main's runner	red	red	red	red	red	red	red	red	red	red	green
first push of this PR	green	green	green	green	green	red	red	red	red	red	green
signal names only above 130	red	green	green	green	green	green	green	green	green	green	green
status file ignored	green	green	green	green	green	red	red	green	green	green	green
no `tee` line	green	green	green	green	green	red	red	green	green	green	green
a `tee` failure does not fail the file	green	green	green	green	green	red	green	green	green	green	green
`tee` line without the signal name	green	green	green	green	green	green	red	green	green	green	green
124 and 143 lines as on main	green	green	green	green	green	green	green	red	green	green	green
group kill only after a timeout status	green	green	green	green	green	green	green	green	red	green	green
no group kill after a non-zero exit	green	green	green	green	green	green	green	green	red	green	green
state folder not deleted	green	green	green	green	green	green	green	green	green	red	green
nvim line uses the status of the subshell	green	green	green	green	green	red	green	green	green	green	green
status file in the temp folder (previous push)	green	green	green	green	green	green	green	green	green	green	red
status write without the guard for a folder that is gone	green	green	green	green	green	green	green	green	green	green	red

The existing test of what a timed-out file leaves running is green for this PR and for the variant that kills the group only after a timeout status. It is red when the group kill is removed. late_tee is green for main and for the first push because neither has a status file.

Not tested: the fallback for a status file that the runner cannot read. The runner then uses the exit status of the subshell. A variant whose subshell exits with the status of Neovim only, and so loses the status of tee in that fallback, passes every test. A follow-up PR in this run adds a test for it.

Real Neovim

A tree with one spec, timeout 0.10.0 (uutils) and 9.7 (GNU), stdin /dev/null:

Case	main	this PR
the runner's own Neovim dies by SIGSEGV after the spec's Neovim started	`FAILED: … (nvim exit 139)`, the spec's Neovim still running 3 s after the runner ended	`FAILED: … (nvim exit 139, SIGSEGV)`, the spec's Neovim gone
SIGTERM to the `timeout` that runs the runner's Neovim, before the summary, uutils	exit 124, `TIMEOUT: … (exit 124): nvim did not exit within 40s`	exit 124, the same line ending `, or an outside signal stopped it`
the same, before the summary, GNU	real Neovim exits 1 on SIGTERM: `FAILED: … no plenary summary (nvim exit 1)`, no `TIMEOUT:` line	the same output
the same, after the summary, uutils	the `TIMEOUT:` line only	the `TIMEOUT:` line only, with the new ending
the same, after the summary, GNU	no line	`FAILED: … nvim exit 1`
a `tee` that exits 3 after a clean summary	runner exit 1, no reason line	`FAILED: a_spec.lua: tee exit 3`

Early window, a stub bash that delays the process id file, a hanging spec, the runner leading a pty session:

Signal to the runner	Neovim started	Temp items left
SIGTERM, SIGHUP, SIGINT (Ctrl-C)	no	none
SIGKILL	no	all four
SIGQUIT	yes, the file runs to its end (bash ignores the signal)	not applicable

Without a terminal, SIGKILL to the runner leaves all four items and the Neovim starts.

Whole runner, main versus this PR

An independent verifier ran this on this exact head, with stdin /dev/null and TMPDIR unset. The rows are identical: 123 per-file rows each (30 files with a Running line and three count lines, plus three totals), no difference, and no FAILED, TIMEOUT, E886, Killed or No such file line.

Neovim	main	this PR
0.12.3	527 passed, 0 failed, 0 errors	527 passed, 0 failed, 0 errors
v0.8.0	390 passed, 0 failed, 0 errors	390 passed, 0 failed, 0 errors

With TMPDIR set to a folder with a space and a % (what the CI guard uses), this PR on 0.12.3 also gives 527 passed, 0 failed, 0 errors.

Gate

The verifier ran mise run clean && mise run all on this exact head. The formatter changed no file (0 of 426), luacheck reports 0 warnings and 0 errors in 185 files, and busted reports 1751 successes, 0 failures, 0 errors, 28 pending. shellcheck on scripts/*.sh and actionlint pass.

A gate run of mine earlier in this round failed one test: the window test found the status file in TMPDIR after a signal stop, because it had been next to the output file. I moved it into the state folder and added the late_tee test.

Three merges of main came after the commit that moved the status file (51ed703b). They brought #556 with #458 (a docs change and a repro_issue_218.sh change), then #494 (a diff change with its own integration spec), then #555 (a wire-probe cleanup change). My last gate run covered only the first of them. I ran the spec that #494 added through this runner 10 times alone just now: 10 passed, in 3.8 to 5.4 seconds. One earlier run of mine hit the 40 second limit on it. I did not find the cause.

Follow-up

A follow-up PR in this run has no number yet. It holds these changes:

Wording. The README line on the output lines and a runner comment say "a timeout or a signal" for the two-line case. It is status 124, 137 or 143, and a crash with 139 or 134 gets only the no-summary line. The README line and a comment say an outside SIGTERM gives 143 with GNU timeout. That holds only when the child dies of the signal, and real Neovim gives 1. The README line says "because nvim did not exit in time", which the hedged TIMEOUT: text contradicts. The README sentences on the cleanup ("deletes the four" and the status file) leave out the known limit in item 1 above. Optional: no signal name prints for a status from 193 to 255, because bash has no signal above 64.
A fix for the known limit in item 1: no file path is opened after the cleanup starts, with a regression test. Two ways are not tried yet. The runner can open the status file on a fixed file descriptor before it starts the subshell, or the cleanup can kill and reap the subshell before rm -rf.
A test that reaches the fallback for an unreadable status file, and fails for the variant that exits with the status of Neovim only.
A record of the group id limit from item 3 and the reason I declined it. No code change.

Not in this PR

A 40 second limit per file (FILE_TIMEOUT) was reached at a high host load while specs ran. The limit is the same on main.

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

…a reason - A file whose nvim exits non-zero gets one line with a reason: the `FAILED: <file>: no plenary summary` line, the `TIMEOUT:` line, or a new `FAILED: <file>: nvim exit N` line. Before, a crash or an exit 1 after a clean summary printed nothing and the file only showed up in the final list. An exit status above 128 names its signal (`nvim exit 139, SIGSEGV`), also in the no-summary line. - The TIMEOUT line for exit 137 names SIGKILL and says "or another process killed it", because the out-of-memory killer gives 137 too. - The `-i NONE` test also checks `--headless` and `-u tests/minimal_init.lua`. - scripts/README.md says that a runner that leads its own session on a terminal leaves nothing after any signal in the early window, and lists the new output lines. - The comment on the subshell says that set -e ends it with the status of `tee` when `tee` fails. _Generated with xum • Model: anthropic:claude-sonnet-5-5 • Thinking: xhigh_

ThomasK33 · 2026-11-12T07:27:04Z

@codex review

ThomasK33 · 2026-11-12T07:27:12Z

@codex security review

chatgpt-codex-connector · 2026-11-12T07:27:22Z

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-12T11:11:48.760813Z	`79cf904`	Manual request
🔒 Security Review	✅ Completed 2026-11-12T11:11:52.415292Z	`79cf904`	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-12T07:28:52Z

Codex Review: Didn't find any major issues. Bravo.

Reviewed commit: 6e2cc4daa5

ℹ️ 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-12T07:29:06Z

🛡️ Codex Security Review

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

Reviewed commit: 6e2cc4daa5

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.

…ord the TIMEOUT lines - The subshell writes the exit statuses of nvim and of `tee` to a status file next to the output file. A failing `tee` gets its own line (`FAILED: <file>: tee exit N`) and no longer shows as an nvim exit. When both fail, both lines print. - The process group of the file is killed after any non-zero nvim exit, not only after a timeout status. The nvim that plenary starts for a spec used to run on when the nvim the runner starts died by a signal. - The TIMEOUT lines for 124 and 143 end with "or an outside signal stopped it": an outside SIGTERM reaches timeout as 124 with uutils and 143 with GNU. - The comment on the reason lines no longer says "always one line": a file with no summary that a timeout or a signal stopped gets two, as on main. - New tests: exit 129 and 130 name SIGHUP and SIGINT, tee stages, the outside SIGTERM with both timeout builds, the group kill after a crash, and that no temp item stays after a failed file. - scripts/README.md says what SIGKILL and SIGQUIT do in the early window of a runner that leads a pty session, and describes the new lines. _Generated with xum • Model: anthropic:claude-sonnet-5-5 • Thinking: xhigh_

_Generated with xum • Model: anthropic:claude-sonnet-5-5 • Thinking: xhigh_

The file's subshell wrote its exit statuses next to the output file. A subshell that ended after the runner cleaned up (a signal stop) wrote the file again and left it in TMPDIR. The file now lives in the state folder: the runner's cleanup deletes it with the folder, and a late write finds the folder gone and fails without a message. A test stubs a `tee` that ends a second after the signal and fails when anything is left in TMPDIR or the runner prints "No such file". _Generated with xum • Model: anthropic:claude-sonnet-5-5 • Thinking: xhigh_

_Generated with xum • Model: anthropic:claude-sonnet-5-5 • Thinking: xhigh_

ThomasK33 · 2026-11-12T11:08:46Z

@codex review

ThomasK33 · 2026-11-12T11:08:53Z

@codex security review

chatgpt-codex-connector · 2026-11-12T11:11:47Z

Codex Review: Didn't find any major issues. What shall we delve into next?

Reviewed commit: 79cf904191

ℹ️ 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-12T11:11:51Z

🛡️ Codex Security Review

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

Reviewed commit: 79cf904191

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.

…ing off (coder#541) ## What this fixes With `disable_broadcast_debouncing = true` and an empty queue, `_broadcast_at_mention` broadcast a send at once and skipped the readiness check. A Claude whose WebSocket handshake completed less than `connection_wait_delay` ms ago (600 ms by default) got the mention while it was still starting up. On real Neovim 0.8.0 and 0.12.3 it arrived 3 to 10 ms after the handshake. With the default config the same send waits the full 600 ms. The option is not in the README or the `config.lua` defaults. It is typed in `types.lua` and described in `lua/claudecode/README.md`, so a user config can set it. The fix is in `lua/claudecode/init.lua` (`M._broadcast_at_mention`): the immediate path now also needs `ms_until_a_client_is_ready()` to be 0. That means some connected client completed its handshake `connection_wait_delay` ms ago. Otherwise the mention goes through the queue, which sends it once a client is ready. Behavior is unchanged in two cases: - Some client is ready: the send still goes out at once, with nothing queued. - No client reports a handshake time: the immediate path still runs and fails at once with "Failed to broadcast …", as before. The verify-mention-queue skill (coder#523, now on main) found this through a Codex review on its model. coder#523 excluded this config from its `SentWhenReady` rule as a known bug. coder#523 merged first, so this PR adds the model branch and flips that wording. ## The verify-mention-queue model now covers this path - `model/MentionQueue.tla`: a new variable `immediate` holds the user's `disable_broadcast_debouncing`. Each run starts with either value and keeps it. With it on, a send while Claude is connected and the queue is empty broadcasts at once while some connected client is ready. Otherwise the send queues as before. The writes all succeed, all fail later, or are refused at once, as in a flush. - `check.sh`: a new fact, `ImmediateWaitsForReady`, read from `_broadcast_at_mention`. It reads TRUE only when the readiness guard comes before every `.broadcast(` and `queue_mention(` line of that function (a broadcast written as an indexed call or through an alias still reads TRUE; the unit tests catch those), so an unguarded broadcast added before the guard reads FALSE. With it FALSE, `SentWhenReady` breaks. `FailedWriteReported` now also reads the immediate send's `report_unwritten("command", …)`. - `SKILL.md`: `SentWhenReady` holds with `disable_broadcast_debouncing` on or off. The scope note now describes the check instead of a known bug, and the facts table lists the new fact. ## Tests The first two tests were committed before the fix (ebcdb89) and failed on main: - `tests/unit/mention_queue_spec.lua`, `D: with debouncing off, a send right after the handshake still waits for connection_wait_delay`: a fake loop runs a send 100 ms after the handshake. It expects the mention in the queue, nothing at 590 ms and the mention at 610 ms. On main the queue stayed empty and the mention went out at once. - `tests/integration/mention_immediate_send_spec.lua` (real Neovim, so CI runs it on stable and v0.8.0): a loopback Claude completes the handshake. - A send right after the handshake must reach Claude only once `connection_wait_delay` has passed. The time is measured on the loop clock that stamps `connected_at`. On main: `Claude got the mention 3 ms after its handshake, before 600 ms` (0.8.0) and `6 ms` (0.12.3). - A send once Claude is ready must still go out at once, with nothing queued. This one passes before and after the fix. - `tests/unit/mention_queue_spec.lua`, `D: with debouncing off, a send 1 ms before Claude is ready still waits in the queue` (verifier F1 on 24b7ee7, tightened after the verifier on 700cbf6). The first two tests send 500 ms or more before ready, so a readiness check with a tolerance survived them. The fake clock makes the 1 ms exact on any Neovim. Each of these mutations of the guard fails only this test: `< 2`, `< 20`, `<= 20` and `< 100`. With the fix, the spec passes 53 of 53. ## Replay runner and wording (minor findings from coder#523 round 6) - `replays/run.sh`: it exits 2 before the first replay when `NVIM` names no Neovim, and the error names the path as given. A `NVIM` in a missing folder used to turn into `/nvim`. bash's "Alarm clock" note now goes to the replay's log, not the terminal. The comment no longer cites a VimLeavePre error, because Neovim exits 0 after one. - `drain.lua` and `SKILL.md` now say "an 8.6 s wait before Claude connects" where they said "handshake". ## Checks run The gates ran on 4a06553, with main at d123707. The head then merges main's 37ee1e3 (coder#554, coder#564): `diff_inline.lua`, the integration runner and diff specs. None of them touches the mention queue or a file of this PR, and all four `covered.lock` files match the head. Two results ran on earlier commits: - `--mutants` ran on 3980bc3. Since then `check.sh` changed only in how it reads `ImmediateWaitsForReady`, which still reads TRUE, so each mutant run gets the same TLC input. - The statechart runs ran on 334f122. The model is the same on the head. The results on 4a06553: - `verify-mention-queue/check.sh`: PASS, 20 facts TRUE. `main` 540,240 distinct states, `live-NoStrandedMention` 115,628, `live-ReadyDelivers` 115,628, `live-FlushDelivers` 48,296. Three counts are exactly twice the old ones and `live-FlushDelivers` is 1.96 times (24,692 before), because each run checks both values of `immediate`. - `verify-mention-queue/check.sh --mutants`: exit 0 in 6543 s on a loaded host (load average about 60 to 118). Every guard is caught by the rule `EXPECT` names (`ImmediateWaitsForReady=FALSE` by `SentWhenReady`), and every rule breaks at least once. - A scratch copy of the model with an action property that says the immediate send never happens: TLC breaks it (314 states, 1 mention, 1 connect), so the branch is reachable. With `ImmediateWaitsForReady = FALSE`, `SentWhenReady` breaks. - `/statechart` with the committed view still charts three scratch breaks, `StopClearsQueue` and `FlushWaitsForReady` off. All 3 exited 1 with a chart. - Every other `verify-*/check.sh` passes: - `verify-diff-lifecycle`: 89,205 / 5,269 states. Its lock merged from main. - `verify-selection-tracking`: 129,986 states. - `verify-terminal-lifecycle`: 263,060 states. - The selection and terminal models hash all of `init.lua`, so I re-recorded their `covered.lock` files after the fix. Neither model reads `_broadcast_at_mention`. - `replays/run.sh`: 12 of 12 kept on Neovim v0.8.0 and 12 of 12 on v0.12.3, on the head. - With `NVIM=/nonexistent/nvim`, `./no-such-nvim` or `nvim-not-on-path`, it exits 2 before any replay and names the path. - On a copy of the head, a replay that prints KEPT and exits 1 reads BROKEN. A replay that hangs is ended by the alarm and reads BROKEN with exit 142. "Alarm clock" is in its log, and the terminal got 0 bytes of stderr. - busted, full suite, shuffled (seed 12345): 1769 successes, 0 failures, 0 errors, 29 pending. - `scripts/run_integration_tests_individually.sh`: Neovim v0.8.0, 400 successes, 0 failed, 0 errors. Neovim v0.12.3, 537 successes, 0 failed, 0 errors. `mention_immediate_send_spec.lua` passed 2 of 2 on each version. - `ImmediateWaitsForReady` reads TRUE on the head. With an unguarded broadcast before the guard (the verifier's probe R4), it reads FALSE. - luacheck: 0 warnings in 186 files. `treefmt --ci`: 0 files changed. ## Repro ```lua -- In a headless Neovim with the plugin on 'runtimepath', CLAUDE_CONFIG_DIR set to a temp dir: require("claudecode").setup({ terminal = { provider = "none" }, track_selection = false, disable_broadcast_debouncing = true }) -- Connect a WebSocket client, complete the handshake, then right away: require("claudecode").send_at_mention(vim.fn.getcwd() .. "/a.txt", nil, nil, "repro") -- Before this fix the client gets the at_mentioned frame within a few ms. After it, it arrives 600 ms after the handshake. ``` `tests/integration/mention_immediate_send_spec.lua` is the runnable form. --- _Generated with [`xum`](https://github.com/coder/xum) • Model: `anthropic:claude-opus-5-5` • Thinking: `xhigh`_

…e wording (coder#568) ## Why coder#554 merged with four follow-ups from its readiness review. This PR makes three of them and records why the fourth stays open. The first push of this PR opened the status file with `exec {status_fd}>...`, a Bash 4.1 redirection. Bash 3.2, the macOS `/bin/bash`, does not know it. A review found that, and a verifier showed the effect: under bash 3.2 the `exec` failed, and because the runner sets an EXIT trap, bash then exited 0 after the first `Running:` line, without running a spec. That was a false pass, and I had declined the review finding wrongly. The runner now uses descriptor 9, a fixed number that bash 3.2 can open. ## What changed 1. **The status file opens before the subshell starts, on descriptor 9.** In coder#554, the file's subshell opened `exit_statuses` in the state folder by path when its file ended. A subshell that ended while the cleanup's `rm` walked that folder made the file during the walk, so `rm` could not remove the folder: `Directory not empty`, and the folder stayed in `TMPDIR`. The runner now opens the file as descriptor 9 before it starts the subshell, and the subshell writes to it. After the cleanup starts, the subshell opens no path. The subshell closes descriptor 9 for Neovim and `tee`, which do not need it. The runner closes descriptor 9 when the subshell has ended, and then reads the file back by path. When it cannot read the file, it uses the exit status of the subshell. I chose a fixed descriptor and not a `BASH_VERSINFO` refusal like `scripts/tla/check-lib.sh` has, because descriptor 9 works on bash 3.2. The runner runs on bash 3.2 and on bash 5. Under `strace -f`, which slows every traced call, I sent SIGTERM to a runner whose file had made 2000 files in the state folder. Main left `exit_statuses` in `TMPDIR` and printed one `rm` error in 5 of 5 runs. This PR left nothing and printed no error in 5 of 5 runs. 2. **A test for the fallback.** A stub Neovim deletes the state folder, so the runner cannot read the statuses back, and a stub `tee` exits 3. The runner must fail the file and print `FAILED: <file>: nvim exit 3`: the fallback cannot tell the stages apart, so it names the status as Neovim's. A variant whose subshell exits with the status of Neovim only is now red. 3. **A check that would have caught the first push, and one for a failed open.** CI runs bash 5, which accepts `{status_fd}`, so no CI run could see it. A test now fails when the runner source holds a `{name}>` redirection. Only this test fails on the first push. A second test makes `mktemp -d` return a folder that does not exist, so descriptor 9 cannot open, and expects a non-zero exit and no "All integration tests passed". On bash 3.2 and on bash 5 a failed `exec 9>` exits 1 under `set -e`. 4. **README and comments say what is true.** - A file with no summary and a status of 124, 137 or 143 gets two lines, the no-summary line and the `TIMEOUT:` line. A file with no summary and another status, for example a crash with 139 or 134, gets only the `FAILED:` line. - An outside SIGTERM that reaches `timeout` gives 124 with uutils. With GNU it gives 143 only when the child dies of the signal, and the child's own status when the child handles it. Real Neovim exits 1, so there is no `TIMEOUT:` line. - "Because nvim did not exit in time" no longer contradicts the hedged `TIMEOUT:` text. - A signal name appears for a status from 129 to 192, except 160 and 161. Bash on Linux names signals 1 to 31 and 34 to 64, so 160 and 161 get no name, and neither does a status from 193 to 255. - The cleanup sentences describe the open status file and the fallback. A runner comment no longer says "as on main". ## Declined: the process group id can be reused The runner kills the process group of `timeout` after a file ends with a non-zero Neovim status. A process group id that another process reused in the few milliseconds after `timeout` ended would get SIGKILL. This is a limit, and I do not change it. - `pid_max` is 4194304 on my host, so a reuse needs a wrap-around of that many ids inside those milliseconds. - Main has had the same kind of risk since coder#445, which added the kill after a timeout status. coder#554 extended the kill to every non-zero exit of Neovim, so the same chance now applies to every failing file. - A fix would have to keep the id from being reused between the end of `timeout` and the kill. ## Tests Changed or new runner spec tests (the names are the columns of the table): - `no_named_fd`: the runner source holds no `{name}>` redirection. - `open_fails`: descriptor 9 cannot open, so the runner stops with a non-zero exit and no pass. - `fd_closed`: the stub Neovim, the stub `tee` and the stub `rm` that the runner calls after it read the statuses look at their own descriptors and find none that points to the status file. - `n1_open_after_cleanup`: SIGTERM while a stub `rm` waits between its files and its folders, and a stub `tee` ends half a second after its input. Nothing is left in `TMPDIR`, and the runner prints no `Directory not empty`. It reproduces the `strace` result without `strace`. - `n5_fallback`: the test of item 2. - `tee_stage`, `both_stages`, `cleanup` (tests of coder#554) and `window` (a test of coder#532): run again because the code they cover changed. Each test ran against each mutant of the runner, with uutils `timeout` 0.10.0 and with GNU `timeout` 9.7. The two builds gave the same table. Red means the test fails. | Variant | no_named_fd | open_fails | fd_closed | n1_open_after_cleanup | n5_fallback | tee_stage | both_stages | cleanup | window | |---|---|---|---|---|---|---|---|---|---| | this PR | green | green | green | green | green | green | green | green | green | | main's runner | green | **red** | green | **red** | green | green | green | green | green | | nvim keeps descriptor 9 | green | green | **red** | green | green | green | green | green | green | | `tee` keeps descriptor 9 | green | green | **red** | green | green | green | green | green | green | | the subshell writes by path | green | green | green | **red** | green | green | green | green | green | | the subshell exits with the status of Neovim only | green | green | green | green | **red** | green | green | green | green | | the statuses are never read | green | green | green | green | green | **red** | **red** | green | green | | the runner keeps descriptor 9 open after it read the statuses | green | green | **red** | green | green | green | green | green | green | | the runner never opens the status file | green | **red** | green | green | green | **red** | **red** | green | green | | first push of this PR (`{status_fd}`, a Bash 4.1 redirection) | **red** | green | green | green | green | green | green | green | green | | a failed open is ignored | green | **red** | green | green | green | green | green | green | green | ### bash 3.2 GNU bash 3.2.57 built from the GNU source (checksum checked), as the interpreter of the runner, with real Neovim 0.12.3 and GNU `timeout`. Two specs: `a_spec.lua` passes and `b_spec.lua` fails. `bash` on `PATH` is 3.2, as on a stock Mac, or 5.3, as with Homebrew bash first on `PATH`. | Runner | Exit | Specs that ran | Output | |---|---|---|---| | main, with either `bash` on `PATH` | 1 | both | `Total Success: 1 Total Failed: 1`, `b_spec.lua` listed as failed | | first push of this PR, with either `bash` on `PATH` | **0** | none (it printed `Running: a_spec.lua`, then stopped) | `line 72: exec: {status_fd}: not found`, no totals | | this PR, with either `bash` on `PATH` | 1 | both | the same output as main, nothing left in `TMPDIR` | ### Real Neovim, `strace` race Real Neovim 0.12.3. The spec makes 2000 files in the state folder and then waits. A stub `tee` sleeps 0.05 s after its input ends, and `strace -f` traces `openat`, `unlinkat`, `rmdir`, `kill` and `execve`. SIGTERM goes to the runner. The probe is a copy of the verifier's, with paths changed. | Runner | Items left in `TMPDIR` one second later | `rm` error lines | |---|---|---| | main, 5 runs | the state folder with `exit_statuses`, in 5 of 5 | 1 in each of the 5 | | this PR, 5 runs | none, in 5 of 5 | 0 in each of the 5 | ### Whole runner, main versus this PR Stdin is `/dev/null` and `TMPDIR` is unset. I ran this on the tree before the last merge of main. The rows are identical: 135 per-file rows each (33 files with a `Running` line and three count lines, plus three totals), no difference, and no `FAILED`, `TIMEOUT`, E886, `Killed` or `No such file` line. | Neovim | main | this PR | |---|---|---| | 0.12.3 | 631 passed, 0 failed, 0 errors | 631 passed, 0 failed, 0 errors | | v0.8.0 | 613 passed, 0 failed, 0 errors | 613 passed, 0 failed, 0 errors | With `TMPDIR` set to a folder with a space and a `%` (what the CI guard uses), this PR on 0.12.3 also gives 631 passed, 0 failed, 0 errors. ### Gate `mise run clean && mise run all` on the same tree: the formatter changed no file, luacheck reports 0 warnings and 0 errors in 191 files, and busted reports 1837 successes, 0 failures, 0 errors, 31 pending. `shellcheck` on `scripts/*.sh` and `actionlint` pass. A merge of main came after that run: 9 commits, none in `scripts/` or in the runner spec. They add three integration specs (`diff_legacy_cleanup`, `mention_immediate_send`, `selection_demotion_in_terminal`) and delete `diff_accept_trailing_newlines`, so 33 files become 35. An independent rerun of the whole runner on the exact head gave 669 passed on Neovim 0.12.3 and 651 on v0.8.0, 0 failed, 143 rows equal to main's. On the merged tree I ran the static checks (`mise run check`: exit 0, luacheck 0 warnings and 0 errors in 193 files) and the whole runner spec (73 tests, all passed). ## Not covered - Declined from the round-2 review: the source check looks only for `{name}>` and `{name}<` redirections, not every bash 4 construct (the whole runner spec passes 73 of 73 under bash 3.2, so none is in the runner today). The spec Neovims no longer inherit a descriptor 9 that the caller passed in (neither version writes to it, and results are equal). On a host without `/proc`, `fd_closed` checks nothing. A follow-up PR in this run, with the fix for the flaky `dies by SIGINT on Ctrl-C` test, makes it report pending there instead of passing. --- _Generated with [`xum`](https://github.com/coder/xum) • Model: `anthropic:claude-sonnet-5-5` • Thinking: `xhigh`_

ThomasK33 marked this pull request as ready for review November 12, 2026 07:27

ThomasK33 added 6 commits November 12, 2026 09:53

chore: merge origin/main into test/runner-failure-reasons

c651930

_Generated with xum • Model: anthropic:claude-sonnet-5-5 • Thinking: xhigh_

chore: merge origin/main into test/runner-failure-reasons

551e72d

_Generated with xum • Model: anthropic:claude-sonnet-5-5 • Thinking: xhigh_

chore: merge origin/main into test/runner-failure-reasons

5cdf9f7

_Generated with xum • Model: anthropic:claude-sonnet-5-5 • Thinking: xhigh_

chore: merge origin/main into test/runner-failure-reasons

79cf904

_Generated with xum • Model: anthropic:claude-sonnet-5-5 • Thinking: xhigh_

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

Merged via the queue into main with commit 26d4c3c Nov 12, 2026
7 checks passed

ThomasK33 deleted the test/runner-failure-reasons branch November 12, 2026 15:45

This was referenced Nov 12, 2026

fix(queue): keep the readiness wait for a send with broadcast debouncing off #541

Merged

fix(runner): write the exit statuses to an open descriptor and fix the wording #568

Merged

Navigation Menu

Repository navigation

fix(runner): name why a file failed when nvim exits non-zero without a reason - #554

fix(runner): name why a file failed when nvim exits non-zero without a reason#554
ThomasK33 merged 7 commits into
mainfrom
test/runner-failure-reasons

ThomasK33 commented Nov 12, 2026 •

edited

Loading

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!

chatgpt-codex-connector Bot commented Nov 12, 2026

Uh oh!

chatgpt-codex-connector Bot commented Nov 12, 2026

Uh oh!

ThomasK33 commented Nov 12, 2026

Uh oh!

ThomasK33 commented Nov 12, 2026

Uh oh!

chatgpt-codex-connector Bot commented Nov 12, 2026

Uh oh!

chatgpt-codex-connector Bot commented Nov 12, 2026

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

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

Tests

Real Neovim

Whole runner, main versus this PR

Gate

Follow-up

Not in this PR

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

Uh oh!

chatgpt-codex-connector Bot commented Nov 12, 2026

🛡️ Codex Security Review

Uh oh!

ThomasK33 commented Nov 12, 2026

Uh oh!

ThomasK33 commented Nov 12, 2026

Uh oh!

chatgpt-codex-connector Bot commented Nov 12, 2026

Uh oh!

chatgpt-codex-connector Bot commented Nov 12, 2026

🛡️ Codex Security Review

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