Repository navigation
fix(diff): close a new-tab diff without entering its tab - #485
Conversation
…t starts in the user's file Claude's close_tab made the diff tab current before it ran :tabclose. When the current window of that tab showed the Claude terminal (the user focused it and left terminal mode with <C-\><C-N>, or went to another tab), entering it ran the terminal buffer's BufEnter handlers. The diff's own handler and the Snacks auto_insert handler queue a startinsert, and the close then moved the cursor to the user's file, where the insert started. The next keys typed into the file. Close the tab by its number from the tab the user is in, in the split and the unified layout. The new tests fail on main with mode 'i' instead of 'n'. _Generated with xum • Model: anthropic:claude-sonnet-5-5 • Thinking: xhigh_
…nal-normal mode journey-new-tab-diff-close-mode.sh drives a real Neovim in agent-tty. It focuses the Claude terminal in the diff tab, leaves terminal mode, lets Claude call close_tab, and checks that the user's file is in Normal mode and that the next key does not edit it. It repeats this with the user back in the first tab. _Generated with xum • Model: anthropic:claude-sonnet-5-5 • Thinking: xhigh_
|
@codex review |
|
@codex security review |
Codex Review Summary: ✅ CompletedThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
Codex Review: Didn't find any major issues. 🎉 Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
🛡️ 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. |
…ument the close events When the user closed their other tabs, the diff tab was the last tab page, and `:tabclose` refused with E784. The cleanup then deleted the proposal buffer, the cursor fell into the Claude terminal, and the diff's BufEnter handler queued a startinsert, so the user ended in terminal mode and the handler stayed. Release such a tab by construction, in the split and the unified layout: close the terminal window the diff added (its handler goes with the window), turn off diff mode in the file window, and keep the cursor there. A unified diff window gets an empty buffer. The new real-Neovim cases fail before this change with mode 't'. Also: the journey and its feature step name fresh request ids, the journey loads snacks with VCN_RTP, and lua/claudecode/README.md states which events a close fires now. _Generated with xum • Model: anthropic:claude-sonnet-5-5 • Thinking: xhigh_
|
@codex review |
|
@codex security review |
|
Codex Review: Didn't find any major issues. Another round soon, please! 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. |
…stays open, and split the new-tab specs When `:tabclose` cannot close the diff tab (it is the last tab page, or 'nohidden' and a modified buffer of the user's stop it), a window of the user's that shows the file kept 'diff', scrollbind and foldmethod=diff. That happens with a `:split` of the diff's file window, and for every file window when 'diffopt' lacks closeoff. The release now runs `:diffoff` in each window of the tab that shows the diff's file buffer. diff_new_tab_mode_spec.lua took about 24 s on CI against the runner's 30 s per file. The shared child-Neovim harness moves to tests/helpers/new_tab_diff_child.lua, and the cases split into three files: the open, the close, and the tab that stays open. lua/claudecode/README.md now says exactly which events a close fires: a terminal that was visible in the user's tab at open and is hidden at close is shown again, which fires WinLeave, BufLeave and BufEnter on the file, and it names E37 next to E784. _Generated with xum • Model: anthropic:claude-sonnet-5-5 • Thinking: xhigh_
|
@codex review |
|
@codex security review |
🛡️ Codex Security ReviewSecurity review completed. No security issues were found in this pull request. Reviewed commit: Only the user who started this review can view the report in Codex. ℹ️ About Codex security reviews in GitHubThis is an experimental Codex feature. Security reviews are triggered when:
Once complete, Codex will leave suggestions, or a comment if no findings are found. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9ac83f7e82
ℹ️ 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".
…pen, and state the close events exactly When the diff tab is the last tab, the release acted on the stored proposal and terminal windows without checking what they show. A user who switched one of them to a file of their own lost that view: the proposal window got an empty buffer, or the terminal window closed. The release now touches a window only while it still shows the buffer the diff put there, and it drops the terminal's BufEnter handler by id, so the handler goes when the user keeps the window. The record of a diff gets terminal_buffer_in_new_tab and terminal_enter_handler. lua/claudecode/README.md states the events of a close for every position of the user: in the original tab, in the diff tab or a third tab (the close enters the original tab, as before this change), with the original tab gone, and with the terminal hidden. _Generated with xum • Model: anthropic:claude-sonnet-5-5 • Thinking: xhigh_
The merge moves the ten tests of #489, which edit diff_new_tab_mode_spec.lua, into tests/integration/diff_new_tab_original_tab_spec.lua on the shared harness, with its hide_original_terminal option and the tab number of the child's state.
…with 'diffopt' lacking closeoff, E37, the user's own windows The E37 cases move to their own file with the cases that pin the release: with the terminal on the left, :tabclose stops before it closes the terminal window, and the file window keeps 'diff'. The user-windows cases also check that the cursor stays in the user's window. _Generated with xum • Model: anthropic:claude-sonnet-5-5 • Thinking: xhigh_
… another window is left _Generated with xum • Model: anthropic:claude-sonnet-5-5 • Thinking: xhigh
…aves no diff mode in the proposal window Covers the Codex finding on 9ac83f7 about a surviving proposal window left in diff mode when 'diffopt' lacks closeoff. Neovim 0.8.0, 0.9.5 and 0.12.3 all turn diff mode off in a window whose buffer is swapped, so no code change is needed. Also drop a comment that restated a helper's return value. _Generated with xum • Model: anthropic:claude-sonnet-5-5 • Thinking: xhigh_
…window stops the close with 'nohidden' Covers the Codex finding on 9ac83f7: with 'nohidden' and a modified buffer of the user's in the diff tab's terminal window, `:tabclose` and the window close both fail, and the handler used to stay. Both layouts. The cases fail on 9ac83f7 and pass here, and they go red when the release stops deleting the handler by id. _Generated with xum • Model: anthropic:claude-sonnet-5-5 • Thinking: xhigh_
…er window of the last diff tab When the diff tab is the last tab and the user closed the proposal and the file window, the Claude terminal is the only window. `:tabclose` fails with E784 and the window close fails with E444. Main and 9ac83f7 left the diff's BufEnter handler registered. The release now deletes it by id, so the handler goes in both layouts. The terminal window stays, and the provider opens a second window for the terminal as it does on main. _Generated with xum • Model: anthropic:claude-sonnet-5-5 • Thinking: xhigh_
The merge brings in #490, which changes DEVELOPMENT.md only. _Generated with xum • Model: anthropic:claude-sonnet-5-5 • Thinking: xhigh_
|
@codex review |
|
@codex security review |
|
Codex Review: Didn't find any major issues. 🎉 Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
|
Round-7 result: blocked under the recorded waiver's stop rule. No 8th push is made. The fresh verifier on d00e1be found a newly reached break compared with main (F1). A user The cause is in the provider's terminal swap: Generated with |
|
Maintainer decision (answer in the run's chat, recorded here before the push): round 8 is waived for one push. That push only merges current main into this branch after the provider fix PR #518 has merged, re-records Generated with |
… need (coder#514) ## Why CI's "Install build dependencies" step failed twice on 2026-11-11 when the Ubuntu mirrors could not be resolved: run 43034439048 (merge group for coder#513) and run 43033301588 (coder#485, both attempts), between 13:53 and 14:22 UTC. In run 43034439048, `apt-get update` printed `Temporary failure resolving` for every mirror and still exited 0. `apt-get install` then failed with exit 100. One pass of the step took 9 minutes. The step is not needed, and it is the only reason a mirror outage breaks CI: - It runs before the caches restore. On a warm run nothing compiles: the mise cache restores (`mise already installed`, 15 tools `already installed`), and `.luarocks already populated` (run 43030815711). - On the runner image (Ubuntu 26.04.1), it installs only `build-essential` and `libreadline-dev` (185 kB). `unzip` is already there. - A cold run builds without both packages (below). Lua 5.5.0 builds with `make guess`, which does not use readline. mise prints a warning that readline line editing is missing and carries on. ## What changes The step is deleted from `.github/workflows/test.yml` (6 lines). Nothing else in `.github` uses apt. (`.devcontainer/post-create.sh` has the same two apt lines. It is not CI, and this PR leaves it.) The first commit on this branch retried the install through an outage. It is superseded: its skip check never fires on this image (`build-essential` and `libreadline-dev` are not installed), a full outage would hold the job about 30 minutes, and every run would still depend on the mirrors. The net diff of the PR is the deletion. I also checked gating the step on cache hits. `jdx/mise-action` (pinned `1648a78`) restores its cache and runs `mise install` in one step. Its `install: false` input skips the install and also the cache save (`saveCache` runs inside `if (install)`, and the action has no post step), so gating would need separate cache steps. Deleting the step needs none. ## Evidence **Cold run without the step passes.** Run 43038727992 (`workflow_dispatch` on branch `scratch/apt-cold-proof-1`, head `ba9455f602bf7e16d577c243d685a03ab6d05849`) used new cache keys (`mise-cold-1`, `luarocks-cold-1`) and a diagnostic step in place of the apt step. All jobs passed: `unit-tests`, `covered-lock`, `integration-tests (stable)`, `integration-tests (v0.8.0)`, `Required`. | What the log shows | Value | |---|---| | image | Ubuntu 26.04.1 LTS | | present | `gcc` 15.2.0, `make`, `unzip`, `curl`, `tar` | | not installed | `build-essential`, `libreadline-dev` | | mise cache | not found, so cold | | mise install | 15 tools installed in 31.7 s (Lua 5.5.0 built from source) | | luarocks cache | not found, so cold; `busted`, `luacheck`, `luacov` built with `gcc` | | apt calls | 0 | **Warm run without the step.** This PR's own run 43040639103 (head `345373422bd06ae3a973e91518300bbcac8cdc3a`, cache keys unchanged) passed every job: `unit-tests`, `covered-lock`, `integration-tests (stable)`, `integration-tests (v0.8.0)`, `Required`. It has no apt call and no install step. The log shows: | What the log shows | Value | |---|---| | mise cache | hit for `mise-v1-linux-x64-2026.11.5-551ea187...` | | mise install | `installed 0 tools · 15 already installed` | | luarocks cache | hit; `.luarocks already populated` | | unit tests | 1490 passed, 0 failed, 19 pending | Local gate: `mise run all` exit 0 (seed 1794407676, 1490 passed, 0 failed, 19 pending). `actionlint` and `mise run format-check` pass. **Risk.** A future runner image without `gcc`, `make`, `unzip` or `curl` would fail a cold run at `mise install` or `mise run setup`, with a compiler or unzip error. Then the step comes back. --- _Generated with [`xum`](https://github.com/coder/xum) • Model: `anthropic:claude-sonnet-5-5` • Thinking: `xhigh`_
…vim skill (coder#517) ## Why The operator asked for a code-shape refactor of the plugin, landed on main in small PRs, fixes first. His rule for the verification skill: a scratch harness that catches a blocker goes into the repo's skill, and memory points at repo paths. The wire probe that caught the blockers of coder#500 over four review rounds lived only in a scratch folder, so the next agent could not run it. This PR puts it in `.agents/skills/verify-claudecode-nvim`. ## What changed Three new files under `.agents/skills/verify-claudecode-nvim`. No existing file, no `lua/`, `plugin/`, `tests/`, `scripts/` or `.github/` file changes (425 added lines, above the 300-line aim: the launcher, the checker and the page are all in it). - `scripts/proof-tools-call-wire.sh [tree]` is the one command. It starts the plugin server of `tree` (default: this checkout) in one headless nvim and runs the checker. Exit 0 pass, 1 a case failed or a frame leaked, 2 the probe could not run or judged fewer cases than the table holds. - `scripts/proof-tools-call-wire.mjs` holds the case table, the WebSocket client and the checker. It prints one `ok:` or `FAIL:` line per case and ends with `COUNT tools-call-wire cases=<n> failed=<n> leaks=<n>`. - `features/tools-call-wire.md` is the feature page: the command, a pass and a fail, the red proof, the normalization. Rules in the script: - Each case is a row of one table: name, `params`, expected answer. A refusal names the parameter (`-32602`, and the data names it). A normal answer is written out whole. The tools, argument shapes and wrong values are expanded from short lists into rows, and the checker has one path for every row. - Normalization sits next to the table. The leak rule and the token rule read the text of every frame that the server sends as the wire sent it, and no parse or rebuild comes first. One step comes before them: every escaped slash, a run of backslashes and a slash at any depth of nesting, is read as a slash. Then the path rewrite applies to that text: the folder nvim runs in becomes `$CWD`, the plugin tree `$TREE`, the real home folder `$HOME`, in that order. A frame leaks when its text then holds `.lua:` and a digit, `$TREE` or `$HOME`, or 8 or more characters of the auth token in a row. So a path in a sibling field, an object key, a nested list, a duplicate key or a JSON fragment inside a plain string leaks. It also reads a frame that comes before the answer, or within 200 ms after the last answer, and counts a frame for one case only. A frame that arrives after its own answer is judged with the case in flight. The comparison with the table reads a rebuilt copy of the answer that rewrites strings but not keys, so two keys that the rewrite would make equal never pass as a correct answer. The leak rule finds each of the three paths in a frame unchanged, so the probe refuses to run when the tree path, the work folder path or the real home folder path holds a double quote, a backslash or a control character (the characters that JSON writes escaped). It exits 2 with a message that names the path, before it sends a case. A backslash in the tree path can make nvim fail to load the plugin first: the script then exits 2 with `nvim wrote no lock file in 10 s`. The workspace folder is not a leak: `getWorkspaceFolders` returns it on purpose, and its expected answer is `{"folders":[{"name":"work","path":"$CWD","uri":"file://$CWD"}],"rootPath":"$CWD","success":true}`. - A pass needs at least one case and every row judged: an empty table, or fewer judged cases than rows, exits 2 and prints no `COUNT` line. The `cases=` count is the number of cases that the checker judged. - Safe launch: nvim and the checker start with an empty environment. When nvim and node are the binaries themselves, nvim gets only `PATH=/usr/bin:/bin`, `WIRE_PARENT`, `WIRE_TREE`, `WIRE_CWD`, and `HOME`, `TMPDIR`, `XDG_CONFIG_HOME`, `XDG_DATA_HOME`, `XDG_STATE_HOME`, `XDG_CACHE_HOME` and `CLAUDE_CONFIG_DIR` in `/tmp/wire-probe.XXXXXX` (from `mktemp -d`, mode 0700), and the checker gets no variable. A version-manager shim that does start adds variables of its own. nvim runs with `terminal.provider = "none"`. The script exits 2 when a `claude` program is on `PATH=/usr/bin:/bin`, when nvim or node does not start with an empty environment (a version-manager shim does not), and when the temp dir cannot be made. nvim must be a direct child of the script: it checks its parent before the plugin starts, and when the parent is not the script (a wrapper that runs Neovim without `exec`) it quits with an error, so no server starts and no lock file is written. The script then exits 2 with a message that names the wrapper and says to set `VCN_NVIM` to the Neovim binary itself. A wrapper that uses `exec` passes. When the server does not come up for another reason, the script prints the last lines of nvim's output. - The token: it lives in the memory of nvim and of the checker, and in the `0600` lock file in that folder. The checker sends it only in the upgrade header. The checker prints only text that it wrote itself (`stop`) and the text of a frame that passed `show`, which withholds a text that holds 8 or more characters of the token in a row (the output says `<holds the auth token>`). It never prints the message or the stack of an exception that Node or a library raised, because such a message can quote its input (the WebSocket constructor quotes the header value): a frame that is not JSON and a lock file that does not parse end the run with exit 2 and a fixed message, and any other exception ends it with exit 2 and one line that holds only the exception's class name. When the server does not come up, the script prints the last lines of nvim's output. That happens only while no lock file exists, and the plugin logs only the length of its token. On a normal exit, a failed case, TERM, INT and HUP, the script stops nvim and the checker by PID (it polls for their end and sends SIGKILL, and does not `wait` for them) and removes the folder. After SIGKILL no cleanup runs: nvim, a direct child of the script, checks its parent every half second and quits, its server stop removes the lock file and closes the socket, and the checker exits on the closed socket. Both processes and the lock file are gone within 5 seconds (the bound the script holds to; 0.41 s at most in the 10 runs below). The folder stays then, with no token in it. - Why not `vcn.sh launch`: it starts a TUI in agent-tty (agent-tty, Chromium) and puts this checkout on the runtimepath, so it takes no tree argument, which the red proof needs. The probe keeps the skill's conventions: `VCN_NVIM`, node from `mise which node`, and a client that takes the token from the `0600` lock file as `ws-client.mjs` does. It does not call `vcn.sh`, so coder#506 does not change how it runs. ## Cases in this PR, and the cases left out 44 cases: `params` absent, `false`, `null`, `true`, a number, a string, `[]`, `[1]` (all name `params`) and `{}` (names `name`). A non-string `name` (8 values). For `getWorkspaceFolders` and `openFile`: `arguments` absent, `{}` and each of the 7 non-object values, and for `openFile` each wrong type of `filePath` (8 values) plus `""`. Left out, all for size: the other nine tools (`getOpenEditors`, `getCurrentSelection`, `getLatestSelection`, `closeAllDiffTabs`, `getDiagnostics`, `checkDocumentDirty`, `saveDocument`, `close_tab`, `openDiff`) with their string parameters and the `getDiagnostics` `uri`, the `ctl_*` valid-call controls, and the two notification cases of the scratch `wire3.lua`. They are the next PR, which adds table rows and the silent-answer check. The index lines (`SKILL.md` Helpers and `features/README.md`) are not in this PR either, because coder#506 and coder#508 are open and change those files. They go in the next PR. ## How I proved it Neovim 0.12.3 and the official v0.8.0 release, each three runs in a row, from the head of this branch (merged with main `4c1df79`. Main has moved since to `c9e57d13`, which changes nothing in the skill folder or in these three files, so I did not merge it, and a run on a tree of `c9e57d13` printed the same line on both versions): every run printed `COUNT tools-call-wire cases=44 failed=0 leaks=0` and exit 0, and the six outputs are byte-identical. Red proof (`git archive f932d7b`, main before coder#500), exit 1 on both versions: ```text COUNT tools-call-wire cases=44 failed=30 leaks=15 ``` Round 3 (findings from the third review round, my proofs on this head): - A tree path with a double quote (`vim.json.encode` writes `"` as `\"`, so the rewrite to `$TREE` misses it). The mutant that puts the tree path into an object key of an answer, in the folder `/tmp/wp517-q"t/`: the previous head exits 0 with `cases=44 failed=0 leaks=0` on both versions, and this head exits 2 with the message above and no `ok:` line on both versions. The same mutant in a plain path stays red (`failed=2 leaks=2`), and the mutant with the tree path in every refusal gives `failed=31 leaks=31`. The checker alone refuses a quote in the tree, in the work folder and in the home folder, on Node 22.23.4 and 26.12.1. A tree path under the real home folder is read as `$HOME`, so the old gap shows only outside it. A backslash in the tree path makes nvim fail first, and the script exits 2 with `nvim wrote no lock file in 10 s` on both versions. A newline in the tree path does not: the checker refuses it with its own message (exit 2, both versions). - The token in an exception of Node: a lock file that is valid JSON whose token holds a line break makes the WebSocket constructor throw a message that quotes the header value. The previous head printed 26 eight-character runs of such a value on stderr (my mutant's value has 33 characters, one more than a real token of 32 characters, which has 25 such runs). This head exits 2 with `stopped by an unexpected TypeError`, 0 runs on stdout and stderr, on both versions. - A `cd` into the new temp dir that fails (a stand-in `mktemp` that prints a dir that does not exist, or a dir with mode 000): this head names the dir that `mktemp` printed in both cases, and leaves 0 dirs. The previous head named an empty path in both, and left 1 dir in the mode 000 case. - A HUP, TERM or INT that came just after the checker exited could hang the launcher in its cleanup, with nvim and its lock file alive until the launcher was killed (`wait <pid>` blocked on a child that was already gone: 3 hangs in 118 signals sent right after the `COUNT` line, all three HUP on 0.12.3). The cleanup no longer calls `wait`. My 100 TERM trials on 0.8.0, each right after the `COUNT` line, hung 0 times (my log keeps no exit code, so it does not show that the trap ran in each one, and I count none of my HUP or INT trials after the change). Two independent verifier runs on this head: 0 hangs in 130 and in 120 signals. With `wait` back in the cleanup: 1 hang in 130 and 1 in 60. I did not find the cause inside bash. - A server that died in the middle of a run could print `WebSocket handshake failed`. The checker now drops that handler when the connection opens, and prints `the server closed the connection` on both versions (SIGKILL of nvim, exit 2, no process left, no temp dir). - The same runs on this head as before: green three times on each version with `cases=44 failed=0 leaks=0` and byte-identical outputs, red proof `failed=30 leaks=15` on both, wrapper without `exec` exit 2 and with `exec` pass (3 runs each, both versions), SIGKILL of the script (5 runs on each version, 0.41 s at most), 12 signal trials (TERM, INT and HUP, each between the lock file and the end of the run and right after the `COUNT` line, both versions: exit 143, 130, 129 or 0, no process, no temp dir left). Findings of rounds 1 and 2 and their proofs (copies, not committed): - Round 2, the leak rule reads the text of each frame as the wire sent it. A leaking value behind a key collision (mutant `mE`: one key is the work folder path and holds `/virtual/probe.lua:42`, one key is `$CWD` and holds `safe`): `failed=41 leaks=41` on both versions, and the previous head printed `failed=0 leaks=0`. A duplicate key in the text item of a correct result (the real home path in the first, so the parser keeps the last): `failed=2 leaks=2`, and the previous head passed on both versions. A path in a JSON fragment inside a plain string (the slashes are `\/` on Neovim 0.8): `failed=31 leaks=31` on both versions, and the previous head passed on 0.8. The sibling-field, object-key, nested-array and late-frame mutants of round 1 stay red (`failed=2 leaks=2` for the first three, `failed=1 leaks=2` for the late frame). - Round 2, a server frame that is `x` followed by the token (not JSON): exit 2 and one fixed message, and 0 matches of any 8-character run of the token on stdout and stderr, on both versions. The previous head printed 2 such matches on stderr. A lock file whose token value is not valid JSON, with a made-up value (not a token): 0 matches of any 8-character run of the value on stdout and stderr, and 3 on the previous head. The token in an error answer (mutant `mT`: the token as a field `t` of every `-32602` error object): `failed=41 leaks=41` and 0 matches. - Round 2, a wrapper that runs Neovim without `exec` (`VCN_NVIM` is a script that runs the real binary as a child): three runs on each version exit 2 with the message, print no `ok:` line, and leave no process and no temp dir. A wrapper with `exec`: three runs on each version pass with `cases=44 failed=0 leaks=0`. - Round 2, a leak in the answer of the last case: `failed=1 leaks=1` and one `FAIL` line, where the previous head printed `failed=2` and a second line for the same frame. A failing `mktemp` or `mkdir`: exit 2 and a message, where the previous head exited 1. Round 1: - A path in a sibling field of a correct result (`result.debugPath = "/virtual/probe.lua:42"`): `failed=2 leaks=2` on both versions. Before, the probe printed `leaks=0` and exit 0. The tree path as an object key of an answer: `failed=2 leaks=2`, and the same mutant passes when the checker does not rewrite keys. An extra server frame 100 ms after each valid answer, which only the read after the last case sees: `failed=1 leaks=2`. - The auth token in an error answer: `failed=31 leaks=31`, no `FAIL` line printed the token, and the match count of the token in stdout, stderr and nvim's output file is 0. - SIGKILL of the script after the checker connected: five runs on each Neovim version, both processes (recorded by PID) gone after 0.2 to 0.4 s, lock file gone after 0.2 to 0.4 s, 0 files with the token in the folder. The launcher of round 1, same test: both processes still up after 5 s, the lock file still there, 1 file with the token. A server killed in the middle of a run ends the checker with exit 2 and one message. - An empty table: exit 2, one message, no `COUNT` line. A loop that judges no case, or skips one row: exit 2, no `COUNT` line. - A version-manager shim as `VCN_NVIM`: exit 2 and a message that names it and says to set `VCN_NVIM` to a real binary. A node that does not start: exit 2 and a message that names it. A tree whose `require` fails: the last lines of nvim's output and exit 2. In a fresh clone of the branch on this host, which mise does not trust, the page's commands failed with exit 2 and that message until `VCN_NVIM` named a real binary. With it, the whole proof exits 0, the red proof exits 1 with `failed=30 leaks=15`, and the Neovim 0.8 run prints the same lines as the 0.12.3 run. - The real home path in an answer, with the caller's `HOME=/tmp`: `failed=31 leaks=31`. The previous head printed `failed=0 leaks=0` there. A module named in the caller's `NODE_OPTIONS` ran inside the checker of the previous head (1 marker line) and does not run now (0). - nvim's own temp root: in a fresh `/tmp` (a private tmpfs), the previous head left `/tmp/nvim.coder` and this head leaves nothing, on both versions. Controls that must go red (copies, not committed): main with the entry error holding the tree path, or the real home path: `failed=31 leaks=31` each. The checker without the `$CWD` rewrite: `failed=2 leaks=0`. A checker whose home folder is `/tmp`, so that the run dir lies under it: with the `$CWD` rewrite first it prints `failed=0 leaks=0`, and with the `$CWD` rewrite after the `$HOME` one `failed=2 leaks=2`. A wrong expected `rootPath`: `failed=2`. A wrong named parameter for `filePath`: `failed=8`. Other checks: - Token: match counts of the token in stdout, stderr and nvim's output file, and in the command line and environment of the child processes: 0 each. The token has 32 characters, and none of it was printed. - A `claude` on the stripped PATH (a stand-in that writes a marker file): exit 2, the marker was never written, no temp dir was made. - TERM, INT and HUP, each sent between the lock file and the end of the run and right after the `COUNT` line, left no process and no temp dir (12 trials, see round 3). The cleanup ignores TERM, INT and HUP while it runs. - Missing tree, a tree without `lua/claudecode`, a missing nvim, an nvim that exits at once, and a tree whose `require` fails: exit 2 with a message each. - Node 22.23.4 and 26.12.1 both ran the whole probe on this head (pass on both Neovim versions, and `failed=30 leaks=15` on the red tree with Node 22). - Local merges of this head with coder#523 (`eb16b3aa`), coder#508 (`17252700`), coder#506 (`ab9d431c`) and coder#494 (`c219b155`), never pushed: no file of this PR changes, and each ran `cases=44 failed=0 leaks=0` on both versions. coder#511 (`ef610b7d`) and coder#485 (`d00e1be2`) conflict with current main in `covered.lock` on their own, and their trees do not hold coder#500 (the probe prints `failed=30 leaks=15` on them), so a merge check says nothing about these three files. Neither changes a path of this PR. - A fresh clone of this head, the page's commands word for word: the whole proof exits 0 with `failed=0 leaks=0`, the red proof exits 1 with `failed=30 leaks=15`, and Neovim 0.8 prints the same line. Without `VCN_NVIM` the clone (which mise does not trust) exits 2 and names the shim. Gates on this head: `mise run format-check` 351 files 0 changed, `mise run check` 0 warnings / 0 errors in 176 files, `mise run test` native exit 0 with 1586 successes / 0 failures / 0 errors / 22 pending (seed 1794431103), and `bash .agents/skills/verify-diff-lifecycle/check.sh --hash-only` prints `covered.lock matches the code`. ## Limits - Only `getWorkspaceFolders` and `openFile` are in the table. The feature page says so. - The run dir is `/tmp/wire-probe.XXXXXX`. The probe ignores `TMPDIR`. - `vcn.sh` has the same `bin()` and then `env -i` start for nvim, so a version-manager shim fails there too. This PR does not change `vcn.sh`, because coder#506 rewrites it. - A frame that comes more than 200 ms after the last answer is not read. - The probe has no CI job. It needs nvim and Node, and CI runs neither the harness nor this probe. --- _Generated with [`xum`](https://github.com/coder/xum) • Model: `anthropic:claude-sonnet-5-5` • Thinking: `xhigh`_
|
Maintainer decision, amending the round-8 waiver above: #518 closed unmerged, so its precondition changes. The one waived push now comes after the new terminal-placement PR (the source fix that keeps a user's Generated with |
Conflicts: covered.lock (main's side, re-recorded in the next commit), lua/claudecode/README.md, lua/claudecode/diff.lua, lua/claudecode/utils.lua and tests/integration/diff_new_tab_mode_spec.lua. Main's code stays everywhere except this PR's own additions. _Generated with xum • Model: anthropic:claude-sonnet-5-5 • Thinking: xhigh_
verify-diff-lifecycle reported STALE after the merge (the other four check.sh pass). The check passes with the model unchanged: the merge keeps this PR's close path and main's end-of-diff cleanup side by side. _Generated with xum • Model: anthropic:claude-sonnet-5-5 • Thinking: xhigh_
… Ex commands The helper's `edit` and the tabclose spec's `split` took the temp path as it is, so the specs failed with E499 under a TMPDIR with a space and a `%`, which the integration job of main runs. Both calls now pass the path through `fnameescape`, as the other new-tab specs already do. _Generated with xum • Model: anthropic:claude-sonnet-5-5 • Thinking: xhigh_
… vcn.sh get Main's proof-static check no-source-run-env rejects a script that sources a run.env, and this branch's journey-new-tab-diff-close-mode.sh did. It reads OUT and WORK through `vcn.sh get` now, as journey-new-tab-diff-mode.sh does. This is a shell script, not Lua. _Generated with xum • Model: anthropic:claude-sonnet-5-5 • Thinking: xhigh_
The corrected README sentence said a terminal window that "comes before" the window that `:tabclose` cannot close is already closed. A probe in Neovim 0.12.3 and v0.8.0 shows the opposite reading of the layout: `:tabclose` tries the windows in an order that reaches a terminal window on the right before a modified window on the left (the terminal window closes), and stops at a modified window on the right before it reaches a terminal window on the left (the terminal window stays). The sentence now says "reaches". _Generated with xum • Model: anthropic:claude-sonnet-5-5 • Thinking: xhigh_
|
Record before the one waived round-8 push (waiver above, amended in comment 6817325625):
Generated with |
|
@codex review |
|
@codex security review |
|
Codex Review: Didn't find any major issues. Already looking forward to the next diff. 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. |
…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 can call
close_tabon a diff that opened in a new tab (diff_opts.open_in_new_tab = true) while the user is in Normal mode in their file. The user then lands in Insert mode in that file, and the next keys they type edit it. With the defaultterminal.auto_insert = truethis happens with the native and the Snacks provider, in the split and the unified layout.It needs the Claude terminal of the diff tab to be the current window of that tab: the user focuses it (it enters terminal mode, as documented), leaves terminal mode with
<C-\><C-N>, and Claude then closes the diff. The same happens when the user goes back to the first tab with:tabfirstafter that, so the diff tab waits behind with the terminal current. A user who is still in terminal mode whenclose_tabarrives is not affected:mode()readstduring the cleanup, so the handler'sncheck fails. Withterminal.auto_insert = falsethe diff registers no handler and the file stays in Normal mode.Two more cases leave the user in the wrong place because the diff tab stays open after
:tabclose::tabcloserefuses it (E784). The cleanup deleted the proposal buffer, the cursor fell into the Claude terminal, and the diff's handler started terminal mode there. The handler stayed registered after the diff.'nohidden'is set and a modified buffer of the user's in the diff tab stops:tabclose(E37). On main the cursor ended in Normal mode, with no handler left, when it was in the proposal window, and in Insert mode in the user's file when the terminal was the diff tab's current window.#473 kept a new diff in Normal mode when it opens, and #478 drops the terminal's
BufEnterhandler when the diff's terminal window closes. The close itself still had the hole #473 closed for the open.Cause
_cleanup_diff_stateand the unified layout'scleanup_inline_diffmade the diff tab current again only to run:tabclosethere. Entering a tab page runsBufEnterfor the buffer in its current window. When that window shows the Claude terminal, the diff's ownBufEnterhandler (and Snacks'auto_inserthandler) callstartinsert.:tabclosethen moves the cursor to the user's file, and the pending insert starts there.A
startinsertandModeChangedtrace ofclose_tabon main (native provider, split layout, cursor left the terminal with<C-\><C-N>):Snacks adds its own
vim.cmd.startinsertfrom itsauto_inserthandler at the same point.What changed
utils.close_tabpage_without_entering(tab)closes a tab page with:tabclose Nwhile the cursor stays in the tab the user is in, so noBufEnterhandler runs for the diff tab's windows._cleanup_diff_stateandcleanup_inline_diffuse it. The two setup-failure paths keep their own code: the cursor is already in the new tab there, so nothing is entered again.BufEnterhandler by its id first, so the handler goes whether or not the terminal window closes. Then it runs every window and buffer step with all autocmds blocked (:noautocmd, throughutils.without_autocmds): it gives theunifiedproposal window an empty buffer, turns diff mode off in the windows of that tab that show the file (a window the user split from a diff window, or adiffoptwithoutcloseoff, kept'diff', scrollbind andfoldmethod=diff), moves a cursor that sits in a window of the diff to the window the diff stored for the file, and closes the proposal window of a split diff (after the user closed the file window, it keeps that window with the empty buffer) and the terminal window the diff added. It keeps that terminal window when it is the only window that shows the Claude terminal and the cleanup shows the terminal again afterwards (the terminal was visible before the diff). A float that Snacks parked withhidedoes not count as showing it. Once every close is done, it firesWinEnterandBufEnterfor the window the cursor ended in, orBufEnteralone when only its buffer changed.terminal_buffer_in_new_tabandterminal_enter_handlerfor this. Before this change the release acted on the stored windows, so it blanked such a proposal window or closed such a terminal window.BufLeaveinside the proposal swap that moved the cursor, so Neovim loaded the empty buffer into the user's window, aBufNewfrom creating that buffer that closed the tab, and aWinEnterthat entered the terminal while Snacks queued an insert. A recheck before each call cannot close that, because the autocmd runs inside the call. So those calls run no autocmd at all.utils.without_autocmdsuses:noautocmdand not'eventignore', because restoring'eventignore'fires anOptionSetevent. The cursor'sWinEnterandBufEntercome last, after the closes, so an autocmd that runs then finds no proposal window to switch, enter or close (only a terminal window that the release keeps, whoseBufEnterhandler is already gone), and nothing in the release follows it, not even when it closes the tab. The release fires no leave event for the windows of the diff.lua/claudecode/README.mdstates the rule, the events a close fires for every position of the user, and both reasons:tabclosecan leave the tab open.ensure_visiblehad to open another one. Main never reaches that call there, because it leaves the terminal window in place. The provider's new window firesBufLeaveandBufEnter. A userBufLeaveautocmd that enters a window made the provider put the terminal into that window, so the user's edited file lost its window (the verifiers'actor-blank.sh, native and Snacks, both layouts). A user autocmdBufEnter term://* startinsertqueued an insert that started in the user's file once the provider moved the cursor back (native provider, both layouts). The release now keeps the terminal window in exactly that case, soensure_visiblefinds a visible terminal and opens nothing, as on main. It still closes the window when another window shows the terminal (E37 with the original tab's terminal) or when the cleanup does not show the terminal again (the user hid it before the diff). When it closes the window, it removes that window'sWinClosedautocmd, which:noautocmdkept from running.tests/helpers/new_tab_diff_child.luaand run in eight files (counts in Evidence), each under the runner's 40 s limit: open and mode, close, the tab that stays open, the user's own windows,:tabclosestopped by E37, the terminal handler from the original tab, autocmds of the user during the release, and the diff's terminal window in a tab that stays open. TheBufEnterhandler is covered by main's in-process spec, which this PR does not change.tests/integration/utils_without_autocmds_spec.luacovers the helper.:splitof the file window with and withoutcloseoff, adiffoptwithoutcloseoff, a file window the user closed (a vertical diff whose original pane is gone), and every window but the terminal closed by the user (the terminal cannot close, E444). In every last-tab case the diff's terminal window stays, because it is the only window that shows the terminal.BufEnter term://* startinsertleaves Normal mode in the file, aBufLeaveautocmd that enters the user's window leaves it showing the user's edited file (the actor-blank sequence), the terminal window closes when the user hid the terminal before the diff, and it stays when the only other window that shows the terminal is a float parked withhide(Neovim 0.10 and later).b.txt, in both layouts. The view, the text and the cursor must stay, and the handler must go, also when a modified buffer and'nohidden'make the window close fail. Two more: no empty buffer is left when'winfixbuf'stops the swap (Neovim 0.10 and later), and two windows of the user's in diff mode keep it.close_tab): no autocmd runs while the proposal window, or a terminal window that the release closes, is open, and the only events areWinEnterandBufEnterfor the window the cursor ends in (BufEnteralone when it stays in the proposal window of aunifieddiff, none when it was in a window of the user's), in both layouts. ABufLeaveautocmd of the proposal buffer that switches its window never runs during the swap, so the swap cannot blank a window of the user's. A nestedWinEnterautocmd that enters a second window on the terminal buffer finds the diff's handler gone. With the user's terminal hidden, so that the terminal window closes, aWinEnterautocmd that enters any terminal with Snacks' auto_insert leaves Normal mode, and the events come after both windows of the diff are gone. The cursor goes to the window the diff stored for the file, not to a window of the user's next to the terminal or next to the proposal window.'diff', and with the terminal on the left:tabclosestops before it closes the terminal window, so the release must close it and drop its handler. With the modified window at the right every window of the diff survives:tabclose: aBufNewautocmd that closes the tab (unified) or aWinClosedautocmd that does (split) never runs, and one that closes the tab when the cursor leaves the proposal window leaves no error and no record. The terminal window that the release closes leaves noWinClosedautocmd behind. A release called for a tab that is gone does nothing.'winfixbuf'case, red when the delete goes), the cursor goes to the window the diff stored for the file (the cursor-destination case, red when the cursor is not moved), and:diffoffruns only in windows that show the file (the two-windows-in-diff-mode case, red when it runs in every diff window). The old per-window validity check before:diffoffis gone: the loop lists the windows of the tab right before it, and no autocmd can run in it.tests/unit/utils_close_tabpage_spec.luacovers the helper. Thevimmock now honours:tabclose N, which it ignored.journey-new-tab-diff-close-mode.shinverify-claudecode-nvimreplays the user's keys and Claude'sclose_tabin agent-tty, with a last-tab phase andVCN_RTPfor Snacks. The feature step inopen-diff.mduses a fresh launch and fresh ids.utils.win_set_buf_in_split). The merge conflicted in five files.utils.luaanddiff.lua: main's code stays and this PR's own functions (without_autocmds,close_tabpage_without_entering, the release of a surviving diff tab) follow it.lua/claudecode/README.md: main's text stays, with this PR's two helpers added to theutils.lualine and this PR's paragraph about closing the diff tab added to the new-tab bullet, next to main's sentence about the new tab that aBufLeaveautocmd can leave.tests/integration/diff_new_tab_mode_spec.lua: this PR's version, which runs through the shared helper, because main's one-line change to the old inline code (fnameescapeof theeditpath) now lives in the helper.covered.lock: main's file first, then recorded again in its own commit aftercheck.shprinted STALE forverify-diff-lifecycle(the other four skills print PASS).no-source-run-env(proof-static.sh) rejectedjourney-new-tab-diff-close-mode.sh, which sourced arun.env. It readsOUTandWORKthroughvcn.sh getnow, asjourney-new-tab-diff-mode.shdoes. That is a shell script, not Lua.fnameescape(tests/helpers/new_tab_diff_child.lua,tests/integration/diff_new_tab_tabclose_stops_spec.lua), so the specs pass under theTMPDIRwith a space and a%that main's integration job uses.Behavior change: events at close
Closing the diff tab no longer enters it. Which events fire depends on where the user is:
close_tabfiresWinClosed,BufWinLeave,BufHiddenandTabClosed, and noBufLeave,WinLeave,TabLeave,BufEnter,WinEnterorTabEnter. On main it also firedBufLeave,WinLeaveandTabLeaveon the user's file and the enter and leave events of the terminal.close_tabgoes back to the original tab first, as before this PR. It firesBufLeave,WinLeaveandTabLeavefor the window the user is in, thenWinEnter,TabEnterandBufEnterfor the original tab's window. ABufLeaveautocmd on the user's file in the third tab runs.close_tabgoes to no tab first. A user in a third tab stays there. A user in the diff tab goes to the neighbouring tab, as:tabclosedoes, andTabLeaveandTabEnterfire.:tabclosecannot close the diff tab (the last tab, or E37): the release's own steps fire no event. Afterwards, with the cursor in another window, it firesWinEnterandBufEnterfor that window, with only its buffer changed (aunifiedproposal window that got the empty buffer under the cursor)BufEnteralone, and with the cursor in a window of the user's own nothing.ensure_visibleopens no window there when the terminal has another window or:tabcloseleft the diff tab's terminal window open. When:tabcloseclosed that window before it stopped at E37 and no other window shows the terminal, the close shows the terminal again, as the next bullet says.ensure_visible). That firesWinLeave,WinEnter,BufLeaveandBufEnterfor the user's window and enter and leave events for the terminal, as when the user opens the terminal. ABufLeaveautocmd, such as an autosave, can run then. A diff tab that stays open keeps its own terminal window in that case, so the close opens nothing there, unless:tabcloseclosed that window before E37 stopped it (e37-hidden, see Known limit). This PR does not change howensure_visibleruns. It is provider code, and suppressing the events there would hide them from the provider's own autocmds as well.A trace of the events of
close_tab(native provider, split layout, Neovim 0.12.3, the positions original tab, diff tab, third tab, third tab with the original tab gone, terminal hidden, last tab and E37) shows the events above in each of the 7 positions (see Evidence).Known limit, not changed here
When the user closes every other window of the last diff tab (for example
:qin the diff window), the Claude terminal is the only window left. It cannot close (E444), so it stays. The release now drops the diff'sBufEnterhandler there, which main left registered. The user keeps the mode they had in the terminal. With the native and the Snacks provider no second terminal window opens:ensure_visibleopens a terminal only when no window shows its buffer.The terminal-placement fix #598 is merged on main, and this branch has it. It removes the one problem that the earlier heads of this PR had: a user
BufLeaveautocmd that enters another window made the provider put the Claude terminal into the user's window whenclose_tabshows the terminal again withensure_visible. That happens in three positions: the user hid the terminal in the original tab (hidden), the original tab is gone and the diff tab closes from a third tab (third-gone), and E37 with the terminal hidden in the original tab (e37-hidden). The probe runs the three positions with a run-once and with an every-timeBufLeaveautocmd, on the native and the Snacks provider, in both layouts. Before the merge (d00e1be2) the terminal went into the user's window in 20 of 24 runs on Neovim 0.12.3 and in 10 of 12 native runs on v0.8.0. With main merged, it goes there in 0 of 24 and 0 of 12, the same as main (4b788d58). The rows are in Evidence.One README sentence on the earlier heads overstated the E37 case: it said that the terminal window stays in the diff tab's window, but
:tabclosecan close that window first. The sentence now says: when:tabcloseleaves the diff's terminal window open, a terminal that was visible before the diff and that no other window shows stays in that window, because closing it would maketerminal.ensure_visibleopen another window, which fires the events of a new terminal window.:tabclosecloses the windows of the tab one after the other and stops at the one it cannot close (E37), so a terminal window that it reaches before that one is already closed, as on main, and the provider shows the terminal again. A probe on Neovim 0.12.3 and v0.8.0 ('nohidden', a modified buffer in one window, a terminal in the other,:tabclosefrom another tab) shows the order: with the terminal window on the right and the modified window on the left,:tabclosecloses the terminal window and stops with E37. With the terminal window on the left and the modified window on the right,:tabclosestops with E37 and the terminal window stays. The result is the same on both builds.Evidence
Main is
4b788d58. The head is45d280b8. Real Neovim,sleepas the Claude job, a stubclaudefirst onPATHin every run. Trees, probes and logs of this round ran from one temporary folder.4b788d58close_tab): 14 positions, native and Snacks, split and unified, no autocmd: 56 runs on Neovim 0.12.3, 28 native runs on v0.8.0nin 32 of 56 and 16 of 28,i(Insert in the user's file) in 12 of 56 and 6 of 28,tin 4 of 56 and 2 of 28nin 56 of 56 and 28 of 28. No row is worse than main: 28 and 14 rows go from not OK to OK, 24 and 12 have the same status, and 4 and 2 rows keep the statusUSERWIN-CLOSED(the user stays in the diff tab, whose windows close) without the Insert modehidden,third-goneande37-hidden, oneBufLeaveautocmd that enters a window of the user's (run-once or every-time), native and Snacks, split and unified: 24 runs on Neovim 0.12.3, 12 native runs on v0.8.0d00e1be2was OK in 4 of 24 and 2 of 12 (the terminal in the user's window in 20 and 10)other.txt) in the 8hiddenandthird-gonerowshiddenandthird-gonerows the cursor ends in the user's own window (a.txtorb.txt), not in the autocmd's. In the 4e37-hiddenrows every field equals main's. The 12 every-time rows have the same status: the autocmd runs once less in the 8hiddenandthird-gonerows, where the cursor also ends in the user's own window, and in the 4e37-hiddenrows it still fires but its callback returns at the probe's tab check, because the close does not enter the diff tabclose_tabby position (trace, native, split, 0.12.3): original tab, diff tab, third tab, third tab with the original tab gone, terminal hidden, last tab, E37:tabcloseorder probe ('nohidden', a modified window and a terminal window), 0.12.3 and v0.8.0:tabclosestopped by E37 7 / 7, terminal window 8 / 8, user windows 10 / 10new_tab_diff_child.lua, under aTMPDIRwith a space and a%(0.12.3)edit(the merge result before thefnameescapeedits):splitraises no error without itsfnameescape, so no test catches that one edit-U0digest of this PR's own change overlua/andtests/(index lines removed,@@numbers removed):d10fecf..d00e1be2and4b788d58..headc043ff2c0a95and421088cdb561utils.luanames,win_set_buf_in_split, main's sentence about the new tab) and the corrected sentence, the twofnameescapelines, and the old inlineeditline that this PR removes indiff_new_tab_mode_spec.lua(main's copy of it hasfnameescape, from #510)Gate on
45d280b8, which has main4b788d58merged, with GNUtimeoutfirst onPATH, a stubclaudeafter it (no realclaude) and a scratchCLAUDE_CONFIG_DIR. I read the native exit code of each command:mise run clean && mise run allexits 0. treefmt changes no file. Lint: 0 warnings and 0 errors in 223 files. Busted: 2015 successes, 0 failures, 0 errors, 51 pending.scripts/run_integration_tests_individually.sh) exits 0 on Neovim 0.12.3 (1106 successes, 0 failures, 0 errors) and on v0.8.0 (1102 successes, 0 failures, 0 errors), in a plain run and in a run under aTMPDIRwith a space and a%. No$XDG_CACHE_HOME/nvimappears (the real-cache check).check.shpass (verify-diff-lifecycle,verify-terminal-lifecycle,verify-mention-queue,verify-selection-tracking,verify-ws-connection), andcovered.lockofverify-diff-lifecyclematches. The changes in_cleanup_diff_stateare only how its tab closes and what stays in a tab that cannot close, which the lifecycle model lists as not modeled (window and tab layout).Generated with
xum• Model:anthropic:claude-sonnet-5-5• Thinking:xhigh