Repository navigation
fix(diff): say the unified layout runs on Neovim 0.8.0, and run its specs there - #564
Merged
Merged
Conversation
…specs there The unified layout uses vim.diff, which Neovim 0.8.0 has. The README said it needs Neovim 0.9, and eight integration specs skipped every unified case on 0.8.0 because of that. With the skips removed, the specs pass on 0.8.0: accept, reject, :q, window choice and sizes, the trailing line break cases, and the close and hand-over cases. 133 more tests run there. Both READMEs drop the 0.9 claim. The 0.9 gate that stays is the one for `:w ++p` in diff_hand_over_spec.lua, which Neovim 0.8.0 does not have.
setup_inline_diff raises -32000 when Neovim has neither vim.text.diff nor vim.diff. The message said "Inline diff requires Neovim >= 0.9.0", but the guard checks for the function and never for a version. Neovim 0.8.0 has vim.diff, so the layout runs there. The message now names the missing function, and a unit test drives the guard. The diff lifecycle model passes on the changed block (89205 states, every fact TRUE), so covered.lock is recorded for it.
The case "gives the sizes back when the unified diff's setup fails" checked only that some error came back. With the unified layout broken on Neovim 0.8.0 (a diff function that ignores vim.diff) it stayed green, because the guard raised an error too. It now asserts the "Diff setup failed" message and that the error data carries the failure of the user's autocmd, so it fails when the unified path itself breaks. It passes on Neovim 0.8.0 and 0.12.3.
Member
Author
|
@codex review |
Member
Author
|
@codex security review |
Codex Review Summary: ✅ CompletedThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
Codex 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. |
This was referenced Nov 12, 2026
hareki
pushed a commit
to hareki/claudecode.nvim
that referenced
this pull request
Nov 13, 2026
…oder#558) ## Why The goal is that every openDiff call that reached the server gets one answer when Neovim exits. A pending diff gets `DIFF_REJECTED`. A call that the exit cut off during its setup gets a -32603 error. Before, two cases left Claude waiting on a connection that then closed with 1001. Case 1: diffs that no `BufUnload` reaches. At exit, Neovim fires `BufUnload` for each loaded buffer before `VimLeavePre`, and a proposal buffer's `BufUnload` autocmd rejects its diff. That loop stops as soon as an autocmd deletes the buffer it fired for (`src/nvim/main.c` in v0.12.3, the `bufref_valid` check). Three things leave a diff without its `BufUnload`: 1. The `keep_empty` reject of a new-file diff (`diff_opts.on_new_file_reject`, the default), in a tab that the diff did not open, deletes its proposal buffer. The diffs after it get no `BufUnload`. 2. Another plugin's buffer that deletes itself in its own `BufUnload` stops the loop the same way, so a single diff can lose its `BufUnload`. 3. SIGTERM and SIGHUP run `VimLeavePre` without the loop. Such a diff reached the `VimLeavePre` autocmd of `diff.lua` pending, which removed it without an answer. `server.stop` then found nothing to reject. The docs verifier found the first case (B1). Case 2: an exit inside a handler, during setup. A `nested = true` `ClaudeCodeDiffOpened` handler that runs `:qa!` exits Neovim while the openDiff call is still in setup. That call has no response sender yet, because the server stores one only after the tool handler yielded. The exit never unwinds the stack, so the early answer of the setup never leaves. A verifier of this PR found it, and it also exists on `origin/main`. Repro with a real WebSocket client that plays Claude (a plugin server, `auto_start = true`, default `diff_opts`). The client sends `openDiff` for a new file (id 1), then for an existing file (id 2): | Neovim 0.12.3, 0.11.5, 0.10.4, 0.9.5, 0.8.0 | origin/main | this PR | |---|---|---| | `qa!` once both diffs are open | `DIFF_REJECTED` for id 1 only, then Close 1001 | `DIFF_REJECTED` for id 1 and for id 2, once each, then Close 1001 | | a handler runs `qa!` during the setup of id 2 | `DIFF_REJECTED` for id 1 only, then Close 1001 | `DIFF_REJECTED` for id 1, a -32603 error for id 2, then Close 1001 | ## What changed 1. `lua/claudecode/diff.lua`. The `VimLeavePre` callback (now the local `cleanup_at_exit`) first rejects each diff whose status is `pending`, oldest first. It runs `_resolve_diff_as_rejected` under `pcall`, so one diff that fails to reject cannot leave the others waiting. Then it calls `M._cleanup_all_active_diffs("shutdown")`. The pass is inline because it has one call site, as AGENTS.md (Code Shape) asks. 2. `lua/claudecode/server/init.lua`. `_handle_request` records each request in `M.state.in_flight` while its handler runs. `M.stop` answers every request still recorded with -32603 "Internal error" before it stops the TCP server, and `_handle_request` sends nothing for a request that `stop` answered. A call in setup gets this error, and a waiting call keeps getting `DIFF_REJECTED` from the reject pass. 3. Tests. Three specs run under `mise run test`, and one real-Neovim spec runs in the integration runner. One test checks the order of the stop: the -32603 answer must leave before the TCP server stops. See Verification. 4. Model. `verify-diff-lifecycle` gets the exit modeled. See Verification. 5. Docs. The root `README.md` has one bullet, "Exit answers" (under Global Invariants), that states what Claude gets for each kind of call in each kind of exit. Each fact in it was probed. The other docs describe only the mechanism that they own and link to that bullet: `ARCHITECTURE.md` (the Resource Cleanup section and the Resource Cleanup bullet under Performance & Security: which autocmd runs what, in which order), `lua/claudecode/README.md` (the `BufUnload` loop and the two passes), the server README (what `M.stop` sends), `SKILL.md`, `check.sh`, the comments of `diff.lua`, and two test headers. 6. `covered.lock` of `verify-diff-lifecycle` records the new hashes of `diff.lua` and `server/init.lua`. It was re-recorded on the merged tree each time main moved, after the full `check.sh` passed. The locks of `verify-mention-queue` and `verify-selection-tracking` also cover `server/init.lua`, so each records its new file hash (see Gates, item 7). ## Scope Not changed: 1. `_cleanup_all_active_diffs`. It still removes every entry, in any status. A saved diff is removed at exit as before, and it gets no second answer. 2. `close_pending_diffs` in `server.stop`. It stays for a stop that is not an exit (`:ClaudeCodeStop`). At exit the diff autocmd runs first, so it finds nothing pending there. 3. SIGKILL, a crash, and a quit from inside an autocmd that is not `nested`. They run no `VimLeavePre`, so no diff gets an answer, Claude's socket closes with 1006, and the lock file stays. No code in the plugin runs then, so the plugin cannot change this. The READMEs say so. ## Follow-up PR (after this merges) 1. `README.md` (Global Invariants, "Exit answers") says that with `"close_window"` both diffs have their answers before the first `ClaudeCodeDiffClosed` fires. That holds for `:qa!` and SIGTERM, the exits that were probed. For a nested `:qa!` during the setup of a diff, the call that the exit cuts off gets its -32603 from `server.stop`, after the `ClaudeCodeDiffClosed` events. Every call still gets exactly one answer before Close 1001. Every call still gets its answer, so this does not block the merge. A follow-up PR limits that README sentence to waiting calls and says that a call cut off in its setup gets its -32603 after the `ClaudeCodeDiffClosed` events. 2. `tests/integration/diff_exit_reject_spec.lua` skips the unified layout below Neovim 0.9, with the comment "The unified layout needs Neovim 0.9". coder#564, now on main, removed that claim and those skips. The same follow-up PR drops the three skips and the comment, so the 14 unified scenarios also run on v0.8.0 (28 pass there in a trial run). ## Blast Radius Only a call that is open at exit, or a call whose handler a non-exit `server.stop` interrupts, changes. 1. A pending diff that gets no `BufUnload` (an exit through SIGTERM or SIGHUP, or another autocmd that deletes its buffer first) went unanswered on main, and now gets `DIFF_REJECTED`. A single pending diff that gets its `BufUnload` behaves as before, because the new pass finds nothing pending. 2. The `ClaudeCodeDiffClosed` event of a diff that the new pass rejects carries the reason `shutdown`. In the `vertical` and `horizontal` layouts, a new-file diff with `keep_empty` in a tab it did not open fires `diff rejected (keep_empty)` instead, as it does for a `BufUnload` reject. The `unified` layout keeps the reason `shutdown`. That event fires while a later diff is still `pending`, so a `ClaudeCodeDiffClosed` handler can see a later diff that is not rejected yet. The README says so. 3. `server.stop` answers a request whose handler is still running with an error. Before, an exit inside a handler answered nothing. A stop that is not an exit and runs inside a handler (for example `:ClaudeCodeStop` from a `ClaudeCodeDiffOpened` handler) also answered nothing: the handler's result came after the stop, when `send_response` sends nothing. Now the call gets the error once, before Close 1001. ## Verification Red, then green, run again at the head. The new specs fail on `origin/main` code and pass with the fix. | Spec | `origin/main` code | This PR | |---|---|---| | `tests/unit/diff_exit_reject_spec.lua` (6 tests) | 4 red, 2 green | 6 green | | `tests/unit/diff_exit_during_setup_spec.lua` (2 tests: the exit inside a handler, both orders of the two autocmds) | 2 red | 2 green | | `tests/unit/server/in_flight_request_spec.lua` (6 tests) | 5 red, 1 green. Two of the red tests, "a finished request" and "a request whose handler raised", fail on main only at `next(server.state.in_flight)`, because the table does not exist there. Main answers those requests correctly, so they guard the new table. | 6 green | | `tests/integration/diff_exit_reject_spec.lua` (28 scenarios, a child Neovim does the real exit) | 11 red, 17 green on 0.12.3, 0.11.5, 0.10.4 and 0.9.5. On 0.8.0, 7 red and 7 green (14 run) | all green on all five versions | The integration scenarios are 20 for `:qa!` (split and unified layout, `keep_empty` and `close_window`, 5 open orders), 4 for SIGTERM and for a buffer that deletes itself in its own `BufUnload` (both layouts), and 4 for the exit inside a handler (both layouts). On Neovim 0.8.0 the unified layout is pending, so 14 run there. Mutants of the code (one edit each), against the three unit specs and the integration spec on Neovim 0.12.3: | Mutant | unit red | integration red | |---|---|---| | `M.stop` answers nothing | 5 | 4 | | `M.stop` sends the answer after `stop_server`, or after `M.state.server = nil` (it never reaches Claude) | 1 | 0 | | `M.stop` keeps the entry, so the handler sends a second answer | 2 | 0 | | `_handle_request` sends after a stop | 2 | 0 | | `_handle_request` never removes an entry | 4 | 2 | | the reject pass sorts by tab name, not by open order | 1 | 0 | | the reject pass skips unified diffs | 0 | 2 | | no reject pass, or the pass after the cleanup | 5 | 7 | Probes of the exit facts. A real WebSocket client plays Claude: it sends `openDiff` for a new file (id 1), then for an existing file (id 2). A `ClaudeCodeDiffClosed` handler logs the status of every diff. Each scenario ran at the head `43fc68d619e218995dbba2fc07a128c6ff1fbaee` on Neovim 0.12.3, 0.11.5, 0.10.4, 0.9.5 and 0.8.0, and gave the same result on all five: | Scenario | Claude gets | Close | Lock file left | |---|---|---|---| | `:qa!` | id 1 and id 2 `DIFF_REJECTED`, once each | 1001 | no | | SIGTERM, or SIGHUP | the same | 1001 | no | | id 1 accepted (`FILE_SAVED`), then `:qa!` | id 1 `FILE_SAVED` once, id 2 `DIFF_REJECTED` once | 1001 | no | | a nested `ClaudeCodeDiffOpened` handler runs `:qa!` during the setup of id 2 | id 1 `DIFF_REJECTED`, id 2 a -32603 error, once each | 1001 | no | | the same handler runs `:ClaudeCodeStop`, which is not an exit | the same | 1001 | no | | SIGKILL | nothing | 1006 | yes | | `:qa!` inside an autocmd that is not `nested` | nothing | 1006 | yes | Order of `ClaudeCodeDiffClosed`, with one new-file diff and one existing-file diff, in the `vertical` and `horizontal` layouts, for `:qa!` and SIGTERM (40 runs). With `keep_empty`, the handler of the new-file diff runs while the existing-file diff is still `pending`. With `close_window`, both diffs are `rejected` before the first `ClaudeCodeDiffClosed` fires. A crash was not probed, and SIGKILL stands in for it. Order of the two `VimLeavePre` autocmds. A probe fixed the order that `ARCHITECTURE.md` states. A real WebSocket client holds two open diffs, tracer autocmds sit before and after `setup`, and wrappers log each call. All 15 runs gave one sequence: the `BufUnload` loop answers the first diff, tracer 1, the `diff.lua` autocmd (the reject pass of `cleanup_at_exit` answers the second diff, then `_cleanup_all_active_diffs("shutdown")`), the `init.lua` autocmd (`stop`, `lockfile.remove`, `tcp.stop_server`), tracer 2. The runs cover Neovim 0.12.3, 0.11.5, 0.10.4, 0.9.5 and 0.8.0, each with a plain `setup`, a second `setup` call, and `claudecode.diff` loaded before `setup`. The client got `DIFF_REJECTED` for ids 1 and 2, then close code 1001, and no lock file stayed. The order holds because `M.setup` loads `diff.lua` before it registers its own autocmd. Model (`verify-diff-lifecycle`). The exit was not modeled, and the model's `Idle` condition excluded a call in setup, so the model could not see the case 2 gap. The model now has the action `ExitInSetup` (an exit inside an opened handler during one call's setup), the fact `ExitAnswersInFlight` (read by `check.sh` from `_handle_request` and `M.stop`), and the rule `ExitedCallsAnswered`. A call that nothing answered gets the new phase `gone`. From the first round the model also has `DiffCleanupAtExit` and the fact `ExitRejectsPending`. 1. `check.sh --mutants` ends with `every guard is caught`. `ExitAnswersInFlight=FALSE` is caught by `ExitedCallsAnswered`, so the action is reachable and the old behavior breaks the rule. `ExitRejectsPending=FALSE` is caught by `NoStrandedCall` and `DisconnectedCallsAnswered`. Every other guard is caught as before, and no rule is vacuous. 2. After `--record`, `check.sh` reports PASS: `main: pass (93477 distinct states found)` and `live-DisconnectedCallsAnswered: pass (5557 distinct states found)`. Before this PR the counts were 89205 and 5269. `check.sh --hash-only` reports `covered.lock matches the code`. The model runs the two `VimLeavePre` autocmds of an exit in setup in one order, and it does not model a `ClaudeCodeDiffClosed` handler in the exit pass or a saved diff lost at exit, because Neovim quits right after. `SKILL.md` says so. Gates. The head is `43fc68d619e218995dbba2fc07a128c6ff1fbaee`. It merges `origin/main` at `f7f89cb5`. Items 1 to 5 and 7 ran at that head, on a host where nothing else of mine ran: 1. `mise run all` (`BUSTED_SEED=97531`) exits 0. 1751 successes, 0 failures, 0 errors, 29 pending. luacheck: 0 warnings, 0 errors in 189 files. The 29 pending tests need a real Neovim. 2. The unit specs of the diff and server modules (484 tests) exit 0 with 0 not ok. The three new specs in shuffled order, seeds 1 to 6: each exits 0 with 14 ok. 3. `scripts/run_integration_tests_individually.sh` exits 0 on Neovim 0.12.3 (Total Success 555, Failed 0, Errors 0) and on Neovim 0.8.0, the version of the `integration-tests (v0.8.0)` job (Total Success 404, Failed 0, Errors 0). 4. `mise run format-check` (430 files, 0 changed) and `check.sh --hash-only` exit 0. The full `check.sh` exits 0 with `main: pass (93477 distinct states found)` and `live-DisconnectedCallsAnswered: pass (5557 distinct states found)`. 5. `ExitRejectsPending` is the only fact of `check.sh` that reads the reject pass. This is with `check.sh` as it stands at the head. It reads TRUE at the head. With the pass removed from `cleanup_at_exit`, or moved after the cleanup, it reads FALSE, `check.sh` exits 1, and `NoStrandedCall` and `DisconnectedCallsAnswered` break. 6. `check.sh --mutants` of `verify-diff-lifecycle` ends with `every guard is caught`. It caught `ExitRejectsPending=FALSE` by `NoStrandedCall` and `DisconnectedCallsAnswered`, and `ExitAnswersInFlight=FALSE` by `ExitedCallsAnswered`. An independent verifier ran it on a clean checkout of the head `43fc68d6` (exit 0, 22 guard rows caught). 7. CI runs the `check.sh` of all four `verify-*` skills. `verify-mention-queue` and `verify-selection-tracking` cover `lua/claudecode/server/init.lua`, and their locks went stale. Only the file hash changed in each. No covered block changed. The change cannot reach a queue, a timer, the handshake, a broadcast or the selection state: `M.stop` sends the response of a request whose handler still runs, before it stops the TCP server. The full `check.sh` of each skill passes (mention queue: `main` 270120 distinct states, three live properties; selection tracking: `main` 129986). The 12 mention-queue replays all kept their promise, among them `restart-leak` and `stop-in-wait`, which go through `M.stop`. The four `--hash-only` runs exit 0. Both locks were recorded again after the last commit, which only wraps a comment in `server/init.lua`. --- _Generated with [`xum`](https://github.com/coder/xum) • Model: `anthropic:claude-sonnet-5-5` • Thinking: `xhigh`_
hareki
pushed a commit
to hareki/claudecode.nvim
that referenced
this pull request
Nov 13, 2026
…ing off (coder#541) ## What this fixes With `disable_broadcast_debouncing = true` and an empty queue, `_broadcast_at_mention` broadcast a send at once and skipped the readiness check. A Claude whose WebSocket handshake completed less than `connection_wait_delay` ms ago (600 ms by default) got the mention while it was still starting up. On real Neovim 0.8.0 and 0.12.3 it arrived 3 to 10 ms after the handshake. With the default config the same send waits the full 600 ms. The option is not in the README or the `config.lua` defaults. It is typed in `types.lua` and described in `lua/claudecode/README.md`, so a user config can set it. The fix is in `lua/claudecode/init.lua` (`M._broadcast_at_mention`): the immediate path now also needs `ms_until_a_client_is_ready()` to be 0. That means some connected client completed its handshake `connection_wait_delay` ms ago. Otherwise the mention goes through the queue, which sends it once a client is ready. Behavior is unchanged in two cases: - Some client is ready: the send still goes out at once, with nothing queued. - No client reports a handshake time: the immediate path still runs and fails at once with "Failed to broadcast …", as before. The verify-mention-queue skill (coder#523, now on main) found this through a Codex review on its model. coder#523 excluded this config from its `SentWhenReady` rule as a known bug. coder#523 merged first, so this PR adds the model branch and flips that wording. ## The verify-mention-queue model now covers this path - `model/MentionQueue.tla`: a new variable `immediate` holds the user's `disable_broadcast_debouncing`. Each run starts with either value and keeps it. With it on, a send while Claude is connected and the queue is empty broadcasts at once while some connected client is ready. Otherwise the send queues as before. The writes all succeed, all fail later, or are refused at once, as in a flush. - `check.sh`: a new fact, `ImmediateWaitsForReady`, read from `_broadcast_at_mention`. It reads TRUE only when the readiness guard comes before every `.broadcast(` and `queue_mention(` line of that function (a broadcast written as an indexed call or through an alias still reads TRUE; the unit tests catch those), so an unguarded broadcast added before the guard reads FALSE. With it FALSE, `SentWhenReady` breaks. `FailedWriteReported` now also reads the immediate send's `report_unwritten("command", …)`. - `SKILL.md`: `SentWhenReady` holds with `disable_broadcast_debouncing` on or off. The scope note now describes the check instead of a known bug, and the facts table lists the new fact. ## Tests The first two tests were committed before the fix (ebcdb89) and failed on main: - `tests/unit/mention_queue_spec.lua`, `D: with debouncing off, a send right after the handshake still waits for connection_wait_delay`: a fake loop runs a send 100 ms after the handshake. It expects the mention in the queue, nothing at 590 ms and the mention at 610 ms. On main the queue stayed empty and the mention went out at once. - `tests/integration/mention_immediate_send_spec.lua` (real Neovim, so CI runs it on stable and v0.8.0): a loopback Claude completes the handshake. - A send right after the handshake must reach Claude only once `connection_wait_delay` has passed. The time is measured on the loop clock that stamps `connected_at`. On main: `Claude got the mention 3 ms after its handshake, before 600 ms` (0.8.0) and `6 ms` (0.12.3). - A send once Claude is ready must still go out at once, with nothing queued. This one passes before and after the fix. - `tests/unit/mention_queue_spec.lua`, `D: with debouncing off, a send 1 ms before Claude is ready still waits in the queue` (verifier F1 on 24b7ee7, tightened after the verifier on 700cbf6). The first two tests send 500 ms or more before ready, so a readiness check with a tolerance survived them. The fake clock makes the 1 ms exact on any Neovim. Each of these mutations of the guard fails only this test: `< 2`, `< 20`, `<= 20` and `< 100`. With the fix, the spec passes 53 of 53. ## Replay runner and wording (minor findings from coder#523 round 6) - `replays/run.sh`: it exits 2 before the first replay when `NVIM` names no Neovim, and the error names the path as given. A `NVIM` in a missing folder used to turn into `/nvim`. bash's "Alarm clock" note now goes to the replay's log, not the terminal. The comment no longer cites a VimLeavePre error, because Neovim exits 0 after one. - `drain.lua` and `SKILL.md` now say "an 8.6 s wait before Claude connects" where they said "handshake". ## Checks run The gates ran on 4a06553, with main at d123707. The head then merges main's 37ee1e3 (coder#554, coder#564): `diff_inline.lua`, the integration runner and diff specs. None of them touches the mention queue or a file of this PR, and all four `covered.lock` files match the head. Two results ran on earlier commits: - `--mutants` ran on 3980bc3. Since then `check.sh` changed only in how it reads `ImmediateWaitsForReady`, which still reads TRUE, so each mutant run gets the same TLC input. - The statechart runs ran on 334f122. The model is the same on the head. The results on 4a06553: - `verify-mention-queue/check.sh`: PASS, 20 facts TRUE. `main` 540,240 distinct states, `live-NoStrandedMention` 115,628, `live-ReadyDelivers` 115,628, `live-FlushDelivers` 48,296. Three counts are exactly twice the old ones and `live-FlushDelivers` is 1.96 times (24,692 before), because each run checks both values of `immediate`. - `verify-mention-queue/check.sh --mutants`: exit 0 in 6543 s on a loaded host (load average about 60 to 118). Every guard is caught by the rule `EXPECT` names (`ImmediateWaitsForReady=FALSE` by `SentWhenReady`), and every rule breaks at least once. - A scratch copy of the model with an action property that says the immediate send never happens: TLC breaks it (314 states, 1 mention, 1 connect), so the branch is reachable. With `ImmediateWaitsForReady = FALSE`, `SentWhenReady` breaks. - `/statechart` with the committed view still charts three scratch breaks, `StopClearsQueue` and `FlushWaitsForReady` off. All 3 exited 1 with a chart. - Every other `verify-*/check.sh` passes: - `verify-diff-lifecycle`: 89,205 / 5,269 states. Its lock merged from main. - `verify-selection-tracking`: 129,986 states. - `verify-terminal-lifecycle`: 263,060 states. - The selection and terminal models hash all of `init.lua`, so I re-recorded their `covered.lock` files after the fix. Neither model reads `_broadcast_at_mention`. - `replays/run.sh`: 12 of 12 kept on Neovim v0.8.0 and 12 of 12 on v0.12.3, on the head. - With `NVIM=/nonexistent/nvim`, `./no-such-nvim` or `nvim-not-on-path`, it exits 2 before any replay and names the path. - On a copy of the head, a replay that prints KEPT and exits 1 reads BROKEN. A replay that hangs is ended by the alarm and reads BROKEN with exit 142. "Alarm clock" is in its log, and the terminal got 0 bytes of stderr. - busted, full suite, shuffled (seed 12345): 1769 successes, 0 failures, 0 errors, 29 pending. - `scripts/run_integration_tests_individually.sh`: Neovim v0.8.0, 400 successes, 0 failed, 0 errors. Neovim v0.12.3, 537 successes, 0 failed, 0 errors. `mention_immediate_send_spec.lua` passed 2 of 2 on each version. - `ImmediateWaitsForReady` reads TRUE on the head. With an unguarded broadcast before the guard (the verifier's probe R4), it reads FALSE. - luacheck: 0 warnings in 186 files. `treefmt --ci`: 0 files changed. ## Repro ```lua -- In a headless Neovim with the plugin on 'runtimepath', CLAUDE_CONFIG_DIR set to a temp dir: require("claudecode").setup({ terminal = { provider = "none" }, track_selection = false, disable_broadcast_debouncing = true }) -- Connect a WebSocket client, complete the handshake, then right away: require("claudecode").send_at_mention(vim.fn.getcwd() .. "/a.txt", nil, nil, "repro") -- Before this fix the client gets the at_mentioned frame within a few ms. After it, it arrives 600 ms after the handshake. ``` `tests/integration/mention_immediate_send_spec.lua` is the runnable form. --- _Generated with [`xum`](https://github.com/coder/xum) • Model: `anthropic:claude-opus-5-5` • Thinking: `xhigh`_
hareki
pushed a commit
to hareki/claudecode.nvim
that referenced
this pull request
Nov 13, 2026
…oder#577) ## Why In the `vertical` and `horizontal` layouts, a proposal whose line breaks are all CRLF was loaded into the proposal buffer with `'fileformat'` `unix` and a CR at the end of each line. The save joined the lines with LF. So a line that the user typed, or the join of two lines, came back with LF, or with a CR in the middle of a line, where Neovim's `:w` writes CRLF. A CRLF proposal that equals a CRLF file also showed every line as changed, because the file's own buffer is `dos` and holds its lines without CRs. I found it while verifying coder#551. ## What changed 1. `_setup_blocking_diff` (`lua/claudecode/diff.lua`) loads a proposal as Neovim loads a file. When `'fileformats'` includes `dos`, and the proposal has a line break and only CRLF line breaks, the buffer gets `'fileformat'` `dos` and lines without their CRs. Any other proposal gets `'fileformat'` `unix` and is split at LF, as before. The diff sets `'fileformat'` in both cases, because a new buffer takes the first entry of `'fileformats'`, and that entry can be `dos`. 2. `_resolve_diff_as_saved` joins the lines with the line break of the buffer's `'fileformat'` (LF, CRLF for `dos`, CR for `mac`), and adds the final one when `'eol'` is on. Before, it always used LF. So `:setlocal fileformat=dos` on an LF proposal now answers CRLF, as `:w` writes it. 3. The `unified` layout does not change. Its buffer is read-only, and its answer is the proposal. 4. `lua/claudecode/README.md` states both rules, and that one empty line is saved as one line break of the buffer's `'fileformat'`. 5. `tests/integration/diff_accept_crlf_spec.lua` is new (159 tests). `tests/mocks/vim.lua` gives the mock `vim.o` the default `fileformats = "unix,dos"`, because the unit specs of the diff now read it. 6. `covered.lock` of `verify-diff-lifecycle` records the new hashes of `diff.lua` (the file, `resolve_saved` and `setup_blocking`). ## What a user sees 1. A line that the user types into an accepted CRLF proposal, or two lines that the user joins, comes back with CRLF, as `:w` writes it. For the proposal `"a\r\nb\r\n"` and a typed line after the last line, main answers `"a\r\nb\r\nx\n"` and this PR answers `"a\r\nb\r\nx\r\n"`. 2. A CRLF proposal that equals the CRLF file shows no difference. Main marked every line. 3. An unedited proposal still comes back as exactly the proposal, in every layout, under every `'fileformats'` value that I ran. 4. Nothing else changes for the default `'fileformats'` (`unix,dos`): the controls of the spec (an LF proposal, a proposal with CRLF and LF line breaks, a lone CR, no line break, unedited, emptied) give the same bytes on main and here. 5. `'fileformats'` `unix` (no `dos`) keeps a CRLF proposal as `unix` with its CRs, as the file's own buffer does. The equal-text view shows no difference there, and a typed line gets LF, as `:w` writes it. ## Verification The spec compares the answer with the bytes that Neovim's own `:w` writes for the same text after the same commands (an oracle that reads the text as a file under the same `'fileformats'`), or with an exact expected text. 1. Red, then green, run on Neovim 0.12.3, 0.11.5, 0.10.4, 0.9.5 and 0.8.0 (all five give the same counts): 159 tests pass with the fix. With `origin/main`'s `diff.lua`, 105 pass and 54 fail. The unified cases run on 0.8.0 too (coder#564). 2. Fourteen mutants of the changed lines are each red on Neovim 0.12.3 (the last seven also on 0.8.0, with the same counts). Failed tests per mutant: - Split and join: the CRs stay in the lines 70, the join always uses LF 66, the final break is always LF 56, `mac` is ignored 2, dos for any text with a CRLF 12, dos for a text with no line break 2, dos even with a lone LF 32. - `'fileformats'` check: no check (always dos) 8, an exact match of `unix,dos` or `dos,unix` 4, dos for any value except `unix` 4, dos only when it is the first entry 42, `'fileformat'` always `unix` 64, always `dos` 48, set only for `dos` 12. 3. A probe of 1860 cases per run on Neovim 0.12.3 and 0.8.0 (three layouts, four old files, 15 proposals, 15 edits) compares each answer with the bytes of a fresh `nvim --clean` that reads the proposal as a file under the same `'fileformats'`, runs the same commands and writes with `:w`. The columns give mismatches. The two Neovim versions give the same numbers. | `'fileformats'` | main | this PR | unedited answers that differ from the proposal (main / this PR) | |---|---|---|---| | `unix,dos` | 748 | 144 | 0 / 0 | | `dos,unix` | 788 | 184 | 0 / 0 | | `unix` | 304 | 192 | 0 / 0 | | `dos` | 1104 | 516 | 0 / 0 | | `unix,mac` | 608 | 544 | 0 / 0 | Every mismatch that remains is one of the items under "Not fixed here", or a fold (see below). Cleanup leaves no state, buffer, window or tab behind in any case. Under `unix,dos`, `dos,unix` and `unix`, the equal-text view marks 0 lines in every case. 4. The only cases where this PR has a mismatch that main does not have are `dup_line` and `dd_first` (`yyp` and `dd` on line one) against an old file that equals the proposal, for CRLF proposals: 28 cases for each of `unix,dos`, `dos,unix` and `dos`. Diff mode folds every line of an equal text, so those commands act on the whole fold. Main does the same for an equal LF text (`same|lf|dup_line`), and showed every line of an equal CRLF text as changed, so it did not fold. This follows from item 2 of "What a user sees". 5. An independent verifier ran its own probe (2604 cases per run) under `unix,dos`, `dos,unix`, `unix` and `dos` on all five Neovim versions, and under `unix,dos,mac`, `unix,mac` and `dos,mac` on 0.12.3. In every run, 0 unedited answers differ from the proposal. ## Not fixed here These items are tracked in coder#581. All of them exist on `origin/main` already. An independent probe that compares each answer with `:w` of the same buffer found this PR never worse than main. 1. A CR inside a line, then `:setlocal ff=mac`. `:w` writes that CR as NL, and the save keeps the CR. The proposal `"a\rb\n"` gives `"a\rb\r"`, and `:w` gives `"a\nb\r"`. Main joined with LF, which was also wrong. 2. With `'fileformats'` `unix,mac`, Neovim reads a text such as `"a\r\nb\r\r\n"` as `mac`. The diff never detects `mac` (main neither). In 24 cases (an `ff=mac` edit only), main's answer equals the file-read oracle and this PR's answer does not, because main ignores `'fileformat'` when it joins. Against `:w` of the same buffer, both are wrong in these 24 cases. 3. An empty proposal, then typed lines. `'eol'` stays off (coder#542), so `Gox` gives `"\nx"`, where `:w` of an empty file gives `"\nx\n"`. 4. `'fileformats'` that starts with `dos`, and a proposal with no LF (`""`, `"a"`, `"a\r"`). Neovim reads it as `dos`. The diff keeps `unix`, so a typed line gets LF where `:w` writes CRLF. 5. `'fileformats'` with the single entry `dos`. Neovim then reads every file as `dos`, an LF file too. An LF or mixed proposal stays `unix`, so a typed line gets LF where `:w` of the file writes CRLF. Neovim reads the equal file as `dos`, so the equal-text view still marks lines for a text that is not all CRLF: this PR marks 2 of the 15 probe texts (a mixed text, and one with a leading lone LF), and main marks 11 of 15. ## Gates The head is `1c7b5adcfc165963ffc250ab9ebeedad00ddf954`. It merges `origin/main` at `7dbad04b`. Main moved three times while this PR was in progress, and each time only `covered.lock` conflicted. The gates below ran in full at `4525dad1` (the merge with `origin/main` at `c43b6a30`). The last merge brought coder#574. At the head I ran the checks of item 6, and CI runs the rest on this head. 1. `mise run all` (`BUSTED_SEED=97531`) exits 0: 1837 successes, 0 failures, 0 errors, 34 pending. luacheck: 0 warnings, 0 errors in 194 files. 2. `mise run format-check` exits 0. 3. `scripts/run_integration_tests_individually.sh` exits 0 on Neovim 0.12.3 (Total Success 828, Failed 0, Errors 0) and on Neovim 0.8.0 (Total Success 810, Failed 0, Errors 0). The new spec alone gives 159 passes on Neovim 0.12.3, 0.11.5, 0.10.4, 0.9.5 and 0.8.0. 4. `check.sh --hash-only` of the four `verify-*` skills exits 0. The full `check.sh` of `verify-diff-lifecycle` exits 0 (`main` 93477 distinct states, `live-DisconnectedCallsAnswered` 5557). The merge with main conflicted in `covered.lock` only. I took my side, ran the full `check.sh` on the merged tree (all 23 facts TRUE, the model passes), and then ran `--record`. The last merge moved only `file` hashes (`diff.lua`, `init.lua`, `selection.lua`) and no block. Against `origin/main`, the lock moves 3 lines: the file hash of `diff.lua`, and the blocks `resolve_saved` and `setup_blocking`. No guard moved. The change decides which bytes the accepted text holds. It does not change a diff's status, who gets its answer, or when. 5. At the head, after the last merge: the full `check.sh` of `verify-diff-lifecycle` (all 23 facts TRUE, `main` 93477, `live-DisconnectedCallsAnswered` 5557) and then `--record`. coder#574 changed the blocks `open_blocking`, `handle_invoke` and `setup_blocking` on main, and the lock of this PR moves only the 3 lines of this PR against `origin/main` (the file hash of `diff.lua`, `resolve_saved`, `setup_blocking`). 6. At the head: `format-check` exits 0, `check.sh --hash-only` of the four skills exits 0, the new spec gives 159 passes on Neovim 0.12.3 and 0.8.0, and the 44 unit specs of the diff, tools and server modules give 752 successes, 0 failures. --- _Generated with [`xum`](https://github.com/coder/xum) • Model: `anthropic:claude-sonnet-5-5` • Thinking: `xhigh`_
hareki
pushed a commit
to hareki/claudecode.nvim
that referenced
this pull request
Nov 13, 2026
… exit answers docs (coder#572) ## Why Keep the exit tests and the "Exit answers" docs true after coder#558 and coder#564. 1. coder#564 made the unified layout work on Neovim 0.8.0 and removed the "needs Neovim 0.9" skips from eight specs. `diff_exit_reject_spec.lua` came in with coder#558 and still skipped the unified layout below 0.9 in three places. So 14 unified exit scenarios stayed pending in the v0.8.0 job, and the file kept a comment that is no longer true. 2. The advisor and the verifier of coder#558 found README sentences that say more than the code does. They concern the exits that run no `VimLeavePre`. ## What changed - `tests/integration/diff_exit_reject_spec.lua`: - The three skips and the comment "The unified layout needs Neovim 0.9" are gone. - Three new cases (each in the vertical and unified layouts) start the exit inside an autocmd, with two waiting calls. A direct `:qa!` in an autocmd that is not `nested`, and SIGTERM while such an autocmd waits in `vim.wait`: no call is answered. A direct `:qa!` in a `nested` autocmd: both calls get `DIFF_REJECTED`. Each case first checks that both calls were waiting. The two no-answer cases also check that the child ended by itself. - The spec has 34 passes on Neovim 0.8.0 and 34 on 0.12.3. On main it has 14 passes and 14 pending on 0.8.0, and 28 passes on 0.12.3. - `README.md`, Global Invariants, "Exit answers": - The `"close_window"` sentence now says "waiting calls". A call that a nested `:qa!` cuts off during its `openDiff` setup has no response sender yet. The server stop answers it with -32603 after the `ClaudeCodeDiffClosed` events. Every call still gets one answer before close code 1001. - The sub-bullet for exits without `VimLeavePre` keeps SIGKILL, a crash and a `:qa!` from inside an autocmd that is not `nested`, as on main, and adds `:noautocmd qa!` and a Lua `os.exit()`, which also run no `VimLeavePre`. It adds one rule: a `:qa!`, SIGTERM or SIGHUP that Neovim handles while an autocmd that is not `nested` is still running can end Neovim without `VimLeavePre` too. A synchronous LSP format on save, which waits in `vim.wait`, is the common example. It says "can" because Neovim looks only at the autocmd that runs when it handles the exit, so a `nested = true` handler that such an autocmd started (with `:doautocmd` or `nvim_exec_autocmds`) still gets `VimLeavePre`. It promises nothing for a signal during a plain Lua loop, because Neovim handles a signal only at its next event check. - `ARCHITECTURE.md`, `lua/claudecode/README.md` and the comment of `cleanup_at_exit` in `lua/claudecode/diff.lua` say the same. ## Scope Tests and docs. The only change under `lua/` is that comment in `diff.lua`. `verify-diff-lifecycle` passes (93,477 distinct states, and 5,557 for the live case), and its `covered.lock` records the new file hash. The plugin does not answer calls in the exits that the README lists as running no `VimLeavePre` (SIGKILL, a crash, `:noautocmd qa!`, `os.exit()`, and `:qa!` in an autocmd that is not `nested`), because none of its exit code runs in them. The docs say so. ## Blast Radius None at runtime. The v0.8.0 CI job runs 20 more tests of this spec, and the stable job runs 6 more. ## Verification - Spec on Neovim 0.8.0 and 0.12.3: 34 passes, 0 failures, in 5 of 5 runs on each version, and in 3 of 3 runs again after the last commits, which change comments, docs and the lock only. Mutants: - The autocmd's `:qa!` replaced by a no-op: the first no-answer case is red on both versions (the file then runs into the runner's 40 s limit, because each affected case waits 20 s). - `cleanup_at_exit` returns at once: 8 cases are red on both versions, among them the nested `:qa!` case in the vertical layout. The no-answer cases stay green, because they pin what Neovim does. A Neovim that changes it turns them red, and the README has to change. - Probes on 0.8.0 and 0.12.3 (vertical and unified, two waiting calls, 6 runs per row, a `VimLeavePre` marker and the answers in one log). In an autocmd that is not `nested`, each of these runs no `VimLeavePre` and answers no call, in all 54 runs per version: a direct `:qa!`, a timer `:qa!` during `vim.wait`, SIGTERM and SIGHUP during `vim.wait`, SIGTERM and SIGHUP in the Lua part of a handler that waits later, SIGTERM in the first of two handlers when the second waits, SIGTERM in a Vim script loop, and SIGTERM in a `:%s` loop. In a `nested = true` autocmd the same cases answer both calls. With no autocmd, a timer `:qa!`, SIGTERM and SIGHUP answer both calls (18 of 18). `:noautocmd qa!` and a Lua `os.exit()` from a timer run no `VimLeavePre` and answer no call (9 of 9 on each version, three layouts), and a plain `:qa!` and `:cq` run it and answer both calls (9 of 9). A non-nested autocmd that starts a `nested = true` handler which waits in `vim.wait`, with SIGTERM during the wait, runs `VimLeavePre` and answers both calls (6 of 6 on each version). - Waiting calls with `close_window`, `:qa!` and SIGTERM, three layouts: both answers come before the first `ClaudeCodeDiffClosed` (6 of 6 on each version). A nested `:qa!` that cuts a call off in its setup: the -32603 comes after every `ClaudeCodeDiffClosed` event (12 of 12 on each version), and no id is answered twice. - Independent reads with a real WebSocket client on Neovim 0.8.0, 0.9.5, 0.10.4, 0.11.5 and 0.12.3: a verifier (330 runs), an advisor (225 runs) and two claim checks (375 runs in the first, and 140 runs for the nested-handler case in the second). They found that the first wording dropped the direct `:qa!` case, that the second promised `VimLeavePre` for a signal during a Lua loop, and that "ends" is false when the running autocmd started a `nested` handler. The sub-bullet above says only what survives all three, with "can". A last narrow check found only that "Without `VimLeavePre`" was too general, because `eventignore` can skip `VimLeavePre` and a `:qa!` still rejects diffs while buffers unload. The README now says "In these exits". - At the head after the merge of main (coder#573): `mise run format-check` and `mise run all` pass (1892 successes, 0 failures, 0 errors, 35 pending), and the integration runner passes on 0.12.3 (844) and on 0.8.0 (840). The commits of this PR change comments, docs, the spec header and the lock only. `check.sh --hash-only` passes for the four verify skills, and the full `verify-diff-lifecycle` check passes (93,477 and 5,557 states). --- _Generated with [`xum`](https://github.com/coder/xum) • Model: `anthropic:claude-sonnet-5-5` • Thinking: `xhigh`_
ThomasK33
added a commit
that referenced
this pull request
Nov 13, 2026
The guard said the unified layout needs Neovim 0.9. Main dropped that claim in #564, and the 24 cases pass on v0.8.0. _Generated with xum • Model: anthropic:claude-sonnet-5-5 • Thinking:
hareki
pushed a commit
to hareki/claudecode.nvim
that referenced
this pull request
Nov 14, 2026
## 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`_
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why
The README says the
unifieddiff layout needs Neovim 0.9. It does not. The layout usesvim.diff, and Neovim 0.8.0 has that function. The 0.9 claim made eight integration specs skip everyunifiedcase on Neovim 0.8.0, so the 0.8.0 CI job ran no accept, reject,:q, window-size or trailing-line-break check for that layout. A regression there would have passed CI.What changed
README.md(thediff_opts.layoutcomment) no longer says "Requires Neovim >= 0.9.0".lua/claudecode/README.md(theunifiedbullet) now says that Neovim 0.8.0 hasvim.diff, so the layout runs on every Neovim the plugin supports.unifiedcases now run on every Neovim:diff_quit_reject_spec.lua,diff_window_choice_spec.lua,diff_window_sizes_spec.lua,diff_close_save_spec.lua,diff_hand_over_spec.lua,diff_accept_trailing_newlines_spec.lua,diff_accept_missing_final_newline_spec.lua,diff_accept_emptied_buffer_spec.lua. Indiff_quit_reject_spec.luaanddiff_window_choice_spec.luathe skip covered the whole file.setup_inline_diffinlua/claudecode/diff_inline.luaraised-32000with "Inline diff requires Neovim >= 0.9.0" when Neovim has no diff function. The guard checks for the function (vim.text.difforvim.diff) and never for a version, so the message now says what is missing: "The unified layout needs vim.text.diff or vim.diff, and this Neovim has neither". A unit test drives the guard with both functions removed. It fails on the old message (50 successes, 1 failure) and passes on the new one (51 successes).diff_hand_over_spec.luapicks:w ++ponly when Neovim has it (0.9 and later) and amkdirplus:won 0.8.0. That is a real difference between the versions, and it is not about theunifiedlayout.diff_window_sizes_spec.lua, the case "gives the sizes back when the unified diff's setup fails" checked only that some error came back. With the unified path broken on Neovim 0.8.0 (a diff function that ignoresvim.diff), it stayed green, because the guard raised an error too. It now asserts theDiff setup failedmessage and that the error data carries the failure of the user's autocmd. With that broken path it fails on 0.8.0, and it passes on 0.8.0 and 0.12.3.The tree holds no other 0.9 claim about the
unifiedlayout. I checked every0.9andnvim-0.9line in code, docs, comments, error strings, specs, scripts and the CI workflow. The ones that remain are about something else:nvim -l(0.9 and later only),statuscolumnand:w ++pgates indiff.luathat are correct, the LuaJIT bug in Neovim 0.8 and 0.9, visual mode in 0.8 and 0.9, and the RPC process model.CHANGELOG.mdand its fixture copy keep their old lines, because they describe what the release did when it shipped.diff_inline.luais covered by the diff lifecycle model. The change touches only thesetup_inlineblock, and only a comment and an error string in it, so no behavior changed. The model passes (89205 states, every fact TRUE) andcovered.lockis recorded for the new file.How I checked it
The specs ran on a real Neovim 0.8.0 (
v0.8.0-1210-gd367ed9b2) before and after the change:diff_quit_reject_spec.luadiff_window_choice_spec.luadiff_window_sizes_spec.luadiff_close_save_spec.luadiff_hand_over_spec.luadiff_accept_trailing_newlines_spec.luadiff_accept_missing_final_newline_spec.luadiff_accept_emptied_buffer_spec.luaThat is 133 more tests on Neovim 0.8.0, with 0 failures and 0 errors. 13 of the 133 are vertical-layout cases of
diff_quit_reject_spec.lua, which the file's 0.9 skip also hid. Three other specs (diff_new_tab_bufenter_spec.lua,diff_new_tab_mode_spec.lua,diff_terminal_float_spec.lua) already ran theirunifiedcases on 0.8.0 without a 0.9 skip, and they pass.I also ran a unified diff by hand in the real Neovim 0.8.0 terminal UI (tmux, 110 columns). The proposal changed
twotoTWOand addedfour. The screen showed a red-sign and a red-tinted line for the deleted row, and a green+sign and a green-tinted line for each added row.:wansweredFILE_SAVEDwith the exact proposal textone\nTWO\nthree\nfour\n.:qansweredDIFF_REJECTED.Gates on the final commit
598b0acc, which mergesorigin/main523ee20 into the branch (private XDG folders). Since #545,mise run allalso runs the static proofs of theverify-claudecode-nvimskill, and they pass:mise run all: native exit 0. luacheck: 0 warnings / 0 errors in 185 files. busted: 1738 successes / 0 failures / 0 errors / 28 pending. treefmt: 0 files changed.mise run format-check: native exit 0 (427 files, 0 changed).scripts/run_integration_tests_individually.shon Neovim 0.12.3: 527 successes, 0 failed, 0 errors, native exit 0. On Neovim 0.8.0: 523 successes, 0 failed, 0 errors, native exit 0 (390 onorigin/main, 133 more now).verify-diff-lifecycle/check.sh, then--record, then all four.agents/skills/verify-*/check.sh --hash-only: every one matches itscovered.lock.Left as it is
CHANGELOG.mdkeeps its old lines about 0.9 and about the 0.8.0 job skippingunified. They describe what the release did when it shipped.Generated with
xum• Model:anthropic:claude-sonnet-5-5• Thinking:xhigh