Repository navigation
fix(diff): answer a call that a close handler lets register while a diff is replaced - #536
Conversation
…out an answer Two orders left an openDiff without an answer. A ClaudeCodeDiffClosed handler that runs the event loop lets a queued openDiff register for a tab name while openDiff replaces the old diff of that name. The replace step then cleaned up the registered diff of the queued call, and nothing answered it. The replace step now repeats until no diff is registered for the tab name, so each diff it removes is answered as rejected. A plugin reload inside a setup left the diff of that setup in the old module. The accept, deny and close-all commands use the new module, so none of them could answer it. The old module now registers the diff in the new module too, and rejects a pending diff that the new module holds for the tab name first. _Generated with xum • Model: anthropic:claude-sonnet-5-5 • Thinking: xhigh_
The cases queue a second call inside a ClaudeCodeDiffClosed handler, and run a plugin reload inside a setup, in both layouts. Each checks that every call gets one answer with its own text and that no tab or window stays. Two controls keep the :w path of the old module and a diff that is already answered. _Generated with xum • Model: anthropic:claude-sonnet-5-5 • Thinking: xhigh_
…README The module code checks the type of the loaded module before it adopts a diff. _Generated with xum • Model: anthropic:claude-sonnet-5-5 • Thinking: xhigh_
…the two orders it leaves unmodeled The replace guard of open_diff_blocking is now a loop, so the ReplaceRejects and ReplaceKeepsOthersSaved facts read `while existing_diff do`. The model needs no new action: a loop pass is one replace. _Generated with xum • Model: anthropic:claude-sonnet-5-5 • Thinking: xhigh_
The model passes, and --mutants shows every guard caught. The ReplaceRejects and ReplaceKeepsOthersSaved facts read the new loop. The adoption after a reload and a queued call that registers during a replace stay outside the model, and the skill says so. _Generated with xum • Model: anthropic:claude-sonnet-5-5 • Thinking: xhigh_
_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. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 823735dbcb
ℹ️ 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".
🛡️ 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. |
…-call # Conflicts: # lua/claudecode/README.md
When a plugin reload interrupts a setup and the new module already holds an answered diff for the tab name, the old module kept the late diff only in its own table. No command of the new module could answer it. The old module now rejects that diff and cleans it up, and the case runs the real order. _Generated with xum • Model: anthropic:claude-sonnet-5-5 • Thinking: xhigh_
…ule rejects _Generated with xum • Model: anthropic:claude-sonnet-5-5 • Thinking: xhigh_
…dopt _Generated with xum • Model: anthropic:claude-sonnet-5-5 • Thinking: xhigh_
|
@codex review |
|
@codex security review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 508d1002bc
ℹ️ 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".
🛡️ 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. |
…pts a late one, and skip its open event A reload during a setup leaves the new module with a pending diff for the tab name, and rejecting that diff fires ClaudeCodeDiffClosed. A handler that runs the event loop can let a third call register then, and the adoption overwrote it. The adoption now repeats the replace step like openDiff does. When the old module rejects a late diff that the new module cannot adopt, the setup also stops before it fires ClaudeCodeDiffOpened for a proposal that is gone. _Generated with xum • Model: anthropic:claude-sonnet-5-5 • Thinking: xhigh_
…no open event for a late diff Record covered.lock for the loop in _adopt_diff_state and the early return in the setup. _Generated with xum • Model: anthropic:claude-sonnet-5-5 • Thinking: xhigh_
|
@codex review |
|
@codex security review |
…-call # Conflicts: # .agents/skills/verify-diff-lifecycle/covered.lock
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1f3cce5614
ℹ️ 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".
🛡️ 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. |
…le cannot adopt it When a handler reloads the plugin again while the second module rejects the pending diff for the late diff's tab name, the second module registers the late diff in a third module. A third module that keeps an answered diff for the tab name makes the second module reject and clean up the late diff. The second module still reported the diff as adopted, so the original module kept the rejected entry and its setup fired ClaudeCodeDiffOpened for a closed diff. Registration now reports whether the diff stays registered. A module that cannot hand the diff over forgets its own entry and returns false, and only the module that owns the setup rejects and cleans up, so ClaudeCodeDiffClosed fires once. _Generated with xum • Model: anthropic:claude-sonnet-5-5 • Thinking: xhigh_
The change touches only the replace step of open_diff_blocking, and the code facts ReplaceRejects and ReplaceKeepsOthersSaved read the loop. All code facts stay TRUE. _Generated with xum • Model: anthropic:claude-sonnet-5-5 • Thinking: xhigh_
# Conflicts: # .agents/skills/verify-diff-lifecycle/covered.lock
|
@codex review |
|
@codex security review |
|
Codex Review: Didn't find any major issues. Hooray! 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. |
# Conflicts: # .agents/skills/verify-diff-lifecycle/covered.lock
# Conflicts: # .agents/skills/verify-diff-lifecycle/covered.lock
|
Maintainer decision (ThomasK33, 2026-11-13, in the agent chat): take the recommended option for this PR. Waiver recorded before the request: one merge-only push that brings in current main and re-records Generated with |
# Conflicts: # .agents/skills/verify-diff-lifecycle/covered.lock
…ge of main Only the open_blocking block changed against main's lock: the replace step of open_diff_blocking is a loop. All code facts stay TRUE. _Generated with xum • Model: anthropic:claude-sonnet-5-5 • Thinking: xhigh_
|
@codex review |
|
@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. |
_Generated with xum • Model: anthropic:claude-sonnet-5-5 • Thinking:
…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`_
…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 Neovim refuses window changes in two states: while the user is in the command-line window (`q:`, E11) and under textlock (E565, for example while an `<expr>` mapping waits for a key). coder#499 holds the disconnect cleanup in those states. Five other paths still tear a diff down at that moment and raise: - the server stops (`:ClaudeCodeStop`, or the plugin stops itself), - Claude calls `close_tab`, - Claude calls `closeAllDiffTabs`, - a second Claude client sends `openDiff` with the `tab_name` of an open diff (a replacing openDiff), - a bulk close of many diffs. What the user sees on `main` at `d8e767c` (coder#499): `stop` raises E11 or E565 before the TCP teardown, a pending openDiff call is never answered, the server keeps running, and the next start says "Already running". A replacing openDiff raises in `diffoff`, and the old rejected diff stays on screen after the user leaves the window. The unified layout is worse: nothing raises, but the registry entry is dropped while its windows stay, so nothing can close them. `main` has moved since (`700209ee`, with coder#485 and coder#536), and a replacing openDiff in the command-line window now behaves differently there. On Neovim 0.12.3 it no longer raises in `diffoff`. It drops the registry entry and leaves the old windows, and the new call gets "Diff setup failed". On v0.8.0 the cleanup still raises E11, and the old rejected diff stays registered and on screen. This PR's `diff_cleanup_locked` spec fails 15 of 24 cases on `700209ee` with 0.12.3 and 14 of 24 with v0.8.0. ## What changed - `_cleanup_diff_state` now hands the teardown to `utils.when_windows_allowed`, the gate from coder#499. The old body is `teardown_diff`, and only the gate calls it. When Neovim allows window changes, it runs at once, as before. Otherwise the diff stays registered and the gate's one 50 ms timer runs the teardown later. - The held teardown runs only while `active_diffs[tab_name]` still holds the entry that waited, because a new diff can take the tab name before the timer fires. - The answer to each waiting call goes out before the teardown, so the server stop, `close_tab`, `closeAllDiffTabs` and the bulk closes answer every call at once. - Many held cleanups share the gate's one timer. A spec pins this with 20 held diffs. - A replacing openDiff that finds its old diff held by the gate fails at once with error `-32000` ("Diff setup failed") and runs no setup. A waiting old call gets `DIFF_REJECTED` (an old diff the user already accepted keeps its `FILE_SAVED` answer), and the held teardown closes the old diff after the user leaves the locked state. - `.agents/skills/verify-diff-lifecycle`: the model has a lock on window changes. Three facts (`CleanupHoldsWhenLocked`, `CleanupRetries`, `RetryChecksEntry`) read the gate and `teardown_diff`, and the liveness rule `DeferredCleanupDone` says a held cleanup runs once windows can change. After the merge the model also holds coder#536's reject pass and exit cleanup, about 256,000 states. - `.agents/skills/verify-claudecode-nvim`: two journeys, `journey-diff-disconnect-cmdwin.sh` and `journey-diff-replace-cmdwin.sh`, drive the paths in a real Neovim terminal with the scripted Claude client. ## Merge of main This head merges `origin/main` at `700209ee` (coder#485 is in) into the branch with a normal merge commit (`9baedc48`). No rebase, no force push. Eight files conflicted. I kept main's code in each one and put this PR's gate around it. | File | Resolution | | --- | --- | | `lua/claudecode/diff.lua` | Kept main's `end_diff_mode_if_alone`, `end_diff_mode_in_windows_that_show_the_file`, `release_surviving_diff_tab` and the `diffoff` changes. `teardown_diff` now holds main's whole body, including the new-tab release from coder#485. `_cleanup_diff_state` still calls the gate. The replace loop of coder#536 asserted that the old entry is gone after `_cleanup_diff_state`. With the gate the entry can stay while windows are locked. The merge turned the assert into a `break`, and a verifier found that this was wrong (see "Finding F1" below). `ebff0dc2` replaced the `break` with the call that fails at once. | | `lua/claudecode/README.md` | Kept main's bullets and this PR's gate bullet. The bullet on a refused `:diffoff` now says the cleanup waits for the gate. | | `verify-diff-lifecycle/model/DiffLifecycle.tla` | Kept both sides: main's `RejectEach`, `DiffCleanupAtExit`, `ExitInSetup` and the exit constants, and this PR's `DeferredCleanupRuns`. `Next` lists all of them. | | `verify-diff-lifecycle/check.sh` | The rules end with main's `ExitedCallsAnswered`. The live properties are `DisconnectedCallsAnswered` and `DeferredCleanupDone`. | | `verify-diff-lifecycle/model/statechart-view.ts` and `SKILL.md` | Both sides kept. The notes state the merged model's size and the replace loop's exit. | | `verify-claudecode-nvim/SKILL.md` | Main's journey list plus this PR's two journeys. | `covered.lock` of `verify-diff-lifecycle` came from main and I re-recorded it in its own commit. It was the only STALE lock of the five. Two more re-records followed, one for a comment-only edit of `diff.lua` and one for the replace-loop fix. The model's facts still match the code, and the model never lets a replacing openDiff find windows locked (its `SKILL.md` says what the code does there). Every teardown path that main added since `ef610b7d`: | Path | Gate | | --- | --- | | coder#485 new-tab release (`release_surviving_diff_tab`) | Behind the gate. `teardown_diff` calls it, directly in the split layout and through `cleanup_inline_diff` in the unified layout, and nothing else calls it. | | coder#573 and the split-close helpers (`_close_proposal_window`, `end_diff_mode_in_windows_that_show_the_file`) | Behind the gate through `teardown_diff`. | | coder#536 exit cleanup (`cleanup_at_exit`) | The reject pass resolves statuses, and its one window change (`keep_empty`) calls `_cleanup_diff_state`. The cleanup pass calls `_cleanup_all_active_diffs`. Both go through the gate. | | coder#536 replace loop in `open_diff_blocking` | Calls `_cleanup_diff_state`, so it is gated. When the gate holds the old diff, the loop fails the call before any setup. | | Setup-failure cleanup and `remove_failed_proposal` | No gate for the setup's own leftovers (the branch for a diff that the setup already registered goes through the gate). They run only for a setup that failed, and they tell a diff that the setup registered from one that was already registered only by `active_diffs[tab_name]`. The merge's `break` broke that assumption (F1). The replace loop now fails the call before its setup runs, so these handlers see only an entry that the setup registered, or none. | ## Finding F1 (found by a verifier on `0b34b13c`, fixed in `ebff0dc2`) The merge turned coder#536's assert into a `break`. A replacing openDiff B then ran its setup while the old diff A was still registered under the same `tab_name`. When that setup failed in a locked state, its failure handlers saw A's entry. They held A's teardown a second time and skipped B's own cleanup. What B's setup had made stayed behind: - on Neovim 0.8, a window that shows the `[Command Line]` buffer after the user leaves `q:`, - a hidden proposal buffer that is marked modified, so `:qa` stops with E37 on v0.8.0, - a buffer name that made every later openDiff of that kind and tab name fail with E95 ("Failed to rename buffer" on v0.8.0). It needs a second openDiff with the `tab_name` of an open diff while the user is in `q:` or under textlock. PROTOCOL.md says Claude Code sends a new random `tab_name` per call, so only other clients that reuse a `tab_name` reach it. Fix: when the gate kept A's entry, the replace loop fails B's call with `-32000` ("Diff setup failed") before any setup. I used this form and not the verifier's alternative (the failure handlers compare the entry with the one registered before the setup). Every setup fails in those states anyway, so the first form runs nothing that would need undoing. It changes one place, where the alternative changes the failure handlers in `diff.lua` and in `diff_inline.lua` and still runs a setup that cannot work. The loop holds A's teardown once. B's setup used to hold it a second time. ## Known limit `close_tab` of a diff the user already saved still raises E11 in those states. The file reload that runs 100 ms after the close (`reload_file_buffers_manual` from `close_diff_by_tab_name`) does not go through the gate yet. I re-ran it on this head on 0.12.3 and on v0.8.0, in the command-line window (E11) and under textlock (E565). The reload raises from `edit`, and the file's buffer keeps its old text, because the reload never runs. The diff itself closes after the lock. `main` is no better: its `close_tab` itself fails in those states, the diff stays registered, and the reload raises the same way. A follow-up PR routes the reload through the gate. A replacing openDiff in those states holds the old diff, and the new call fails with error `-32000` ("Diff setup failed") before it sets anything up, so it leaves no window or buffer. The old diff closes once the user leaves the window. Holding the new openDiff itself is not part of this PR. ## Tests I used the `test-audit` authoring gate, `principle-test-behavior-not-implementation` and `tdd` (failing check before the fix, then pass after) for every test change after the merge: - New `diff_replace_locked_spec.lua` (24 cases on each build) drives a replacing openDiff through `tools.handle_invoke` in the command-line window and under textlock, in the split and unified layouts, with the old and the new file of different kinds (existing and new, both orders), while the user is in another tab. Per combination it checks three behaviors. The old call gets `DIFF_REJECTED` once, the new call gets one `-32000` answer, and the old diff stays registered while locked. After the lock the old diff closes, no window shows `[Command Line]`, no proposal buffer is left and no window is left over. A later openDiff with the same tab name and the new call's kind opens and is answered once. On `0b34b13c` 10 of the 24 cases fail on both builds (the command-line window, the leftover proposal buffer, and the later openDiff). With the fix all 24 pass on 0.12.3 and on v0.8.0. - `diff_left_window_spec.lua`, case "answers close_tab from the command-line window...": it pinned the pre-gate behavior (the copy keeps its diff mode). It now expects the diff to stay registered while the window is open, to close after the window closes, and to end diff mode in the copy afterwards. The `no gate` mutant turns it red. - `diff_locked_paths_spec.lua`, replacing openDiff cases: the old assertion looked for `"error"` in the reply. The mutant that restores coder#536's "entry is gone" assert in the replace loop survived it (0 red). The assertion now looks for "Diff setup failed", and the same mutant turns 2 cases red (command-line window and textlock). - `diff_cleanup_locked_spec.lua`: I removed its skip of the unified layout below Neovim 0.9. Main dropped that claim in coder#564, so all 24 cases now run on v0.8.0. - `diff_locked_paths_spec.lua` counts `vim.defer_fn` calls and uses `nvim_replace_termcodes` through `tests/helpers/window_lock.lua`, because v0.8.0 has no `vim.keycode` and `uv.walk` crashes there. - `journey-diff-replace-cmdwin.sh` no longer sends Enter at a "Press ENTER" prompt. The refused call now logs nothing, and Enter would run the command line and close the window. ## Verification On `d8e767c` I replayed each path over a real socket in headless Neovim 0.12.3 (the textlock cases use `getchar()` in a typed `<expr>` mapping). Result after the change: | Path | `main` | this branch | | --- | --- | --- | | stop in the command-line window | `stop` raises E11, server keeps running, restart says "Already running" | stop returns true, both calls answered, both diffs close after the window, restart works | | stop under textlock | raises E565, same leftovers | same as above | | replacing openDiff, command-line window | error names `diffoff`, old rejected diff stays | new call gets -32000 before its setup, old diff closes after the window | | replacing openDiff, textlock | same | same | | 30 held cleanups | `close_pending_diffs` raises E11 after the first | 30 diffs close, 1 timer armed | Replacing openDiff, all combinations (a verifier's matrix, rerun by me on this fix and on `700209ee`, headless, one fresh Neovim per row): 288 scenarios on each of 0.12.3 and v0.8.0, 576 rows. Scenario axes are layout (vertical, horizontal, unified), new-tab diff or not, lock (`q:`, textlock, none), user in the same or another tab, old and new file existing or new, and the old diff pending or saved. Each row also opens a later openDiff, accepts it and closes its tab. - No row is worse than `700209ee`, and 376 rows are strictly better. The 384 rows with a lock have no leftover. The other 192 rows have no lock and give the same result line as `700209ee` in every row. - `0b34b13c` is worse than `700209ee` in 40 rows (a row can count in more than one: 24 with a later openDiff that fails, 16 with a leftover buffer, 8 with an extra window), and none of them is worse on this head. - With the new call arriving through `vim.schedule` (the way a socket message arrives), the result is the same: 0 worse rows, 376 better. - coder#536's close-handler probe (224 runs) and split-path probe (320 runs) give identical output on `700209ee` and on this head in every run. The locked close-handler orders (448 runs): 0 problems on this head, 358 on `700209ee`, none worse. - The verifier's standalone repro (user in another tab, textlock, new file, then two later new-file openDiffs) answers both later calls with E95 on `0b34b13c` and opens both on this head, as `700209ee` does, on 0.12.3 and on v0.8.0. Gates on the merged head `74bbd458`, one run each, none passed only on retry: - `mise run clean` and `mise run all`: exit 0, 2026 successes, 0 failures, 0 errors, 54 pending, luacheck 0 warnings in 227 files, and all 5 static proofs ok. The new spec is one more pending case under busted, because it needs a real Neovim. - Twelve specs, each run in real Neovim on 0.12.3 and on v0.8.0, native exit 0 each time: `diff_cleanup_locked` 24 cases, `diff_locked_paths` 7, `diff_disconnect_window_lock` 2, `diff_new_tab_autocmds` 14, `diff_new_tab_close_mode` 10, `diff_new_tab_last_tab` 12, `diff_new_tab_mode` 12, `utils_without_autocmds` 4, `diff_answer_every_call` 9, `diff_close_save` 77, `diff_left_window` 40 and `diff_replace_locked` 24. All 0 failures and 0 errors. The first two and the last are this PR's specs. The other nine cover the new-tab, answer-every-call, close-save and left-window teardown code that main changed, and the disconnect window lock from coder#499. - `scripts/run_integration_tests_individually.sh` with the stub `claude` first on `PATH`: 1161 successes on 0.12.3 and 1157 on v0.8.0, 0 failures, 0 errors, exit 0. Each ran twice, once with a plain `TMPDIR` and once with a `TMPDIR` that holds a space and a percent sign. - `check.sh` of all five skills: PASS. States: `verify-diff-lifecycle` 255,881 (the two live properties 13,509 each), `verify-mention-queue` 540,240, `verify-selection-tracking` 95,938, `verify-terminal-lifecycle` 279,492, `verify-ws-connection` 262,285. The three fact mutants of the merged model fail as expected (run before the F1 fix, which does not touch the model): `CleanupHoldsWhenLocked=FALSE` breaks `NoRejectedLeftover`, `CleanupRetries=FALSE` breaks `DeferredCleanupDone`, and `RetryChecksEntry=FALSE` breaks `NoStrandedCall`. - `journey-diff-disconnect-cmdwin.sh` and `journey-diff-replace-cmdwin.sh` both print PASS on this head in a real Neovim terminal, and `vcn.sh cleanup` leaves no process. They read `OUT` and `WORK` through `vcn.sh get`, because main's `no-source-run-env` static proof refused the `source` of `run.env`. - Eight mutants, each run on the five spec files that cover the gate and the replace loop (`diff_cleanup_locked`, `diff_locked_paths`, `diff_disconnect_window_lock`, `diff_left_window`, `diff_replace_locked`). The count is red cases on 0.12.3 and on v0.8.0, and a no-op control is green on both: - no gate (`_cleanup_diff_state` calls `teardown_diff` directly): 38 and 35, - no identity check (the held function drops the `active_diffs[tab_name] == diff_data` test): 12 and 12, - own timer per cleanup (`when_windows_allowed` arms a `vim.defer_fn` on every call): 3 and 3, - held function never runs (the retry loop walks an empty list): 39 and 39, - entry dropped while held (the entry is removed before the gate holds the teardown): 39 and 37, - probe always allows (`windows_allowed` returns true): 40 and 37, - replace loop breaks (the merge's form, the cause of F1): 10 and 10, all in `diff_replace_locked`, - replace loop asserts the entry is gone (coder#536's assert): 2 and 2, both in `diff_locked_paths`. The counts depend on the exact edit, so they are not comparable with the counts of an earlier head. - Earlier on `d8e767c`: 6 of the 7 `diff_locked_paths_spec.lua` cases and 16 of the 24 `diff_cleanup_locked_spec.lua` cases fail there. `journey-diff-replace-cmdwin.sh` fails there at the `diffoff` step. On `700209ee` this PR's specs fail 15 and 14 of the 24 `diff_cleanup_locked` cases (0.12.3 and v0.8.0), 6 of the 7 `diff_locked_paths` cases on both, and 16 and 14 of the 24 `diff_replace_locked` cases. Refs coder#483, coder#485, coder#499. coder#485 is merged, and this PR resolves the overlap in `_cleanup_diff_state`. --- _Generated with [`xum`](https://github.com/coder/xum) • Model: `anthropic:claude-sonnet-5-5` • Thinking: `xhigh`_
Why
Claude waits for an answer to every
openDiff. When the plugin never answers a call, Claude waits forever. One order onmainleaves a call without an answer. A handler of the user runs the event loop (vim.wait), so that Claude's queued second call for the same tab name runs inside it. The verifiers of #512 found this order while they checked a candidate. That candidate newly reaches it, butmainhas it without any of its code, so I fix it here first.A close handler. An
openDiffA for a tab name replaces the open diff X. AClaudeCodeDiffClosedhandler of the user runs the event loop while X closes, so a queued call B registers a diff for the tab name. When X is a new file, A then cleans up the diff that is registered for the tab name, and that is B's. Nothing answers B, and its tab or windows can stay. When X is an existing file, B can instead stay open with no answer, or A fails with E95. When X is an existing file and B shares A's proposal name, A fails with E95 as well, and A's failure cleanup removes B the same way.What changed
open_diff_blockingis a loop. It rejects the diff that is registered for the tab name (if it is pending) and cleans it up, as before, and repeats while another diff is registered. It cleans up only the diff that it handled, so the replace step never removes a diff that registered meanwhile without an answer (aWinClosed,BufWipeoutorBufHiddenhandler can still do so, see "Not in this PR"). Each pending call that A replaces gets oneDIFF_REJECTEDanswer, B included, and A sets up on a free tab name, except in theaccept_otherorder, where A is refused with "Diff tab in use" and the accepted diff stays. The change is 11 added and 5 removed lines inlua/claudecode/diff.lua.tests/integration/diff_answer_every_call_spec.luahas 9 cases. Four of them run the close handler order in both layouts, with X a new file and with X an existing file. Five pin whatmaindoes today after a plugin reload inside a setup (see "A reload" below).lua/claudecode/README.mdand theverify-diff-lifecycleskill describe the loop. The skill lists the order as not modeled, itsReplaceRejectsandReplaceKeepsOthersSavedfacts readwhile existing_diff do, andcovered.lockhas the new hashes.A reload. An earlier version of this PR also registered a diff in the new module after a plugin reload inside a setup. I removed that step. A reload inside a setup behaves as on
main, with and without the event loop in the reload: the diff finishes its setup in the old module, so:ClaudeCodeDiffAccept,:ClaudeCodeDiffDenyand:ClaudeCodeCloseAllDiffsof the new module do not reach it.:w,:qand:bwipeout!in its window still answer it, and the user can end it from there. This PR does not promise oneClaudeCodeDiffClosedper diff after a reload, becausemaindoes not either.Every order that the scenarios below cover, except the close handler order, ends as it does on
main.Evidence
Real headless Neovim, one instance per scenario,
tools.handle_invokeand the real commands, a stub in place of theclaudeCLI. The rows below come fromc7d4f61b, a head before the last three main merges whose replace loop is identical, andorigin/mainat48ca93be. On the final headb001b741, which mergesorigin/mainatf28eba7d, an independent verifier reran the close handler probe (0 of 224 runs with a problem, 220 of 224 onmain) and a probe of the newly merged split and CRLF paths. No row is worse thanmain. Neovim 0.12.3 and v0.8.0 give the same rows.mainmainmainSpec. 9 cases. On
main, 4 fail and 5 pass, on both Neovim versions. The 4 that fail are the close handler order:answers a call that a ClaudeCodeDiffClosed handler lets register while the old new-file diff is replacedand the same title withexisting-file, each in the vertical and the unified layout. The 5 that pass are the controls::win A's window after a reload answers A once, and:qor:bwipeout!on a new-file proposal after a reload, with and without the event loop in the reload, fires one open and one close event, also afterclose_taband close-all. This head passes all 9.Mutation. 6 mutants of the loop, all red on both Neovim versions: the replace step runs once, it cleans up what it did not handle, it does not read the registry again, it does not reject a pending diff, it does not clean up, and it rejects only on the first pass. Removing the
assertafter the cleanup changes no result, because it only states what the cleanup already did.Reload equals
main. The verifier's N1 spec (a reload inside A's setup, a queued call B in the new module, and a close handler that closes A's window) gives the same rows onmainand on this head, on both versions (2 cases, both pass). The verifier's D1 probe (a reload inside A's setup, then:q,:bwipeout!, deny or write in A's window, thenclose_tab; 16 runs per tree) gives the same 16 rows on both trees.Other orders. I ran 776 scenarios on
mainand on this head, on both versions (1552 runs per tree): the 657 scenarios of the first verifier of #512 (the core set, the event matrices with and without notifications, three-deep, three queued calls, the window orders, the V3 orders and the reload-after-action orders), the reload, I1, V2, W1, N1, three-deep and three-queued scripts of the second verifier and its 90-scenario harness, and my own close handler orders. 1538 runs give the same output on both trees. 14 differ, which are 7 scenarios on each version, and all 7 are the close handler order: the four split and unified orders without a leftover buffer, the two unified orders with one, andf3_diffclosed_pump. In each,mainleaves a call unanswered and this head answers every call once with its own text and leaves no extra window or tab.Gate on the final head (
b001b741, which containsorigin/mainatf28eba7d, #558 and #573 included):mise run clean && mise run allexit 0 (1904 successes, 0 failures, 37 pending, luacheck 0 warnings in 201 files). The spec runs 9 of 9 on Neovim 0.12.3 and on v0.8.0. The #558 specs pass too:tests/unit/diff_exit_reject_spec.lua6 of 6, andtests/integration/diff_exit_reject_spec.lua34 of 34 on both versions. The #573 specsdiff_close_save_spec(77 of 77) anddiff_left_window_spec(40 of 40) pass on both versions. The integration runner passes with 865 successes and 0 failures on Neovim 0.12.3 and 861 and 0 on v0.8.0, each also under aTMPDIRwith a space and a%.check.shPASS (93477 and 5557 states) and--hash-onlyis fresh for all five verify skills. The 6 mutants of the loop are red on both versions. Onmainthe spec fails 4 of 9 cases and passes 5.Not in this PR
WinClosed,BufWipeoutorBufHiddenhandler that runs the event loop while the diff X is cleaned up (not in theClaudeCodeDiffClosedevent) also lets B register. The last step of_cleanup_diff_statethen removes B without an answer in some variants (aBufWipeoutorBufHiddenhandler on a new file still answers B), and X'sClaudeCodeDiffClosedfires twice, onmainand on this head. A guard on that step reaches every cleanup, not only a replace, so it changes more orders than the one here.openDiffthat registers while the first one is still inside its setup in the same module, without a reload: B is never answered and an extra tab can stay, onmainand on this head. fix(diff): rename a diff buffer that a plugin reload orphaned #512 handles it with a refusal.accept_otherorder, one SKILL.md line, aFileTypehandler that closes the proposal window during setup, and a call that registers during exit) are tracked in Deferred follow-ups from the 2026-11 fix run #603. The last two are on main too.Generated with
xum• Model:anthropic:claude-sonnet-5-5• Thinking:xhigh