Repository navigation
fix(terminal): wait for the child's exec before the snacks close and the owned stop - #602
Conversation
…the owned stop 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 SIGHUP, or the buffer delete after it hangs. The native close has waited for the exec of a young job since #544. The snacks close and the stop in owned.lua called jobstop at once. The wait moves into owned.wait_for_exec, with owned.is_young for the 10 s rule, and native.close uses them with no change. snacks.close records the start time on the instance (_cc.job_started_at) and waits for a job younger than 10 s, so a terminal that a reload adopted is not waited for, as in native. owned.lua stop_buffer has no start time, so it waits for every job: that path only runs after a reload or a provider switch, and the wait ends at the first look for a job that has run exec. The unit mocks of the snacks specs gain vim.loop.hrtime, and one gains vim.fn.getpid, because the provider reads a clock and the wait reads the pid. _Generated with xum • Model: anthropic:claude-sonnet-5-5 • Thinking: xhigh_
…d before exec tests/helpers/hold_pty_child.c is an LD_PRELOAD shim that holds the child of a pty job inside Neovim's init_child() and creates a file when it does. The new integration spec runs a Neovim of its own (-u NONE, so that minimal_init.lua does not wait for the exec and hide the race) per case, starts a Claude terminal job, waits until the child is held and closes it. A plain jobstop is the control and ends Neovim with SIGHUP in every run. The native close, the snacks close (with a stand-in for Snacks) and the owned.lua stop keep Neovim alive. On main the snacks and owned cases fail in 10 of 10 runs per build. proof-pty-child-hangup.sh now compiles the same shim instead of a copy of it. _Generated with xum • Model: anthropic:claude-sonnet-5-5 • Thinking: xhigh_
The wait for the exec of a job's child moved into owned.wait_for_exec, and snacks.lua and owned.lua gained a call to it before jobstop. The model has no processes and no fact reads the wait, so check.sh printed STALE with the model passing (main: pass, 279492 distinct states) and every fact TRUE. The SKILL.md bullet about the wait names the new places. Recorded with check.sh --record, and check.sh then reports PASS. _Generated with xum • Model: anthropic:claude-sonnet-5-5 • Thinking: xhigh_
|
@codex review |
|
@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. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ea0fbfd6c6
ℹ️ 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".
… the exec wait Review of the first head found two gaps in the wait that runs before jobstop. The wait runs scheduled callbacks while the job still runs. snacks.close cleared the mark of the terminal that it closes only for an adopted one. An open() among the callbacks found a started terminal by its mark and adopted the one that was closing, and the close then wiped it. The mark now goes first for every terminal, as in native.close. owned.lua stop_buffer read vim.bo[buf].channel again after the wait, and a callback could have deleted the buffer. It now reads the channel first and stops the job only when the buffer is still valid. The unit specs gain one case for each. The OwnedStopOtherStops fact of verify-terminal-lifecycle reads the new order (mark, wait for the exec, jobstop, wait for TermClose, delete), and the SKILL.md notes say so. _Generated with xum • Model: anthropic:claude-sonnet-5-5 • Thinking: xhigh_
CI runs the integration suite a second time under TMPDIR="odd tmp 50%". The spec compiled the LD_PRELOAD shim into a folder of TMPDIR, and ld.so splits LD_PRELOAD at spaces and colons, so it ignored the shim, the child was never held, and the probe stopped after 5 s (the log shows "object '...odd' from LD_PRELOAD cannot be preloaded"). The spec now compiles the shim into a mktemp folder under /tmp when the TMPDIR-based path holds a space or a colon. _Generated with xum • Model: anthropic:claude-sonnet-5-5 • Thinking: xhigh_
… and verify-diff-lifecycle CI showed that verify-diff-lifecycle hashes lua/claudecode/terminal/native.lua, owned.lua and snacks.lua too, and went STALE with only those three file lines changed. The change cannot reach diff state, an openDiff answer, a disconnect or a diff command: it adds an exec wait before jobstop in the snacks close and the owned stop, and moves native's wait into owned.lua. check.sh of that skill reports the model passing, so it is recorded as the SKILL.md allows. verify-terminal-lifecycle: the OwnedStopOtherStops fact reads the new order, check.sh --mutants reports that every guard is caught, and the record follows the three file lines and the block hashes of snacks_close and owned_stop_buffer. _Generated with xum • Model: anthropic:claude-sonnet-5-5 • Thinking: xhigh_
The longer OwnedStopOtherStops row made treefmt re-pad the table of the snacks provider. format-check reports 0 changed files after this commit. _Generated with xum • Model: anthropic:claude-sonnet-5-5 • Thinking: xhigh_
…k and the buffer check The model counts a closing terminal as ended (JobEnded), so removing the mark clear that snacks.close now runs before its exec wait left every fact TRUE, and only a unit test caught it. A new fact SnacksCloseClearsMark reads the order terminal = nil, the mark clear, owned.wait_for_exec and jobstop in M.close. The model clears the mark in the snacks close as it does in native, and the rule CloseNeverReuses breaks when this fact and OwnedLiveSkipsEndedJob are both removed, as for NativeCloseClearsMark. The model has no wait before jobstop, so the fact alone breaks no rule. The OwnedStopOtherStops fact now also reads the buffer check that guards jobstop in stop_buffer. SKILL.md names the check, adds the row for the new fact, and says which facts read the wait (it said that none does). _Generated with xum • Model: anthropic:claude-sonnet-5-5 • Thinking:
…space The spec keeps the LD_PRELOAD shim out of a folder whose path holds a space or a colon. CI runs a TMPDIR with a space, so dropping the colon from the pattern kept every CI run green. The test is now a unit spec of tests/helpers/ld_preload_path.lua, which the integration spec calls: a path with a space, a path with a colon and a plain path. Dropping either half turns that spec red (1 failure each). _Generated with xum • Model: anthropic:claude-sonnet-5-5 • Thinking:
…SIGCHLD The header said that the shim holds the child of a pty job. It holds every forked child that calls signal(SIGCHLD, SIG_DFL) first, including libuv's uv_spawn children such as each ps that nvim_get_proc starts where /proc is missing. A test that times that path reads the hold as well. _Generated with xum • Model: anthropic:claude-sonnet-5-5 • Thinking:
… colon or a percent sign The script put its shim and probe into mktemp -d. LD_PRELOAD splits the shim path at a space or a colon, and luafile ends the name at a space and expands the percent sign. Under TMPDIR="odd tmp 50%" all three cases failed, on main too. The work folder now goes to /tmp when its path holds any other character than letters, digits and / . _ + -. Generated with xum, Model: anthropic:claude-sonnet-5-5, Thinking: xhigh
…lone The model now clears the mark in every snacks close, so a mutant of OwnedLiveSkipsEndedJob alone no longer breaks CloseNeverReuses. It breaks OpenShowsLiveClaude. check.sh --mutants printed NOT CAUGHT as expected for that row. SnacksCloseClearsMark joins the facts that pair with OwnedLiveSkipsEndedJob in SKILL.md. Generated with xum, Model: anthropic:claude-sonnet-5-5, Thinking: xhigh
_Generated with xum • Model: anthropic:claude-sonnet-5-5 • Thinking:
|
@codex review |
|
@codex security review |
🛡️ 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. |
|
Codex Review: Didn't find any major issues. 🚀 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". |
|
Record before the merge of
Generated with |
…s terminal and new-tab diffs are placed (coder#598) ## Summary A `BufLeave` autocmd of the user's that enters another window no longer makes Neovim load the Claude terminal into the user's window. The terminal lands in the split that the plugin made for it. The providers and the diff put the terminal into that split with a new helper, `utils.win_set_buf_in_split`. This PR replaces #518, which is closed. #518 repaired the swap after a user's `BufLeave` autocmd had already redirected it, and each round of verification found a new problem in that repair. This PR fires `BufLeave` before the swap, so the autocmd cannot redirect it. The helper fires `BufLeave` of the old buffer itself, in the split, before the swap. It lets Neovim fire every other event of the swap, with Neovim's own locks and in Neovim's own order. It fires no other event by hand and it enters no other window to get them. ## Why The providers make a split. The split shows the user's buffer. The provider then calls `nvim_win_set_buf(split, terminal_buffer)`. Neovim runs the `BufLeave` autocmds of the user's buffer inside that call. When one of them enters another window (a window-switch autocmd, a session plugin), Neovim loads the terminal into that window. The call named the split, but the terminal lands in the window of the user's file, in two windows, or in none. Nine earlier versions of this fix broke other behavior, or left a defect, and independent verifiers found each one: - Version 1 fired `BufLeave`, `BufWinLeave` and `BufHidden` by hand before the swap. It decided once, before the user's autocmds ran, whether the buffer lost its last window. A hand-fired event also does not lock the buffer. - Version 2 let Neovim fire the events, but it moved the cursor to the other window and back to do it. `BufLeave` then ran in the window of the user's file, so a `BufLeave` autocmd that closes the window made Neovim fire no `BufWinLeave` or `BufHidden` (an autosave did not run, and a pending diff stayed pending with its proposal in no window). The extra `WinEnter` ran a user's `startinsert` autocmd for terminal windows with the terminal in the split. A `WinEnter` autocmd that closes the split made `:ClaudeCodeOpen` fail. When `'eventignore'` listed the four window events, the terminal's `BufWinEnter` autocmd did not run. - Version 3 asked whether it ran inside an autocmd with a bare `expand("<abuf>")`. Outside an autocmd that call raises E496 when `'verbose'` is above 0 (`:verbose`, `nvim -V1`, `set verbose=1`), so every placement that goes through the helper failed with that option set: no new terminal, no hidden terminal shown, no terminal in a diff's new tab. (The first start of a Snacks terminal does not go through the helper, and it did not fail.) Under `:silent!` the call raises nothing and sets `v:errmsg` to E496 instead. Version 4 left it there, and main leaves it empty. The helper now gives `v:errmsg` back as it was. A verifier found it. An earlier commit (7c525f6f) had removed the `pcall` around the call, and its message said the call raises in no context. The message was wrong. The `pcall` is back, and a spec runs the commands with `'verbose'` at 1 and at 15 and under `:verbose`. - Version 4 fixed version 3, and added a one-line warning for a closed split. A verifier found defects in it: under `:silent!` with `'verbose'` set the placement left E496 in `v:errmsg` (main leaves it empty), the toggles logged an error after the warning, a wipe of the terminal's buffer logged "closed the terminal's window" where no window closed, and a guard (the cursor goes back to the window the show started in only while that window exists) had no test. - Version 5 fixed those. Its guard on the warning read the provider's record of the buffer after a call that runs the user's autocmds. When a `BufLeave` autocmd closed Claude (`:ClaudeCodeClose` or `terminal.close()`), that record was nil, and the show raised E5108 in 16 of 16 native rows, where main raises nothing. A `BufLeave` autocmd that wiped the buffer and started a new terminal drew a false message the same way. - Version 6 removed the warning and read the terminal's buffer into a local before the autocmds of a show. A verifier found one gap in its tests: a mutant that put `term.buf` back at the focused Snacks call passed every spec, while it raised `Invalid 'buf'` in 12 of 12 rows of a `WinNew` or `WinEnter` autocmd that closes Claude. My own mutant of that line changed the focused and the unfocused call in one edit, so the unfocused case killed it. Two comments in the toggles ("When it fails, it has said why.") and one README sentence ("`:ClaudeCode` and `:ClaudeCodeFocus` log what they log on main") were also false. - Version 7 added those tests and fixed the comments of the toggles and the shows. The readiness check of this PR then found a path that loses text, which version 6 introduced in the helper. After `BufLeave`, a user's autocmd can show another buffer X in the split. The helper then swapped with `buffer!`, which skips the E37 check of a plain swap. `buffer!` unloads or wipes X with its text when X is changed, no other window shows it and its 'bufhidden' is `unload`, `delete` or `wipe`, where a plain swap keeps X. In the probe of this path (two shapes: an autocmd that creates X, and one that moves the user's unsaved file into the split), version 7 lost the text in 24 of 40 pairs, and main in none. - Version 8 refused the swap in that case: the helper returned false and left the buffer in the split. A verifier found three problems. A new native terminal raised E37, because a second swap after `termopen` tried to put the terminal into the split that showed the buffer. A diff in a new tab was built in another tab when the autocmd had entered a window there. And in 28 of 40 probe pairs the terminal did not show where main shows it, while main keeps the text. - Version 9 replaced the refusal with a hide: the helper set the 'bufhidden' of a changed buffer to `hide` for the swap and gave it back. A verifier then found that `buffer!` unloads or wipes a buffer with no change too, when that buffer does not stay loaded on its own ('hidden' off, or its 'bufhidden' is `unload`, `delete` or `wipe`). A terminal with a running job lost its job, a scratch buffer lost its text, and a normal file lost its undo history, its marks and its buffer-local options, where main keeps the buffer loaded. In the verifier's probe of that path version 9 unloaded or wiped the buffer in 42 of 80 pairs (main in none), and 84 rows on 0.12.3 and 56 on v0.8.0 of its caller matrix with a terminal or a scratch buffer as X were worse than main. Version 9 replaced the refusal of version 8 with a hide for a changed buffer, and this version (10) hides every buffer that a plain swap would leave loaded and that `buffer!` would unload or wipe (see How, item 2). The helper gives its 'bufhidden' back. The diff enters the new tab again when the helper returns false. Version 7 had added the tests that version 6 lacked and fixed two comments and the README sentence. The code of the two shows is still the code of version 6, and only their comments changed: the comments of the toggles and of the two shows now say that a false return can also mean that the terminal's buffer is gone, when a `BufLeave` autocmd wiped it or closed Claude. The comments of the toggles also say that the next toggle then starts a new terminal. Version 6 removed the warning: the show logs what main logs. It reads the terminal's buffer into a local before the autocmds that a show runs. The `:silent!` fix and the cursor guard stay. The failure of version 5 is gone: a `BufLeave` autocmd that closes Claude during a show raises no error in 24 of 24 rows of the verifier's probe (main raises in the 8 Snacks rows), and a `WinNew` or `WinEnter` autocmd of the show's own split that closes Claude raises no error in 8 of 8 rows (main raises in all 8). ## How `utils.win_set_buf_in_split(win, buf, opts?)` does this, when it can: 1. It fires `BufLeave` for the buffer that `win` shows, with `:doautocmd <nomodeline> BufLeave`, in `win`, while that buffer still shows there. `:doautocmd` passes on the error of a raising autocmd, as a plain swap does. 2. The buffer that `win` shows now can differ from the old one: an autocmd of that `BufLeave` showed another buffer X there (a terminal with a running job, a scratch buffer, a changed or an unchanged file), or closed the other window of the old one. A plain swap leaves X loaded and hidden, but `buffer!` unloads or wipes X when X is not kept by 'hidden' and no other window shows it. A changed buffer loses its text only when its 'bufhidden' is `unload`, `delete` or `wipe`. Otherwise a terminal loses its job, a scratch buffer its text, and an unchanged file its undo history (a wipe also loses its marks). The helper then sets X's 'bufhidden' to `hide` for the swap, with `:noautocmd` so that no `OptionSet` autocmd runs, and sets the old value again after the swap, also when the swap raises. The old buffer, when it is still the buffer in the split, gets the same only when it is changed and an autocmd closed its other window: a plain swap decided before that autocmd ran, and then unloads or wipes it with its text when its 'bufhidden' says so (d1, below). An old buffer with no change is left as it is, because a plain swap unloads or wipes it in the same way. The helper does not use `:buffer` without `!`, because that raises E37 or opens the 'confirm' dialog. 3. When an autocmd of that `BufLeave` moved the cursor, it puts the cursor back in `win` under `:noautocmd`, with no event. 4. It swaps with `buffer!` under `:noautocmd`. For the swap, `'eventignore'` holds the caller's value plus `BufLeave` (it ran already) and `OptionSet` (so setting the option is silent). `:noautocmd` sets the caller's value again afterwards, also when the swap fails, with no `OptionSet`. 5. Neovim then fires `BufWinLeave`, `BufHidden` and `BufUnload` for a buffer that lost its last window (also when a `BufLeave` autocmd closed the other window of it), `BufEnter` and `BufWinEnter`. They run after the `BufLeave` autocmds are done, with the locks that Neovim holds. The helper makes the plain `nvim_win_set_buf` instead when it runs inside an autocmd (it asks with `pcall(vim.fn.expand, "<abuf>")`, and an error of that call means "outside an autocmd"), when `win` is not the current window, when `win` shows the buffer already, in the command-line window, with `'winfixbuf'`, and when a plain swap could refuse to leave the old buffer (it is changed, not hidden and shown in no other window). When a `BufLeave` autocmd closes the split, the helper shows nothing and returns false. The providers then leave the terminal hidden, as for any hidden terminal, and the next `:ClaudeCodeOpen` shows it. The show logs nothing about it, and neither do `:ClaudeCode` and `:ClaudeCodeFocus`. Without focus, the show puts the cursor back in the window it started in, when that window still exists (the autocmd can have closed it too). When a `BufLeave` autocmd wipes the terminal's buffer, no window closed and nothing is left to show. The helper shows nothing and returns false, and the show logs nothing, as on main. Both providers read the terminal's buffer number into a local before the split and the swap, because a user's autocmd of either can close Claude, which clears the provider's record of the buffer. A plain swap loads the terminal into the window that the cursor ends in. `opts.ignore_bufenter` adds `BufEnter` to the ignored events, as before, for the unfocused Snacks show and the diff. Call sites: the new terminal, the swap after `termopen`, and the hidden terminal in `terminal/native.lua`. The re-shown split, focused and unfocused, in `terminal/snacks.lua`. The terminal in a diff's new tab in `diff.lua`. ## What a user's autocmd sees, compared with main 1. `BufLeave` for the old buffer runs in the split, as on main. The events and their order are a plain swap's: the sequence of events is equal to main's in 27 of 27 shapes on Neovim 0.12.3 and 21 of 21 on v0.8.0. In 21 and 18 of them the trace is exactly equal, and in the other 6 and 3 (the shapes that ignore `BufEnter`: the diff in a new tab, the unfocused Snacks show) main also fires `OptionSet` for `'eventignore'` twice and this branch fires none (the shapes: no redirect, defaults, `'nohidden'`, an unsaved file. The paths: native new, hidden and repaired, focused and unfocused, Snacks hidden, and `openDiff` in a new tab). No extra `WinLeave` or `WinEnter` runs. 2. A `BufLeave` autocmd that closes the other window of the file (the `BufHidden` autosave shape), and one that closes the window that the pending diff's proposal is in, make Neovim fire `BufWinLeave` and `BufHidden` after the autocmd returns, as on main. The autosave runs and the diff is answered, nested or not. 3. A `WinEnter term://*` autocmd that runs `startinsert` leaves the user in Normal mode after a diff in a new tab, as on main. 4. A raising `BufLeave` autocmd fails the command with its error and shows no terminal, as on main. The message has more frames around it than on main. 5. When `'eventignore'` lists `WinLeave`, `BufLeave`, `WinEnter` and `BufEnter`, the `BufWinEnter` autocmd for terminal buffers still runs, because Neovim fires it. Differences from main that remain, all measured: - **No `OptionSet` event runs while the swap runs.** This includes the `OptionSet` events for options that a `BufEnter` or `BufWinEnter` autocmd of the user's sets during the swap. The helper adds `OptionSet` to `'eventignore'` so that setting the option fires no event of its own. - **A change of `'eventignore'` that an autocmd makes during the swap is undone afterwards**, because `:noautocmd` sets the caller's value again. - **A `BufLeave` autocmd that closes the split leaves the terminal hidden.** The helper returns false and the providers keep the terminal hidden (a new terminal starts and runs, a hidden one stays hidden, a diff in a new tab opens with no terminal). On main the terminal lands in the window that the cursor ends in. A pending diff then shows the difference: on main the terminal replaces the proposal in the user's window, the proposal is hidden and the diff is rejected. Here the proposal stays on screen and the diff stays pending, 20 of 44 pairs of Opus's handler repro (close the current window by `nvim_win_close(0)`, `:close` or `:hide`, nested or not). The 24 pairs of the other handlers (`:enew`, `:edit`, `:buffer`, `wincmd p`, no-op, and the moves with no terminal) have the same outcome as main. In 12 of them (`:enew`, `:edit`, `:bprevious`, both focus modes, both builds) main fires `BufWinLeave` and `BufHidden` for the proposal while another window still shows it, and this branch does not. - **A `BufLeave` autocmd that shows another buffer X in the split.** The terminal shows as on main, and X stays loaded with its text, its running job and its 'bufhidden' as it was, whatever X is: a changed or an unchanged file, a terminal with a running job or an exited one, a scratch buffer, a help or a quickfix buffer, a buffer that a float or a window of another tab also shows. The helper hides X for the swap, and the probe sees no `OptionSet` run, as on main. Two differences remain. Neovim fires `BufWinLeave` and `BufHidden` for X when the swap leaves it (240 of the 288 pairs of the verifier's probe over 12 kinds of X, the pairs where X leaves its last window). Main fires none for X, and fires them for the user's file A instead, while A still shows in another window and X is the current buffer. And a `BufHidden` autocmd of X sees `hide` as its 'bufhidden' while the swap runs, when the helper hid X. In the verifier's probe of 12 kinds of X, 288 of 288 pairs equal main's in every field that the probe prints, and in the 80 pairs of the probe of version 9 (the unloaded or wiped X) 80 of 80 equal main's. In the lost-text probe all 40 pairs equal main's. - **A show without focus, when the autocmd entered a third window before it closed the split:** the cursor goes back to the window it was in, as after any show without focus (in version 3 it stayed in the third window). The show logs nothing, and neither do `:ClaudeCode` and `:ClaudeCodeFocus`. A new terminal that starts in this shape logs nothing either, and the cursor ends in the window it was in, with focus as well. When a hidden terminal is shown with focus, the cursor stays where the autocmd put it. On main the terminal lands over the third window's buffer, and with focus the call raises `Invalid window id`. - **`'eventignore'` holds `BufLeave` for the whole swap**, because it cannot name the old buffer. A `++nested` `BufEnter` or `BufWinEnter` autocmd that leaves a buffer during the swap gets no `BufLeave` (`autocmd BufWinEnter term://* ++nested wincmd p`). The layout and the mode are equal to main's. I did not fix this: a fix needs another mechanism, which the stop rule of this work forbids. The README and the helper's comment say it. - **A `BufLeave` autocmd that shows another buffer X in the split, the events.** Neovim fires `BufWinLeave` and `BufHidden` for X, the buffer it put there and left, whatever X is. On main the swap fires them for the user's file A instead, which still shows in another window while X is the current buffer, and fires none for X. A spec pins it. - **Limits that follow from X really leaving the split. The user's own autocmd causes each one.** Because the swap hides X, the user's `BufWinLeave` and `BufHidden` autocmds of X run, and each does what it was written to do. I found no way to remove them without a new mechanism. `'eventignore'` cannot name X, so ignoring the two events would also drop them for the user's own file (the autosave and diff-answer specs pin them), and the verifier's trial of that change differed from main in 64 other pairs. - **m1: an autosave autocmd on `BufHidden` or `BufWinLeave` that writes the buffer of the event writes X's typed text to disk.** 32 of 32 rows, main 0. No work is lost. On main a curbuf-style autosave (`:write` when `&modified`) writes X too, because X is the current buffer when main fires the events for A. - **m2: a `BufWinLeave` or `BufHidden` autocmd of X that raises makes the placement raise.** 128 rows (the four kinds of X, both builds). The split keeps X, and its 'bufhidden' is back. Main raises nothing. The same autocmd raises on any hide of X. - **m3: a user `BufHidden` autocmd that cleans up the hidden buffer removes X.** 96 rows: 64 with a scheduled `bwipeout!` (a terminal's job ends in 16 of them) and 32 with `nvim_buf_delete`. Main 0. The user's autocmd asks for the removal. Version 9 unloaded or wiped X by itself in the same shapes, and now the user's autocmd decides. - **m5 (v0.8.0 only): a user `BufWinEnter` autocmd of the terminal that runs `bwipeout! X` wipes X as asked.** 32 pairs. Main on v0.8.0 keeps X, because it leaves X with a stale window count (`:ls` shows X as active with no window). Main on 0.12.3 wipes X like this branch. - **d1 (better than main): a `BufLeave` autocmd closes the other window of a changed old buffer whose 'bufhidden' is `unload`, `delete` or `wipe`.** A plain swap decided before the autocmd ran, so main unloads or wipes the buffer with its text (12 of 16 pairs, 'hidden' on and off, both builds). The helper hides it for the swap and keeps it (16 of 16). - **Session shape of gpt's round-5 repro** (`'hidden'` off, a `BufLeave` autocmd that enters the first window, a `WinEnter` autocmd that swaps the plugin's own buffer into it): the unsaved text stays and the job runs on both trees. On this branch the user's file shows in no window, because the session plugin's own swap replaced it in the first window, while the terminal is in the split. On main the file shows in one window. In 3 more rows per build of Opus's matrix (a hidden show without focus, with `'nohidden'`, `'bufhidden'` unload or wipe), the user's own options unload (2 rows) or wipe (1 row) the unmodified file when the user's window swaps it out. Main keeps the file in a window but leaves the terminal in none. No text, write or job is lost in any of these rows. - **Rows where this branch lacks an event that main fires**: 179 of 1189 on 0.12.3 and 164 of 825 on 0.8.0 lack a `BufWinLeave`, `BufHidden` or `BufUnload` of the user's file by my count. The verifier's rerun with its own script counts 174 and 166. I did not find out why the two counts differ by 5 and 2 rows. They are the shapes where main loads the terminal over the user's file (the redirect: a `BufLeave` autocmd that enters another window, another tab or a float, or closes the split), so the file loses its window on main and keeps it here. The verifier found that in 161 and 154 of its rows this branch keeps the file in its window, so no event is owed there (I did not count that again). In 12 rows on 0.12.3 and 9 on 0.8.0 the file ends in no window on this branch with no event, and in 3 more rows per build (by the verifier's rerun) the user's own options unload or wipe it, see the session shape above: a `WinEnter` autocmd of the user's swaps a buffer into the user's window while the terminal is shown without focus. Neovim fires no event for a window change inside a non-nested autocmd. On main the redirect hid the file first. ## Evidence Labels such as F1, N1, m4, m9, P1 and P2 name findings from the independent verifiers' reports on earlier versions. Opus and gpt are the two models that ran verification lanes. Real Neovim 0.12.3 and v0.8.0, headless, a stub `sleep` as the Claude job, no real `claude`. Snacks runs on 0.12.3 only, from a clone in my scratch directory (Snacks is not a test dependency). The large matrices (Opus's, gpt's, my swap and edge matrices) ran on `4097d8da`. Since then this branch changed `lua/` in `utils.lua` (the `pcall` around `expand` removed, put back, and `v:errmsg` given back), and later the 'bufhidden' hide of the swap (`f7dc2d87`, `9e0ed389`, `d3484a9c`), in `diff.lua` (the new-tab re-entry, `507e671a`), in `native.lua` and `snacks.lua` (the closed-split branch of a hidden show: the cursor and its guard, and the buffer number read into a local) and in docs. The large matrices never ran against the 'bufhidden' hide or the re-entry. The merges of main brought in changes in `diff.lua`, `diff_inline.lua`, `cwd.lua`, `terminal.lua`, `selection.lua`, `init.lua` and others. The large matrices did not run again on those. These ran again in iteration 6, on `c4951459` and on main `692b460c`: the handler matrix, the F1 repro, real Snacks with `'verbose'`, and the probes of the verifiers of iterations 4 and 5 (the closed split with the user's window closed too, a wipe of the terminal's buffer, `:silent!`, `:ClaudeCode` and `:ClaudeCodeFocus`, a `BufLeave` autocmd that closes Claude, and a `WinNew` autocmd that does). The verifier of iteration 7 ran them again on `1ba631b9` and on main `d05b0101`, and got the same counts. "Main" is `9bfc39c9` for the large matrices, and `692b460c` for everything that ran in iteration 6. The current main, `d21f310a`, has #536 and #602 among later commits, and #602 changed terminal files that this PR also touches. An independent verifier reran the caller, handler and restore matrices and the 12-kinds probe on the merged head `ef27d623` and on main `d21f310a`: no row is worse than main, and the head's rows equal its earlier run apart from file paths in error text. | Check | main | this branch | | --- | --- | --- | | Contract specs (`terminal_placement_contract_spec.lua`, 21 cases, and `terminal_placement_diff_answers_spec.lua`, 16 cases: events and order, autosave, mode, diff answers, raising `BufLeave`, `BufWinEnter` mapping, `WinEnter` autocmds, session shape, E937) | 37 of 37 pass on both builds (`692b460c`) | 37 of 37 pass on 0.12.3 and v0.8.0 (`c4951459`), and on v0.9.5 (an earlier head). Iteration 2 of this PR fails 30 of 37 | | `'verbose'` repro (F1 of the verifier of iteration 3): a new terminal, a hidden terminal and `openDiff` in a new tab with `'verbose'` at 1 and at 15, and `:verbose ClaudeCodeOpen` for the first two, 8 rows per build | 8 of 8 ok on both builds | 8 of 8 ok on both builds, with the same fields as main (16 of 16 rows equal). Iteration 3 (`aaeb33c8`): 0 of 8 ok on each build, E496 in each. Levels 2 and 9 and the modifier ran too: 28 of 28 ok on the head before the last merges. Run again in iteration 6 with the script of the verifier of iteration 3 (32 runs): main `692b460c` 8 of 8 per build, this branch 8 of 8 per build, 16 of 16 pairs equal to main. The spec: 8 of 8 on main and on this branch, on both builds. The verifier of iteration 7 ran the repro on `1ba631b9` and on `d05b0101`: 28 of 28 pairs equal, all ok. Real Snacks (a hidden terminal shown at `'verbose'` 1 and 15 and under `:verbose`, and a new one at 1): 4 of 4 ok on main and on this branch in iteration 6 and in iteration 7, with no E496 | | The probes of the verifier of iteration 4, on native (0.12.3 and v0.8.0) and Snacks (0.12.3), focused and not. The autocmd closes the user's window, then enters W and closes the split | `Invalid window id` in every row (the guard that main does not have) | ok in every row, the cursor in W, nothing logged (6 of 6 rows: native on both builds, Snacks on 0.12.3, with and without focus) | | The same, with an autocmd that wipes the hidden terminal's buffer and closes no window | no message | no message in 6 of 6 rows. Iterations 4 and 5 logged "closed the terminal's window" in some orders, which was false | | `:ClaudeCode` and `:ClaudeCodeFocus` on a hidden terminal whose split a `BufLeave` autocmd closes | the command fails with an error | nothing logged. Iteration 4 logged a warning and then `[ERROR] Failed to show hidden terminal` | | `'verbose'` 1 and `:silent! ClaudeCodeOpen` (v:errmsg) | `v:errmsg` is empty, both builds | empty, both builds. Iteration 4 left E496 in it | | The probe of the verifier of iteration 5: a `BufLeave` autocmd closes Claude (`:ClaudeCodeClose`, `terminal.close()`) while a hidden terminal is shown by `:ClaudeCodeOpen`, `:ClaudeCode`, `:ClaudeCodeFocus` or `ensure_visible`, with `auto_insert` off and on. Native on 0.12.3 and v0.8.0, Snacks on 0.12.3: 24 rows | the 16 native rows are ok, and the 8 Snacks rows raise `Invalid window id` | 24 of 24 ok. The 16 native rows have the same fields as main (layout, cursor, mode). Iteration 5 raised `Invalid 'buf': Expected Lua number` in those 16 native rows | | A `WinNew` or `WinEnter` autocmd of the show's own split closes Claude (`:ClaudeCodeClose` or `terminal.close()`), then `:ClaudeCodeOpen` on a hidden native terminal: 8 rows | raises in 8 of 8 (`Invalid 'buf'`, `Expected Lua number`) | ok in 8 of 8 (the local read of the buffer number) | | The verifier of iteration 6's probe: a `WinNew`, `WinEnter` or `WinLeave` autocmd of the show's split closes Claude (`terminal.close()` or `:ClaudeCodeClose`), four entry points, native on both builds and Snacks on 0.12.3 (the verifier of iteration 7 ran them again on `d05b0101` and `1ba631b9`, with the same counts) | raises in 72 of 72 rows | ok in 72 of 72 rows | | A `BufEnter` or `BufWinEnter` autocmd of `term://*` closes Claude (`terminal.close()` or `:ClaudeCodeClose`) during a show, through the four entry points, native on both builds and Snacks on 0.12.3: 48 pairs | the show raises `Invalid window id` in 38 of 48 rows | the same 38 rows raise, and the 48 pairs are equal in every field (layout, cursor, mode, notes). See the follow-up P2 | | The probe of the lost-text path: a `BufLeave` autocmd shows another changed buffer X in the split. Two shapes (an autocmd that creates X, and one that moves the user's unsaved file into the split), 'bufhidden' empty, `hide`, `unload`, `delete` and `wipe`, 'hidden' on and off, the helper on 0.12.3 and v0.8.0: 40 pairs | main keeps the text of X and shows the terminal in 40 of 40 pairs | version 7 loses the text in 24 of 40 pairs. Version 8 shows the terminal in 12 of 40. This head keeps the text and shows the terminal in 40 of 40, and its rows equal main's in 40 of 40 | | The events of X in the first shape (20 pairs: BufLeave, BufWinLeave, BufHidden, BufUnload, BufWipeout, BufDelete, and the `OptionSet` runs for 'bufhidden') | `OptionSet` 0 runs. X gets no leave event, because its own swap runs inside the swap of main | `OptionSet` 0 runs in 20 of 20 pairs. X gets one `BufWinLeave` and one `BufHidden`, and no unload or wipe event. Its 'bufhidden' is as it was in 20 of 20 | | The caller matrix of the verifier of version 8 (native start and show, each with and without focus, Snacks show with and without focus, and the diff in a new tab, with 8 `BufLeave` handlers and option sets: 280 rows on 0.12.3 and 200 on v0.8.0, and I reran his scripts) | reference | Version 8: 86 rows worse than main on 0.12.3 and 56 on v0.8.0 (E37 in a new native terminal, the terminal not shown, the diff in another tab). Versions 9 and 10: 0 rows worse on both builds. Version 10 (`d3484a9c`): 0.12.3 has 120 better, 24 mixed and 136 same, and v0.8.0 has 92 better, 16 mixed and 92 same. The mixed rows are all closed-split rows (`close-split`, `tab-close`), where main shows the terminal in a window of the user's or raises, and this branch leaves it hidden, as the differences above say. The next `:ClaudeCode` shows it in a new split with one job | | The same caller matrix with a terminal or a scratch buffer as X (the verifier of version 9: 560 rows on 0.12.3 and 400 on v0.8.0) | reference | Version 9: 84 rows worse than main on 0.12.3 and 56 on v0.8.0. Version 10: 0 rows worse on both builds (0.12.3: 240 better, 48 mixed, 272 same. v0.8.0: 184 better, 32 mixed, 184 same). The 48 and 32 mixed rows are the same closed-split rows, once for each kind of X | | The probe of the unloaded or wiped X (the verifier of version 9: a `BufLeave` autocmd shows X in the split, X a changed file, a terminal with a running job, a scratch buffer or an unchanged file, 5 'bufhidden' values, 'hidden' on and off, both builds: 80 pairs) | main keeps X loaded, with its job and text, in 80 of 80 pairs | version 9: X unloaded or wiped in 42 of 80 pairs (the job dead in 14, the scratch text gone in 14). Version 10: 80 of 80 pairs equal main's | | The probe of 12 kinds of X (the verifier of version 10: a changed buffer, a running terminal, an exited terminal, a `nofile` scratch buffer, an unchanged file, a help buffer, a quickfix buffer, an unlisted changed buffer, a scratch buffer from `nvim_create_buf(false, true)`, X also in a float, X in a float that the autocmd closes, X also in a window of another tab; 6 'bufhidden' values, 'hidden' on and off, both builds: 288 pairs) | reference | version 9: 116 pairs worse. Version 10: 288 of 288 pairs equal in every field (X loaded, text, job, call, split, 'bufhidden', 'buflisted', X's window count, current window, mode, `OptionSet` runs). The only difference is the events: `BufWinLeave` and `BufHidden` for X in 240 pairs, and none on main | | The restore of 'bufhidden' (the verifier of version 10: a changed file, an unchanged file, a terminal and a scratch buffer as X, 15 scenarios (a raising `BufWinLeave`, `BufHidden` or `BufWinEnter`, a wipe in the terminal's `BufWinEnter`, a show in it, a `BufHidden` that sets 'bufhidden', the nested placement from X's `BufHidden`, autosaves, `mkview`), 4 'bufhidden' values, 'hidden' on and off, both builds: 960 pairs) | reference | `hide` left on X: 0 pairs. `OptionSet` runs: 0. The 'bufhidden' values that differ from main's come only from the user's own wipe or the user's own autocmd that sets the option | | The old buffer whose other window an autocmd closes (d1: a changed buffer, 'bufhidden' empty, `unload`, `delete`, `wipe`, 'hidden' on and off, both builds: 16 pairs) | main keeps the text in 4 of 16 pairs and loses it in 12 | version 10 keeps the text in 16 of 16 pairs | | The K1 probe of that verifier (a new native terminal starts, and a `BufLeave` autocmd shows a changed buffer with 'bufhidden' wipe or unload, with and without focus, on both builds: 8 rows) | Version 8 raises E37 in 8 of 8 rows. Main raises in none | This head raises in none, shows the terminal in 8 of 8, keeps the text, and a second entry adopts the one Claude job | | The K2 probe of that verifier (the real native terminal, a diff with `open_in_new_tab`, and a `BufLeave` autocmd that enters the first tab and closes the new split) | Main builds the diff in the new tab on 0.12.3, and replaces the user's window in tab 1 with the terminal. On v0.8.0 main fails the open (`wincmd` E16). Version 8 builds the diff in tab 1 and leaves the new tab empty | This head builds the diff in the new tab on both builds, and tab 1 keeps the file and the terminal | | The orders probe of the verifier of iteration 5 (60 rows: the autocmd closes the split and wipes the buffer, closes Claude through the API, runs `:ClaudeCodeOpen` or `:ClaudeCode`, or wipes the buffer and runs `:ClaudeCode`, on native and Snacks, with 4 entry points) | reference | no row raises where main does not, and 14 are better. 13 are equal, and 33 differ: the shapes where the autocmd runs `:ClaudeCodeOpen` or `:ClaudeCode` itself (see the follow-ups), and the mode after a focused show | | The verifier's `own_probe` (m1, m2, `'switchbuf'`, a nested option): 6 scenarios × 2 builds | reference | layout and mode equal to main's in 8 of 12 rows. The other 4 are the closed-split rows (the terminal is hidden here, see above), where the cursor after an unfocused show is now in the window it was in, as on main (iteration 3: the third window). The events differ as listed above (the `BufLeave` of m1, the `OptionSet` events of main, the events of main's redirect) and in the two nested rows (`autocmd BufWinEnter term://* ++nested wincmd p`). In the focused nested row this branch fires 12 events and main fires 9: four more and one fewer. The four more: after the nested `wincmd p`, the provider's focus step enters the split again (`BufLeave` and `WinLeave` of the file, then `WinEnter` and `BufEnter` of the terminal), where main's API swap had restored the window with no events. The one fewer is the `BufLeave` of the terminal that main fires for the nested `wincmd p` (the `'eventignore'` limit above). In the unfocused nested row main fires 13 events and this branch fires 8. Main fires five more: two `BufLeave` of the terminal, and a second `WinLeave` of the terminal, `WinEnter` and `BufEnter` of the file. The counts are the same on both builds. I did not find out why main fires that set. The final layout and mode are equal on both builds. I did not change this: a change needs a mechanism beyond this one | | Opus's matrices, run by his scripts (1189 scenarios on 0.12.3, 825 on 0.8.0: redirect, event traces, 17 adversarial autocmds × 5 option sets, nested context) | reference | 0.12.3: 461 same, 728 different, 0 worse, 339 better. 0.8.0: 269 same, 556 different, 0 worse, 255 better. A first run had 19 worse rows (a hidden new terminal when the user's `BufLeave` autocmd wiped the user's file raised `Invalid window id`). Fixed in `4097d8da`, with a spec. The 19 rows ran again | | The same matrices, a richer score (events of a hidden buffer, mode, writes, text, extra `WinEnter`) | reference | the flags that remain: 12 and 9 rows lose `BufWinLeave` and `BufHidden` of a file that the user's own non-nested `WinEnter` autocmd hid (see above). 28 and 19 rows have one more `WinLeave` and `WinEnter` than main. In the rows I opened, one for each of the six actors, main raised, crashed or lost the terminal, or the provider goes back to the user's window after the user's autocmd closed the split. Exit codes of the matrices: main 32 runs on 0.12.3 and 4 on 0.8.0 end with a signal or an error, this branch 2 and 0 (the 2 are Snacks with `'bufhidden'` wipe, which main crashes in too) | | gpt's matrices (108 pairs: redirect, autosave, public actors, diff in a new tab, Snacks fixbuf) | reference | 0 worse. Redirect rows without the terminal in the user's window: 10 of 30 on main, 26 of 30 here on 0.12.3 (the 4 left are the first start of a Snacks terminal), 6 of 18 and 18 of 18 on 0.8.0. Diff in a new tab: 0 of 12 on main, 12 of 12 here. Autosave rows and public actors: equal. All the rows that differ with the same status are rows where main's terminal is in the user's window | | gpt's focused scenarios (F1 autosave, F2 pending diff, F3 `BufWinEnter` mapping, F4 `WinEnter` that closes the split, raising autocmds, 32 scenarios × 2 builds) | reference | equal in 62 of 64 pairs. Different: the pending diff with a close-the-current-window autocmd (see above), on both builds | | Opus's `WinEnter startinsert` repro (diff in a new tab, unfocused hidden show, Snacks, 4 event sets, 40 runs) | the mode and the typed key | equal in 40 of 40 | | Opus's F2 repro: a pending diff and 11 `BufLeave` handler kinds × focused and not × 2 builds | 44 pairs | 24 equal, 20 different (the close-the-split kinds) | | Opus's stale-diff repro (a pending diff and a `BufLeave` autocmd that closes the other windows of the proposal), 8 runs | the diff is rejected and answered | equal. In iteration 2 it stayed pending | | gpt's hidden-autosave repro | disk saved, 3 of 3 per build | disk saved, 3 of 3 per build. The assert of the script that `BufLeave` runs inside the split holds again | | gpt's visible-scratch `BufHidden` repro, `BufWinEnter` facts (16 pairs), Opus's round-5 Snacks repro (24 runs) | | equal. 24 of 24 shown with the terminal in its split, where main redirects | | Handler matrix: autocmds of 19 shapes that enter, create, close, wipe or raise, × 6 option sets, per path (912 rows on 0.12.3, 708 on 0.8.0), with a mode, text and job check | reference | 0 worse. 178 and 142 rows better. 0 rows with a wrong mode, a lost file, text or job. 89 rows on 0.12.3 and 60 on 0.8.0 end in another mode than main. In each of them main put the terminal in the user's window or in the other tab, or raised, and this branch ends in the mode that the focus asks for (the verifier's list). Run again in iteration 6 against main `692b460c`: the same counts, and every row has the same result as in the run of iteration 5 (0 of 912 and 0 of 708 differ). The verifier of iteration 7 ran it on `1ba631b9` against main `d05b0101`: the same counts, and the head's rows equal the run of iteration 6 in 912 of 912 and 708 of 708. I ran it again on `8b28b377`: 912 rows with 0 worse, 178 better, 610 same and 124 changed with the same score, and 708 rows with 0 worse, 142 better, 484 same and 82 changed. The rows of main are those of `692b460c`, and no file of `lua/` changed on main since. Main ends with no result (a crash) in 5 rows per build, and this branch in none. The m2 shape is not one of its handlers. On `d3484a9c` the rows are byte-identical to those of `cd136bbf` on both builds: 912 rows with 0 worse, 178 better, 610 same and 124 changed, and 708 rows with 0 worse, 142 better, 484 same and 82 changed. This matrix has no handler that shows another buffer in the split, so the probes above cover that shape | | My swap matrix (native 18 rows, Snacks 12 rows per build) | 10 of 30 ok on 0.12.3, 6 of 18 on 0.8.0 | 26 of 30 and 18 of 18 | | My edge matrix: 216 rows on 0.12.3, 144 native rows on 0.8.0 | ok 28 of 217 and 16 of 146 | ok 216 of 216 and 144 of 144 | | `openDiff` in a new tab with a `BufLeave` autocmd that enters another window, 24 runs per build | ok 6 of 24 | ok 24 of 24 | | Event traces, no redirect: 27 shapes on 0.12.3, 21 on 0.8.0 | reference | the sequence of events is equal in all of them, and the trace is exactly equal in 21 and 18. In the other 6 and 3 (the shapes that ignore `BufEnter`) main also fires `OptionSet` for `'eventignore'` twice. Iteration 2 had one more `WinLeave` and `WinEnter` pair in each | | Snacks `fixbuf` repro, 24 runs | terminal in the user's window | terminal in its split | | #485's close probe, 24 runs, local trial merge of this branch into #485's head | #485's head alone switched the user's window in 20 of 24 | trial merge: 24 of 24 ok, 0 switched. The 10 `diff_new_tab_*` specs of #485 and the 3 placement specs pass in it, on both builds | ## Not covered - **`Snacks.terminal.open` for a terminal that Snacks creates for the first time.** Snacks makes that window itself, so a `BufLeave` autocmd still redirects it, as on main. It is 4 of the 12 Snacks rows. - **`WinNew` autocmd that opens and enters its own window during the provider's `:vsplit`.** The terminal replaces that window's buffer, on main and here (12 of 12 rows). I did not change it. - **Callers inside an autocmd.** The helper makes the plain swap there, so an autocmd that calls `:ClaudeCode` from a nested `BufEnter` still redirects, as on main. A spec pins that the plain swap fires `BufLeave` once. - **An unsaved file that no other window shows, with `'hidden'` off.** A plain swap raises E37 there, and the helper makes the plain swap, so it raises E37 too, as on main. - **Neovim 0.10 and 0.11.** I ran 0.12.3, v0.9.5 (the four placement and diff specs only) and v0.8.0. Snacks ran on 0.12.3 only, and has no integration spec, because snacks.nvim is not a test dependency. The Snacks sites have unit specs with a fake window model, and the Snacks rows above ran against a real clone. - **The large matrices after `4097d8da`.** Later commits of this branch change `lua/` in `utils.lua`, `native.lua`, `snacks.lua` and `diff.lua` (see Evidence), and main's merges changed other files. The unit and integration specs, the handler matrix, the F1 repro, the N1 and orders probes, and the closed-split and nested rows of `own_probe` ran again (on `c4951459`, before the 'bufhidden' hide, the diff's re-entry of the new tab and main's #536 and #602. The caller, handler, restore and 12-kinds matrices ran again on `ef27d623`). The handler matrix ran again on `8b28b377`. Opus's, gpt's and my swap and edge matrices did not. - A user's `BufHidden` autocmd on X that closes Claude now runs during the swap, and it closes Claude. Nothing is lost. - **Follow-up (m9, from iteration 3, not worse than main): a `BufLeave` autocmd that runs `:ClaudeCodeOpen` or `:ClaudeCode` while a hidden terminal is shown.** This branch shows the terminal in two windows (the split and the nested split). Main shows it in the nested split only and leaves the split with the user's file. Each tree ends with one extra window, and neither raises. I did not change it. - **Follow-up (P1, pre-existing, not worse than main): a `BufLeave` autocmd that closes the window a native show started in, while the split survives.** The show without focus ends with `nvim_set_current_win(original_win)` (`native.lua:500` at `ef27d623`) and raises `Invalid window id`, on main and on this branch (the same row on 0.12.3 and v0.8.0, 12 of 12 rows equal in the verifier's probe). Snacks does not raise. I did not change it. - **Follow-up (P2, pre-existing, not worse than main): a `BufEnter` or `BufWinEnter` autocmd of the terminal that closes Claude during a show.** The autocmd runs inside the swap, after the terminal is in the split. The show then enters the window that the close removed and raises `Invalid window id` (`native.lua:494` at `ef27d623` in the focused native show, and `snacks.lua:438`, the full-height `nvim_win_set_height` step of the Snacks show). 38 of 48 rows raise on main and on this branch, and the 48 pairs are equal. I did not change it. AGENTS.md says not to add a check of the split after the swap for a close that a user autocmd made. - **The swap after `termopen` in a native start** is a plain swap when the split then shows a changed buffer that a plain swap can refuse to leave, as on main. The verifier of version 10 probed it: a `TermOpen` autocmd that shows another buffer in the split never reaches that swap (`nvim_buf_call` puts the terminal buffer back into the split when it returns), and a `BufWinEnter` autocmd of the first placement does reach it. 96 of 96 pairs equal main's for each. - **No integration spec runs Snacks** (snacks.nvim is not a test dependency). The `'verbose'` integration spec covers the native provider and the diff. The Snacks sites have unit cases with a fake window model and a fake `expand`, and I ran real Snacks with `'verbose'` by hand (4 of 4 ok, in the Evidence table). ## Tests - `tests/integration/terminal_placement_contract_spec.lua` (21 cases) and `terminal_placement_diff_answers_spec.lua` (16 cases): what a plain swap gives in shapes where it already works. They use the public commands and `openDiff`, so they pass on main: 37 of 37 on Neovim 0.12.3 and v0.8.0. They failed 30 of 37 on iteration 2 of this PR. A shared runner, `tests/helpers/placement_child.lua`, starts one headless Neovim for each case, so a crash or a hang fails one case. - `tests/integration/terminal_placement_spec.lua` (114 cases) and `terminal_placement_shapes_spec.lua` (13 cases): the redirect, the events of the swap against a plain swap, the closed split (the cursor after a hidden terminal's show and that the show logs nothing, the user's window closed too, `:ClaudeCode` and `:ClaudeCodeFocus`, a wipe of the terminal's buffer), Claude closed by a `BufLeave` autocmd (`:ClaudeCodeClose` or `terminal.close()`, through each of the four entry points) or by a `WinNew` or `WinEnter` autocmd of the show's own split (each through the four entry points, with and without focus), the buffer that an autocmd showed in the split (two shapes of a changed buffer with each 'bufhidden' value, for the helper and for each caller: a new native terminal, a native show with and without focus, the Snacks show with and without focus, and the diff in a new tab; and, for the helper, a scratch buffer, a terminal with a running job and an unchanged file with 'hidden' off and each 'bufhidden' value), the `'eventignore'` value, the shapes where the helper makes a plain swap, and the diff in a new tab. Red on main `692b460c`, counted when the placement spec had 102 cases (it now has 114): raw, 61 of 102 placement cases and 12 of 13 shapes cases fail on 0.12.3 (60 and 12 on 0.8.0), and most of them fail for lack of the helper. With a plain-swap alias for the helper in a copy of main (it honors `ignore_bufenter`), 31 of 102 on 0.12.3 and 32 on 0.8.0, and 12 of 13 shapes cases, fail, and they fail for the behavior. On iteration 5 (`bdd389db`), 37 of 102 and 2 of 13 fail on both builds, and 17 of the 74 unit cases of the helper and the Snacks show fail. On version 8 (`8b28b377`), 17 of 102 and 2 of 13 fail on both builds, and 10 of the 74 unit cases fail: the cases of a changed buffer in the split for each caller, and the diff case of an autocmd that enters the first tab. On version 6 (`64c43323`) all 33 pass, because its Lua is right: only a mutant shows the gap. - `tests/integration/terminal_placement_verbose_spec.lua` (8 cases): the native new terminal with `'verbose'` at 1 and at 15, under `:verbose ClaudeCodeOpen` and under `:silent!`, the hidden terminal at 1, under the modifier and under `:silent!`, and `openDiff` in a new tab at 1. Each case also asserts that `v:errmsg` is empty. It goes through the public commands, so it passes on main (8 of 8 on both builds). On iteration 3 (`aaeb33c8`) it fails 6 of 6 of the cases that existed then, and on iteration 4 the 2 `:silent!` cases fail. On this head it passes 8 of 8. - `tests/unit/utils_win_set_buf_spec.lua` (44 cases, a fake that logs the calls in order, with a fake `expand` that raises outside an autocmd when `'verbose'` is set, or sets `v:errmsg` under `:silent!`), and `tests/unit/terminal/snacks_unfocused_show_spec.lua` (35 cases: the Snacks unfocused show through the real helper, with the same fake `expand`, a fake `nvim_set_current_win` that raises for an invalid window as Neovim does, a fake `nvim_buf_is_valid` that raises for a nil buffer as Neovim does, the closed split with the cursor and nothing logged, the user's window closed too, a wipe, and Claude closed by a `BufLeave` autocmd or by an autocmd of the split, each with and without focus). The fake has no 'buftype', so it cannot tell a file from a scratch buffer: the integration cases pin the unchanged file. The other provider specs fake a window model with no buffer options, so they use `tests/helpers/plain_placement.lua`, a plain swap for the helper. - Mutants of the helper and its call sites: 100 mutants, each one text edit of `utils.lua`, `native.lua`, `snacks.lua` or `diff.lua`, with a syntax check of the result before it runs. They are my mutants of the earlier rounds (a guard dropped or inverted, an event not fired or not ignored, the swap by API or without `!`, a validity check dropped, a call site back on a plain swap, `v:errmsg` not given back, the toggles' error back), the 13 of the verifier of iteration 4 (V1 to V13), and 7 for this round: the helper's call sites that read the module's record of the buffer (`bufnr`, `term.buf`) after the split or the swap, and the warning put back with and without that read (L1 to L7). All 100 fail at least one spec: 74 fail the unit specs, and 26 fail only the integration specs, on Neovim 0.12.3. The unmutated control passed in each of the 3 workers. V6 and V10, the two mutants that drop `nvim_win_is_valid(original_win)` in the native and the Snacks closed-split branch, survived every test of iteration 4. V6 fails the integration case with the user's window closed too, and V10 fails the matching Snacks unit case. These 100 ran on `c4951459`. They had no mutant at the focused Snacks call on its own (my L2 changed both Snacks calls in one edit), and the focused call had no test, which the verifier of iteration 6 found (his mutant Lk). On `1ba631b9` my run of the verifier's 19 mutants and an unmutated control gave: all 19 die (9 by the unit specs, 10 by the integration specs only), and the control is clean on both builds. The verifier of iteration 7 ran 25 (those 19 and 6 new ones, for example the local read moved after the split): all 25 die, and his control stays green. I ran the verifier's 19 mutants again on this head: all 19 die with the same split (9 by the unit specs, 10 by the integration specs only), and the control is clean on both builds. For the helper that hides the buffer for the swap and the fix of the diff I ran 16 mutants of my own: the restore of 'bufhidden' dropped, the hiding dropped, the restore without its validity check, no `pcall` around the swap, the error re-raised with a position, the old value not restored, the condition inverted or dropped, the option set by assignment (which runs `OptionSet` autocmds) for the hide and for the restore, the check of the buffer without its 'hidden', other-window or changed leg, and the three changes of the diff's re-entering of the new tab. All 16 die. The 13 mutants of the helper die by the unit specs, and 10 of them by the integration specs too. The restore without its validity check dies by the unit case only, because `setbufvar` on a wiped buffer does nothing on both builds, and the check is a guard after user autocmds. The error re-raised with a position survived my first run, because the unit case compared the error without its position, and it dies now. The three mutants of the diff die by the integration specs only. Lk dies by the new Snacks case with focus, and Lk2 (the unfocused call) by its twin. For `d3484a9c` I ran 33 more mutants of the helper and the diff (the new condition and each clause of it, `old` read after the autocmds, the hide and the restore, the guards of the plain swap, and the diff's re-entry of the new tab): 30 die by the unit specs, and the 3 of the diff die by the integration specs. The 15 mutants of the verifier of version 9 that apply (H1 to H5, H7 to H11, H13, H14, K2a to K2c) die. The verifier of version 10 ran V1 to V10 on the condition. V10 (skip the hide for an unchanged buffer with an empty 'buftype', a normal file) survived every unit and integration spec until the 4 cases of an unchanged file were added. Those 4 fail under V10 on both builds. - `verify-terminal-lifecycle`: the facts of the native start read `utils.win_set_buf_in_split` instead of `nvim_win_set_buf`. `check.sh` passes and `covered.lock` is recorded. In iteration 6 I read the model for `show_hidden` and `cc_show` before I recorded: it describes the show of a hidden terminal as "a window shows the terminal" and does not model the closed-split branch. The change removes the warning there, keeps the cursor return, reads the buffer number into a local, and adds no tracked state (`winid`, `term.win`, the job). `verify-diff-lifecycle`: `check.sh` passes, and `covered.lock` has only file hash changes (no block of the model changed). ## Gate On `ef27d623` (this branch with main `d21f310a` merged), with GNU `timeout` first on `PATH`, a stub `claude` after it (no real `claude`) and a scratch `CLAUDE_CONFIG_DIR`: 1. `mise run clean && mise run all` exits 0. treefmt changes no file. Lint: 0 warnings and 0 errors in 213 files. Busted: 2013 successes, 0 failures, 0 errors, 43 pending. 2. The CI steps for the integration job, run locally (`scripts/run_integration_tests_individually.sh`): Neovim 0.12.3, 1041 successes, 0 failures, 0 errors, in a plain run and in a run under a `TMPDIR` with a space and a `%`. Neovim v0.8.0: 1037 successes, 0 failures, 0 errors, in both runs. No `$XDG_CACHE_HOME/nvim` appears (the real-cache check). The runs include the contract, diff-answers, verbose, shapes and placement specs. 3. All five `check.sh` pass (`verify-diff-lifecycle`, `verify-terminal-lifecycle`, `verify-mention-queue`, `verify-selection-tracking`, `verify-ws-connection`). Since version 9 the change in `utils.lua` is the condition of the hide and comments. The helper adds no tracked state (`winid`, `term.win`, the job, the active diffs) and no new caller path. No fact reads FALSE, and I changed no fact. Both `covered.lock` files were recorded again after the last merge (main's #602 had taken the terminal lock first), after I read the diff. `check.sh --mutants` passed on iteration 4's head (`31bd8931`), and I did not run it again. 4. The mutants are in Tests: 33 of mine on the condition and the diff die, the verifier's 15 of version 9 that apply die, and V1 to V10 of the verifier of version 10 die, V10 only since the 4 cases of an unchanged file. 5. The handler matrix (912 rows on 0.12.3 and 708 on v0.8.0), the caller matrices (280 and 200 rows with a changed X, 560 and 400 with a terminal or a scratch buffer as X) and the probes of the unloaded or wiped X (80 pairs), of 12 kinds of X (288 pairs) and of the restore (960 pairs) ran on `d3484a9c`, and the verifier of version 10 ran them on it too: 0 rows worse than main in every one. Since `d3484a9c` this PR changed `lua/` only in comments of `utils.lua` (checked by hash of the file without its comment lines) and in the README of `terminal/`. The other changes of `lua/` since then (`server/tcp.lua`, `diff.lua`, `terminal/native.lua`, `terminal/owned.lua`, `terminal/snacks.lua` and READMEs) came with main's merges. The verifier of version 10 ran `mise run all` on `d3484a9c` and on a merge of it with main `f28eba7d`: rc 0 and 1958 successes, 0 failures, 0 errors, 41 pending. Main moved since `bcf1f084` by eight commits (skills, the format check, the CI workflow spec, the integration runner, #536 and #602), and I merged them. The last merge took #602 after it had recorded the terminal lock. No item failed on any complete run of this head. --- _Generated with [`xum`](https://github.com/coder/xum) • Model: `anthropic:claude-sonnet-5-5` • Thinking: `xhigh`_
Why
This is a follow-up to #593. It fixes the Snacks close and the stop in
owned.lua, the two close paths that #593 left open.A job that was just started can still be a fork of Neovim until its child has run
exec. Ajobstopon it then ends Neovim withCaught deadly signal 'SIGHUP', or the buffer delete after it hangs. #593 documents the race, and #544 made the native close wait for the exec of a young job. The Snacks close and the stop inowned.luastill calledjobstopat once, so a close within milliseconds of the start could kill Neovim.Cause
pty_process_spawnforks the child, andinit_child()runssetsid()and resets the signal handlers only after that. Untilexecthe child has Neovim's libuv handlers. When the pty master closes in that gap (ajobstop), 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 inowned.lua, on 0.8.0 and 0.12.3. The native provider survived in every run.Fix
The wait moves out of
native.luaintoowned.wait_for_exec(jobid), withowned.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(ornvim_get_procwhere/procis missing) and waits up to 1 s until the child has another name.native.closecalls it for a job younger than 10 s, as before.snacks.M.openrecordsterm._cc.job_started_atwhere it already patches the instance.snacks.closereads 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 twopscalls of thenvim_get_procfallback.owned.luastop_bufferhas 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 runexec.The wait runs scheduled callbacks while the job still runs. Two Codex findings on
ea0fbfd6were about that, and12f1d45ffixes both:snacks.closenow clears the owned mark before the wait for every terminal, not only for an adopted one. Anopen()among the callbacks can no longer adopt the terminal that is closing by its mark.stop_bufferreads the channel before the wait and checksnvim_buf_is_valid(buf)beforejobstop. A callback that deleted the buffer during the wait no longer makesjobstopraiseInvalid buffer id.lua/claudecode/terminal/README.mdand theverify-terminal-lifecycleskill describe the three places. The skill changed as follows:OwnedStopOtherStopsreadsM.wait_for_exec(channel)and the buffer check beforejobstop.SnacksCloseClearsMarkreads the mark clear before the wait. The model has a switch of the same name inCloseBegin. The model counts a closing terminal as ended, so the fact breaks a rule only together withOwnedLiveSkipsEndedJob.check.sh --mutantshas a row for the pair.OwnedLiveSkipsEndedJobalone now breaksOpenShowsLiveClaude, notCloseNeverReuses, because the model clears the mark in every Snacks close.CI on
ea0fbfd6and the fixesTwo checks failed on
ea0fbfd6.TMPDIR="$RUNNER_TEMP/odd tmp 50%".LD_PRELOADsplits its value at a space or a colon, so the shim was ignored, the child was never held, and the probe timed out.f910015bputs the shim intomktemp -d /tmp/claudecode.XXXXXXwhen the TMPDIR path holds a space or a colon.verify-diff-lifecyclealso hashesterminal/{native,owned,snacks}.lua, so itscovered.lockwas stale.3a1961a0re-records the locks ofverify-terminal-lifecycleandverify-diff-lifecycle.Tests
tests/integration/terminal_close_young_job_spec.luaruns one Neovim per case with-u NONE(sominimal_init.luadoes not wait for the exec and hide the race) andtests/helpers/hold_pty_child.cpreloaded. The shim holds the child insideinit_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 plainjobstopas 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 theowned.luastop. The spec is pending without Linux, a C compiler andtimeout. It puts the shim into a/tmpfolder when TMPDIR holds a space or a colon, andtests/helpers/ld_preload_path.luamakes that choice.tests/unit/ld_preload_path_spec.luahas 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.luacovers the wait (names from/proc, fromnvim_get_proc, the 1 s limit, a job with no pid, unreadable names),is_young, the order of the stop inowned.lua(wait, thenjobstop, then delete) and "leaves a buffer alone that a callback deleted during the wait".tests/unit/terminal/snacks_close_spec.luacovers the Snacks close: it waits beforejobstopfor 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.vim.loop.hrtimein their vim mock, and one gainsvim.fn.getpid, because the provider now reads a clock and the wait reads the pid.proof-pty-child-hangup.shcompiles the same shim instead of a copy of it. Under a TMPDIR with a percent sign it failed all three cases, onmaintoo. 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. Nowd6019d13puts its work folder into/tmpfor such a path.hold_pty_child.cnow says that the shim holds every fork of Neovim that callssignal(SIGCHLD, SIG_DFL), including thepschildren thatnvim_get_procstarts, not only the pty child.Evidence
Builds: the official
v0.8.0release binary (NVIM v0.8.0-1210-gd367ed9b2, as in CI) and Neovim 0.12.3, scratch HOME and XDG dirs, on a shared host.owned.luacases fail, the control and native pass)TMPDIR="odd tmp 50%"ea0fbfd6times out in this TMPDIR, as in CItests/unit/terminalplusnative_terminal_toggle_spec, 20 runsMutants 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 intests/unit/terminalplusnative_terminal_toggle_spec. "Integration" is the new spec on 0.8.0 and 0.12.3, with the same result on both. "Fact" is theverify-terminal-lifecyclefact that turns FALSE in a facts-only run ofcheck.sh.SnacksCloseClearsMarkea0fbfd6)SnacksCloseClearsMarkSnacksCloseClearsMarkstop_bufferdoes not waitOwnedStopOtherStopsstop_bufferhas nonvim_buf_is_valid(buf)check beforejobstopOwnedStopOtherStopsstop_bufferre-reads the channel with no check (the form onea0fbfd6)OwnedStopOtherStopswait_for_execdoes not callvim.waitwait_for_execstops while the names are equalwait_for_execwaits 0 mswait_for_exechas no guard for an unreadable name of Neovimwait_for_execcallsjobpidwithoutpcall(no guard for a job without a pid)is_younguses<=is_youngreads anilstart time as young (nonilguard)The control mutant (no change) is green in every column, with 374 successes. I ran the facts-only
check.shon every row, and "none" means that every fact stayed TRUE. A mutant oftests/helpers/ld_preload_path.luathat drops the colon, and one that drops the space, each fail one case ofld_preload_path_spec.lua.The model has its own mutants.
check.sh --mutantsremoves each guard of the model in turn, and it ends with "every guard is caught" on the merge commit191a3e24. The pairSnacksCloseClearsMarkwithOwnedLiveSkipsEndedJobis caught byCloseNeverReusesandOpenShowsLiveClaude.MISE_LOCKFILE=false mise run clean && mise run allwith 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 fix(diff): answer a call that a close handler lets register while a diff is replaced #536, which needs a real Neovim.mise run format-check: 464 files, 0 changed.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.shpasses 3 of 3 cases on 0.8.0 and on 0.12.3, with a plain TMPDIR, withTMPDIR="odd tmp 50%"and (on 0.12.3) with a TMPDIR that holds a colon. Ond6019d13's parent, with the odd TMPDIR, it failed 3 of 3 on both builds.check.shscripts (verify-diff-lifecycle,verify-mention-queue,verify-selection-tracking,verify-terminal-lifecycleandverify-ws-connection) all print PASS. The branch mergesorigin/main(test: pin the termopen wrapper's default bindings and correct its notes #597, fix(scripts): make mise run format-check write nothing #594, feat(skills): add verify-ws-connection, a TLA+ model of the WebSocket connection lifecycle #474, test(ci): make the workflow spec read only a bare or a fully quoted key #596 and fix(diff): answer a call that a close handler lets register while a diff is replaced #536). fix(diff): answer a call that a close handler lets register while a diff is replaced #536 changeddiff.lua, its README,verify-diff-lifecycle(SKILL.md,check.shandcovered.lock) and addeddiff_answer_every_call_spec.lua. Git merged the two edits ofverify-diff-lifecycle/covered.lockwith no conflict, andcheck.shof all five skills prints PASS on the merge result, so no lock needed a new record after3a1961a0.191a3e24, the specs this PR adds or changes pass 20 of 20 runs in a plain TMPDIR and inTMPDIR="odd tmp 50%"(24 successes forowned_wait_for_exec_spec,snacks_close_specandld_preload_path_spec, and 374 for the terminal unit specs plusnative_terminal_toggle_spec). The integration spec of this PR passes 20 of 20 runs per build in both TMPDIRs.diff_answer_every_call_specfrom fix(diff): answer a call that a close handler lets register while a diff is replaced #536 passes 10 of 10 runs per build in both TMPDIRs (9 successes each run).Not covered
snacks.nvim. The provider code under test is the real one, and the stand-in starts a real pty job withtermopen./procorps. Where neither gives a name, the close goes on tojobstopand the race stays open, as in native before this PR. Apsthat hangs would hang the close for as long as it hangs. I did not reproduce that.psfallback only on Linux, with/prochidden from Lua. I did not run it on macOS or BSD.jobstopthat other code issues, for example:bwipeoutof the terminal buffer by the user right after the start, is not covered.native.lua,snacks.lua,owned.luaor acovered.lockmust be merged one at a time with this one, and re-record the lock after its merge.Generated with
xum• Model:anthropic:claude-sonnet-5-5• Thinking:xhigh