Repository navigation
test: return from termopen only after the job's child ran exec - #593
Merged
Merged
Conversation
Neovim forks a pty job's child with its own libuv signal handlers and resets them in init_child(), after login_tty() made the child the session leader. A hangup in that gap (a jobstop, a buffer wipe or the quit that ends a file) runs the inherited handler in the child. It writes into the signal pipe that the child shares with Neovim, so Neovim reads a SIGHUP of its own and exits with "Caught deadly signal 'SIGHUP'". diff_window_sizes_spec died that way once under host load. tests/minimal_init.lua now wraps vim.fn.termopen so that it returns after the child's /proc/<pid>/exe no longer names Neovim. A new integration spec checks that, and proof-pty-child-hangup.sh reproduces the race with an LD_PRELOAD shim that holds the child in the gap. _Generated with xum • Model: anthropic:claude-sonnet-5-5 • Thinking: xhigh_
ThomasK33
marked this pull request as ready for review
November 12, 2026 23:48
Member
Author
|
@codex review |
Member
Author
|
@codex security review |
Codex Review Summary: ✅ CompletedThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
🛡️ Codex Security ReviewSecurity review completed. No security issues were found in this pull request. Reviewed commit: Only the user who started this review can view the report in Codex. ℹ️ About Codex security reviews in GitHubThis is an experimental Codex feature. Security reviews are triggered when:
Once complete, Codex will leave suggestions, or a comment if no findings are found. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4c02fa0e2f
ℹ️ 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".
…in the spec Review of the first head found two gaps. A job that runs Neovim itself keeps the same /proc/<pid>/exe after exec, so the wrapper waited the full 10 s and failed. It now also compares /proc/<pid>/cmdline with Neovim's own. The spec could pass without the wrapper when the child won the race to exec before the spec looked. It now gives each job an environment of 20000 entries. The child copies it before exec, which keeps the child in Neovim's code for milliseconds. A new case covers a job that runs Neovim itself. _Generated with xum • Model: anthropic:claude-sonnet-5-5 • Thinking: xhigh_
…pt clean Review of 4c02fa0 found that no spec step could tell a missing wrapper from a present one when a TermOpen autocmd takes a few milliseconds: the child runs exec first either way, and a child's gap after the fork is 2 to 6 ms even with a large environment. So the wait now lives in tests/helpers/termopen_wait.lua and the spec drives it with stand-ins for /proc (a child that runs Neovim's code for N reads, a child that is Neovim with another command line, a child that never runs exec, a job with no pid). The spec also checks that vim.fn.termopen is the wrapper, and keeps four checks with real jobs: a plain job, a TermOpen jobstop, a TermOpen bwipeout and a job that runs Neovim. The proof script saves the PID and start time of the job's child and kills only that process, so the window case leaves no `sleep 60`. Its comments say that setsid() is the first call of init_child(). DEVELOPMENT.md says what the wrapper does not cover, and its paragraph no longer sits between the Claude stub paragraph and the strace recipe. _Generated with xum • Model: anthropic:claude-sonnet-5-5 • Thinking: xhigh_
Member
Author
|
@codex review |
Member
Author
|
@codex security review |
|
Codex Review: Didn't find any major issues. Chef's kiss. Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
🛡️ Codex Security ReviewSecurity review completed. No security issues were found in this pull request. Reviewed commit: Only the user who started this review can view the report in Codex. ℹ️ About Codex security reviews in GitHubThis is an experimental Codex feature. Security reviews are triggered when:
Once complete, Codex will leave suggestions, or a comment if no findings are found. |
This was referenced Nov 13, 2026
hareki
pushed a commit
to hareki/claudecode.nvim
that referenced
this pull request
Nov 14, 2026
…es (coder#597) ## Why The verifier of coder#593 passed it and left four minor findings. This PR closes them. All four sit in the test wrapper that makes `termopen` return after the job's child ran exec (`tests/helpers/termopen_wait.lua`), its spec, and the notes about them. ## What changed 1. **Notes (N1).** A job with the test Neovim's exact command line keeps its executable and command line across exec, so `termopen` blocks until that job exits, and raises when the job lives 10 s. The verifier saw a 2 s job block for 2032 ms, and a 15 s job raise 13 of 16 times. A poll can also land inside exec, where the command line reads empty, and return before exec has ended (3 of 16). The helper comment and `DEVELOPMENT.md` say so. 2. **Pointers (D1).** `SKILL.md` and the proof script named `tests/minimal_init.lua` as the wrapper's home. It lives in `tests/helpers/termopen_wait.lua`, and `minimal_init.lua` installs it. 3. **Defaults (K2b).** With a 50 ms `TermOpen` autocmd, a wrapper that read nothing from `/proc` (the verifier's m09) or whose default `jobpid` raised (m10) passed the spec 5 of 5, because the child ran exec before `termopen` returned. Two new cases do not depend on timing. One builds the wrapper with stand-ins for the clock and the pid only, and gives it Neovim's own pid, so the wait must end at the deadline of the fake clock. That reads `/proc` and the default `readlink` for real. The other replaces `vim.fn.jobpid`, `vim.loop.hrtime` and `vim.loop.sleep`, and builds the wrapper with no stand-ins, so the wrapper must take its defaults from them. 4. **Clock (M11).** The same second case pins the default `hrtime` binding, which gives the 10 s deadline of the installed wrapper. The verifier's m11 (a clock that always reads 0) passed the spec in 3 of 3 runs before (red 0 of 3). No file under `lua/` or `plugin/` changes. ## Evidence Builds: the official `v0.8.0` release binary (`NVIM v0.8.0-1210-gd367ed9b2`, as in CI) and Neovim 0.12.3. Each mutant of `termopen_wait.lua` ran 3 times per build against the spec. "Slow" means a 50 ms `TermOpen` autocmd added to `tests/minimal_init.lua`. | Mutant | main's spec, plain | main's spec, slow | this spec, plain | this spec, slow | |---|---|---|---|---| | control (no change) | 0 of 3 red | 0 of 3 red | 0 of 3 red | 0 of 3 red | | m09: `read_proc` returns nil | 3 of 3 red | 0 of 3 red | 3 of 3 red | 3 of 3 red | | m10: default `jobpid` raises | 3 of 3 red | 0 of 3 red | 3 of 3 red | 3 of 3 red | | m11: default `hrtime` always 0 | 0 of 3 red | 0 of 3 red | 3 of 3 red | 3 of 3 red | | default `sleep` does nothing | not run | not run | 3 of 3 red | 3 of 3 red | | default `readlink` returns nil | not run | not run | 3 of 3 red | 3 of 3 red | | default `read` returns nil | not run | not run | 3 of 3 red | 3 of 3 red | The table has the same counts on 0.8.0 and 0.12.3. The 12 mutants of coder#593 (wrapper not installed, no wait, exe test inverted, a single 1 ms poll, no deadline assertion, a 1 ms deadline, no `pcall` around `jobpid`, and the others) are still red 3 of 3 on both builds against this spec. - Gate: `MISE_LOCKFILE=false mise run all` with GNU timeout first on PATH exits 0 with 1904 successes, 0 failures, 0 errors and 36 pending, and luacheck reports 0 warnings in 200 files. - `mise run format-check`: 444 files, 0 changed. - Integration runner: 852 successes, 0 failures, 0 errors on 0.8.0, and 856, 0, 0 on 0.12.3. - `pty_child_exec_spec.lua`, 20 runs per build: 0 failures on 0.8.0 and 0.12.3. - `check.sh --hash-only` is fresh for the four verify skills. ## Not covered The case that reads `/proc` uses Neovim's own pid, so it needs a Linux `/proc` like the rest of the spec (the spec is pending without it). The default `sleep` is pinned only through the case that replaces `vim.loop.sleep`. Nothing here changes how the wrapper behaves. --- _Generated with [`xum`](https://github.com/coder/xum) • Model: `anthropic:claude-sonnet-5-5` • Thinking: `xhigh`_
hareki
pushed a commit
to hareki/claudecode.nvim
that referenced
this pull request
Nov 14, 2026
…the owned stop (coder#602) ## Why This is a follow-up to coder#593. It fixes the Snacks close and the stop in `owned.lua`, the two close paths that coder#593 left open. A job that was just started can still be a fork of Neovim until its child has run `exec`. A `jobstop` on it then ends Neovim with `Caught deadly signal 'SIGHUP'`, or the buffer delete after it hangs. coder#593 documents the race, and coder#544 made the native close wait for the exec of a young job. The Snacks close and the stop in `owned.lua` still called `jobstop` at once, so a close within milliseconds of the start could kill Neovim. ## Cause `pty_process_spawn` forks the child, and `init_child()` runs `setsid()` and resets the signal handlers only after that. Until `exec` the child has Neovim's libuv handlers. When the pty master closes in that gap (a `jobstop`), the kernel sends SIGHUP to the child, libuv's handler in the child writes into the signal pipe that it shares with Neovim, and Neovim reads a SIGHUP of its own and exits. With the child held in the gap (see Evidence), a close right after the start killed Neovim in every run for the Snacks provider and for the stop in `owned.lua`, on 0.8.0 and 0.12.3. The native provider survived in every run. ## Fix The wait moves out of `native.lua` into `owned.wait_for_exec(jobid)`, with `owned.is_young(started_at)` for the 10 s rule. The code is the same: it reads the names of Neovim and of the child from `/proc/<pid>/comm` (or `nvim_get_proc` where `/proc` is missing) and waits up to 1 s until the child has another name. - `native.close` calls it for a job younger than 10 s, as before. - `snacks.M.open` records `term._cc.job_started_at` where it already patches the instance. `snacks.close` reads it and calls the wait for a job younger than 10 s. A terminal that a reload adopted has no start time and is not waited for, as in native. Without the age rule, every Snacks close on macOS would pay about 200 ms for the two `ps` calls of the `nvim_get_proc` fallback. - `owned.lua` `stop_buffer` has no start time, so it calls it for every job. That path runs only after a reload or a provider switch, and the wait ends at the first look for a job that has run `exec`. The wait runs scheduled callbacks while the job still runs. Two Codex findings on `ea0fbfd6` were about that, and `12f1d45f` fixes both: - `snacks.close` now clears the owned mark before the wait for every terminal, not only for an adopted one. An `open()` among the callbacks can no longer adopt the terminal that is closing by its mark. - `stop_buffer` reads the channel before the wait and checks `nvim_buf_is_valid(buf)` before `jobstop`. A callback that deleted the buffer during the wait no longer makes `jobstop` raise `Invalid buffer id`. `lua/claudecode/terminal/README.md` and the `verify-terminal-lifecycle` skill describe the three places. The skill changed as follows: - The fact `OwnedStopOtherStops` reads `M.wait_for_exec(channel)` and the buffer check before `jobstop`. - The new fact `SnacksCloseClearsMark` reads the mark clear before the wait. The model has a switch of the same name in `CloseBegin`. The model counts a closing terminal as ended, so the fact breaks a rule only together with `OwnedLiveSkipsEndedJob`. `check.sh --mutants` has a row for the pair. - `OwnedLiveSkipsEndedJob` alone now breaks `OpenShowsLiveClaude`, not `CloseNeverReuses`, because the model clears the mark in every Snacks close. - The model still passes with 279492 distinct states. ## CI on `ea0fbfd6` and the fixes Two checks failed on `ea0fbfd6`. 1. The integration spec compiled its shim into a TMPDIR folder. CI's second pass runs with `TMPDIR="$RUNNER_TEMP/odd tmp 50%"`. `LD_PRELOAD` splits its value at a space or a colon, so the shim was ignored, the child was never held, and the probe timed out. `f910015b` puts the shim into `mktemp -d /tmp/claudecode.XXXXXX` when the TMPDIR path holds a space or a colon. 2. `verify-diff-lifecycle` also hashes `terminal/{native,owned,snacks}.lua`, so its `covered.lock` was stale. `3a1961a0` re-records the locks of `verify-terminal-lifecycle` and `verify-diff-lifecycle`. ## Tests - `tests/integration/terminal_close_young_job_spec.lua` runs one Neovim per case with `-u NONE` (so `minimal_init.lua` does not wait for the exec and hide the race) and `tests/helpers/hold_pty_child.c` preloaded. The shim holds the child inside `init_child()` and creates a file when it does. The probe (`tests/helpers/close_young_job_probe.lua`) starts a Claude terminal job, waits for that file, and closes the job. Cases: a plain `jobstop` as the control (it must end Neovim with SIGHUP, so the other cases can fail), the native close, the Snacks close (with a stand-in for Snacks) and the `owned.lua` stop. The spec is pending without Linux, a C compiler and `timeout`. It puts the shim into a `/tmp` folder when TMPDIR holds a space or a colon, and `tests/helpers/ld_preload_path.lua` makes that choice. - `tests/unit/ld_preload_path_spec.lua` has three cases: a plain path, a path with a space and a path with a colon. Before it, a spec pattern without the colon half passed in every CI configuration. - `tests/unit/terminal/owned_wait_for_exec_spec.lua` covers the wait (names from `/proc`, from `nvim_get_proc`, the 1 s limit, a job with no pid, unreadable names), `is_young`, the order of the stop in `owned.lua` (wait, then `jobstop`, then delete) and "leaves a buffer alone that a callback deleted during the wait". - `tests/unit/terminal/snacks_close_spec.lua` covers the Snacks close: it waits before `jobstop` for a job that just started, not for a job older than 10 s, and not for a job that exited. The case "does not let an open during the wait adopt the terminal that is closing" covers the mark clear. - Five Snacks unit specs gain `vim.loop.hrtime` in their vim mock, and one gains `vim.fn.getpid`, because the provider now reads a clock and the wait reads the pid. - `proof-pty-child-hangup.sh` compiles the same shim instead of a copy of it. Under a TMPDIR with a percent sign it failed all three cases, on `main` too. With only a space or only a colon it failed one of three, and its wrapper case passed only because the shim was not loaded. Now `d6019d13` puts its work folder into `/tmp` for such a path. - The header of `hold_pty_child.c` now says that the shim holds every fork of Neovim that calls `signal(SIGCHLD, SIG_DFL)`, including the `ps` children that `nvim_get_proc` starts, not only the pty child. ## Evidence Builds: the official `v0.8.0` release binary (`NVIM v0.8.0-1210-gd367ed9b2`, as in CI) and Neovim 0.12.3, scratch HOME and XDG dirs, on a shared host. | Run | main plus the new tests | this branch | |---|---|---| | integration spec, 10 runs per build | 10 of 10 fail on both builds (the Snacks and `owned.lua` cases fail, the control and native pass) | 0 of 10 fail on both builds | | integration spec, 20 runs per build, plain TMPDIR | not run | 0 of 20 fail on 0.8.0 and on 0.12.3 | | integration spec, 20 runs per build, `TMPDIR="odd tmp 50%"` | the spec of `ea0fbfd6` times out in this TMPDIR, as in CI | 0 of 20 fail on 0.8.0 and on 0.12.3 | | the unit specs of `tests/unit/terminal` plus `native_terminal_toggle_spec`, 20 runs | not run | 0 of 20 fail (374 successes each) | Mutants of the Lua files, one run each, on `89e74e31`. The merge of main changed no terminal file and no terminal spec, so I did not rerun them on the merge commit. "Unit" counts the failing tests among 374 in `tests/unit/terminal` plus `native_terminal_toggle_spec`. "Integration" is the new spec on 0.8.0 and 0.12.3, with the same result on both. "Fact" is the `verify-terminal-lifecycle` fact that turns FALSE in a facts-only run of `check.sh`. | Mutant | Unit | Integration | Fact | |---|---|---|---| | Snacks close does not wait | 2 failures | red | `SnacksCloseClearsMark` | | Snacks close always waits (no age rule) | 1 failure | green (the wait is harmless but slow) | none | | Snacks close never waits (age rule always false) | 2 failures | red | none | | Snacks does not record the start time | 2 failures | red | none | | Snacks clears the mark only for an adopted terminal (the form on `ea0fbfd6`) | 1 error (the open adopts the closing terminal) | green | `SnacksCloseClearsMark` | | Snacks clears the mark after the wait | 1 error (same case) | green | `SnacksCloseClearsMark` | | `stop_buffer` does not wait | 2 failures | red | `OwnedStopOtherStops` | | `stop_buffer` has no `nvim_buf_is_valid(buf)` check before `jobstop` | 1 failure | green | `OwnedStopOtherStops` | | `stop_buffer` re-reads the channel with no check (the form on `ea0fbfd6`) | 1 failure | green | `OwnedStopOtherStops` | | native close does not wait | 3 failures | red | none | | native close never counts a job as young | 3 failures | red | none | | `wait_for_exec` does not call `vim.wait` | 6 failures | red | none | | `wait_for_exec` stops while the names are equal | 4 failures | red | none | | `wait_for_exec` waits 0 ms | 4 failures | red | none | | `wait_for_exec` has no guard for an unreadable name of Neovim | 4 failures | green | none | | `wait_for_exec` calls `jobpid` without `pcall` (no guard for a job without a pid) | 1 failure and 2 errors | green | none | | `is_young` uses `<=` | 2 failures | green | none | | `is_young` reads a `nil` start time as young (no `nil` guard) | 1 failure | green | none | The control mutant (no change) is green in every column, with 374 successes. I ran the facts-only `check.sh` on every row, and "none" means that every fact stayed TRUE. A mutant of `tests/helpers/ld_preload_path.lua` that drops the colon, and one that drops the space, each fail one case of `ld_preload_path_spec.lua`. The model has its own mutants. `check.sh --mutants` removes each guard of the model in turn, and it ends with "every guard is caught" on the merge commit `191a3e24`. The pair `SnacksCloseClearsMark` with `OwnedLiveSkipsEndedJob` is caught by `CloseNeverReuses` and `OpenShowsLiveClaude`. - Gate: `MISE_LOCKFILE=false mise run clean && mise run all` with GNU timeout first on PATH exits 0 with 1925 successes, 0 failures, 0 errors and 38 pending, and luacheck reports 0 warnings in 206 files. The extra pending case is the new integration spec of coder#536, which needs a real Neovim. - `mise run format-check`: 464 files, 0 changed. - Integration runner, with a plain TMPDIR and with `TMPDIR="odd tmp 50%"`: 865 successes, 0 failures, 0 errors on 0.8.0, and 869, 0, 0 on 0.12.3. - `proof-pty-child-hangup.sh` passes 3 of 3 cases on 0.8.0 and on 0.12.3, with a plain TMPDIR, with `TMPDIR="odd tmp 50%"` and (on 0.12.3) with a TMPDIR that holds a colon. On `d6019d13`'s parent, with the odd TMPDIR, it failed 3 of 3 on both builds. - The five `check.sh` scripts (`verify-diff-lifecycle`, `verify-mention-queue`, `verify-selection-tracking`, `verify-terminal-lifecycle` and `verify-ws-connection`) all print PASS. The branch merges `origin/main` (coder#597, coder#594, coder#474, coder#596 and coder#536). coder#536 changed `diff.lua`, its README, `verify-diff-lifecycle` (SKILL.md, `check.sh` and `covered.lock`) and added `diff_answer_every_call_spec.lua`. Git merged the two edits of `verify-diff-lifecycle/covered.lock` with no conflict, and `check.sh` of all five skills prints PASS on the merge result, so no lock needed a new record after `3a1961a0`. - On the merge commit `191a3e24`, the specs this PR adds or changes pass 20 of 20 runs in a plain TMPDIR and in `TMPDIR="odd tmp 50%"` (24 successes for `owned_wait_for_exec_spec`, `snacks_close_spec` and `ld_preload_path_spec`, and 374 for the terminal unit specs plus `native_terminal_toggle_spec`). The integration spec of this PR passes 20 of 20 runs per build in both TMPDIRs. `diff_answer_every_call_spec` from coder#536 passes 10 of 10 runs per build in both TMPDIRs (9 successes each run). ## Not covered - The Snacks case uses a stand-in for Snacks, because CI installs no `snacks.nvim`. The provider code under test is the real one, and the stand-in starts a real pty job with `termopen`. - The wait reads `/proc` or `ps`. Where neither gives a name, the close goes on to `jobstop` and the race stays open, as in native before this PR. A `ps` that hangs would hang the close for as long as it hangs. I did not reproduce that. - I tested the `ps` fallback only on Linux, with `/proc` hidden from Lua. I did not run it on macOS or BSD. - A `jobstop` that other code issues, for example `:bwipeout` of the terminal buffer by the user right after the start, is not covered. - Another PR that edits `native.lua`, `snacks.lua`, `owned.lua` or a `covered.lock` must be merged one at a time with this one, and re-record the lock after its merge. --- _Generated with [`xum`](https://github.com/coder/xum) • Model: `anthropic:claude-sonnet-5-5` • Thinking: `xhigh`_
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why
tests/integration/diff_window_sizes_spec.luadied once under host load withNvim: Caught deadly signal 'SIGHUP'(seen on a #573 head, green on the rerun, and main has the same exposure). The runner then reported "no plenary summary (nvim exit 1)". A test must not kill the Neovim that runs it, and a retry hides the cause.Cause
The cause is a race inside Neovim and libuv. It is not a signal mask or disposition that stays wrong after exec.
pty_process_spawncallsforkpty(). In the child,login_tty()gives it a new session on the pty.init_child()then runssetsid(), resets SIGCHLD, SIGHUP, SIGINT, SIGQUIT, SIGTERM and SIGALRM to their defaults, changes directory, builds the environment and callsexecvp. Until exec the child is a copy of Neovim and has Neovim's libuv signal handlers.jobstop, a buffer wipe), the kernel sends SIGHUP to the child. libuv's inherited handler writes a message into the signal pipe that the child shares with Neovim. Neovim reads it as its own SIGHUP and exits.strace -fon a held child shows it. The child getsSIGHUP {si_code=SI_KERNEL}and runs the handler, which writes to fd 11 on Neovim 0.8.0 (fd 8 on 0.12.3) and then one byte to a second descriptor. The parent then printsCaught deadly signal 'SIGHUP'. The 0.8.0 and 0.12.3 sources have the same order ininit_child().The spec reaches it because several tests start a terminal job (
terminal.open()ortermopen({ "sleep", "60" })), end within milliseconds, andafter_eachwipes the buffer. A child that the host scheduler delays inside the gap is enough. #336 had already measured the same race on a plaintermopenfollowed byjobstop.Fix
tests/helpers/termopen_wait.luawrapsvim.fn.termopen, andtests/minimal_init.luainstalls it. The wrapper returns only after/proc/<pid>/exeor/proc/<pid>/cmdlineof the child differs from Neovim's own, so the child has runexecand holds none of Neovim's handlers. It compares the command line too, because a job that runs Neovim itself keeps the same executable. The wait ends with an assertion after 10 s. Every integration spec and every Neovim that loadsminimal_init.luagets it. The wrapper reads/proc, so it works on Linux only (CI runs on Linux). On another system it does nothing.tests/integration/pty_child_exec_spec.luahas 10 cases. Five drive the wrapper with stand-ins for/proc(a child that runs Neovim's code for N reads, a child that has run exec, a job that runs Neovim with another command line, a child that never runs exec, a job with no pid). One checks thatvim.fn.termopenis the wrapper. Four use real jobs: a plain job, a job that aTermOpenautocmd stops, a job whose buffer aTermOpenautocmd wipes, and a job that runs Neovim..agents/skills/verify-claudecode-nvim/scripts/proof-pty-child-hangup.shreproduces the race with anLD_PRELOADshim that holds the child inside thesignal(SIGCHLD, SIG_DFL)call that followssetsid(), and it checks the wrapper against it. It saves the PID and start time of each case's child and kills only that process.SKILL.mdandfeatures/terminal.mdpoint to it.DEVELOPMENT.mdhas a paragraph about the wrapper and what it does not cover.No plugin code changes. The merge of
origin/mainbrings in main's owndiff.luachange.Evidence
All runs use the official
v0.8.0release binary (NVIM v0.8.0-1210-gd367ed9b2, as in CI) or Neovim 0.12.3, with a scratch HOME and XDG dirs, on a shared host with a load average of 20 to 100. The injected rates depend on the host load.diff_window_sizes_specwith the child held 100 ms in the gap, 0.8.0diff_window_sizes_spec, no shimtermopen, spin 50 to 1200 µs,jobstop, 40 runs per delay (0.8.0)minimal_init.luaproof-pty-child-hangup.shwrappercase fails on 0.8.0 and 0.12.3The wrapper spec against 12 mutants of the wrapper (wrapper not installed, no wait, never waits because
/proc/self/exeis unreadable, wait while the executable differs, wait while the command line differs, executable check only, a single 1 ms poll, no deadline assertion, a 1 ms deadline, nopcallaroundjobpid, no sleep, polling Neovim's own pid) and a control: the control is red 0 of 3 and every mutant is red 3 of 3, on 0.8.0 and on 0.12.3. The first version of the spec caught a single 1 ms poll only 2 of 6 times on 0.8.0 and 4 of 6 on 0.12.3.Two cases that the first version got wrong now hold (the nested-Neovim case was wrong in the wrapper, the slow-parent case in the spec):
sleep 2) or failed after 10 s (forsleep 15). Nowtermopenreturns after 7 to 14 ms.TermOpenautocmd that takes 50 ms. With main'sminimal_init.luaplus that autocmd, the old spec was red 0 of 5 and the new spec is red 5 of 5 on both builds. With this branch'sminimal_init.luaplus the same autocmd, the new spec is red 0 of 5. The child's gap after the fork is only 2 to 6 ms, even with an environment of 20000 to 200000 entries, so no setting of the spec can keep the child in Neovim's code through such an autocmd. That is why the wait is pinned with stand-ins.The proof script left one
sleep 60per run before this fix (3 of 3 runs), and leaves none now.Not proven or not covered
diff_window_sizes_specon main did not die in 200 tries on this host. The original death is explained by the mechanism and by the injected runs, but I did not reproduce that exact occurrence naturally.vim.fn.termopenonly. A job that a spec starts withjobstartandterm = trueis not covered. No spec does that today.TermOpenautocmd issues insidetermopenruns before the wait, so it is outside the wrapper. For a job that runs Neovim with the same command line as the test Neovim,termopenblocks until that job exits and raises if it lives 10 s. A poll that lands insideexeccan return early. No spec starts such a job.:ClaudeCodeCloseof the native terminal waits for the exec of a young job (fix(terminal): keep a native start single when an autocmd interrupts it #544). A directjobstopor:bwipeoutof the terminal buffer right aftertermopen, and the Snacks close, do not wait. This PR changes no plugin code.forkpty()or reset the handlers before the session becomes the pty's). I did not report it upstream.Generated with
xum• Model:anthropic:claude-sonnet-5-5• Thinking:xhigh