From b956792bb19cb1e8c7a5b137eec8bc98afa71966 Mon Sep 17 00:00:00 2001 From: Thomas Kosiewski Date: Thu, 12 Nov 2026 01:57:11 +0000 Subject: [PATCH 01/11] fix(diff): answer every pending openDiff when Neovim exits 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. The keep_empty reject of a new-file diff deletes its proposal buffer, so the diffs behind it got no BufUnload. They reached the VimLeavePre autocmd pending, which removed them without an answer, and server.stop then found nothing to reject. Claude waited on an openDiff that was never answered. The VimLeavePre callback now rejects every diff that is still pending, oldest first, before it removes any entry. A diff that a BufUnload already rejected is skipped, so each openDiff gets one answer. One diff that fails to reject cannot leave the others waiting. The new caller resolves diffs, so the verify-diff-lifecycle model gets an exit action and a fact for the guard: the fact set to FALSE breaks NoStrandedCall and DisconnectedCallsAnswered, and check.sh --mutants catches every guard. covered.lock records the new hash. A unit spec drives the VimLeavePre callback over the states exit can leave. An integration spec runs a real :qa! in a child Neovim for 20 scenarios. --- .agents/skills/verify-diff-lifecycle/SKILL.md | 4 +- .agents/skills/verify-diff-lifecycle/check.sh | 15 +- .../skills/verify-diff-lifecycle/covered.lock | 8 +- .../model/DiffLifecycle.tla | 22 +++ .../model/statechart-view.ts | 1 + lua/claudecode/README.md | 3 +- lua/claudecode/diff.lua | 33 +++- tests/integration/diff_exit_reject_spec.lua | 149 ++++++++++++++++++ tests/unit/diff_exit_reject_spec.lua | 135 ++++++++++++++++ 9 files changed, 362 insertions(+), 8 deletions(-) create mode 100644 tests/integration/diff_exit_reject_spec.lua create mode 100644 tests/unit/diff_exit_reject_spec.lua diff --git a/.agents/skills/verify-diff-lifecycle/SKILL.md b/.agents/skills/verify-diff-lifecycle/SKILL.md index 54a65b428..6da90183c 100644 --- a/.agents/skills/verify-diff-lifecycle/SKILL.md +++ b/.agents/skills/verify-diff-lifecycle/SKILL.md @@ -54,7 +54,7 @@ The record has one group of lines per file: a `== ` header, the file's has Covered blocks: -- In `diff.lua`: `_resolve_diff_as_saved`, `_resolve_diff_as_rejected`, `open_diff_blocking`, `_setup_blocking_diff`, `close_diff_by_tab_name`, `close_pending_diffs`, `close_diffs_for_client`, `close_active_diffs`, `_cleanup_all_active_diffs`, `_register_diff_state`, `_cleanup_diff_state`, `register_diff_autocmds` (the `:w` and buffer-delete paths), `_write_proposal` (what a write of the proposal does), `_register_reject_when_hidden` (the `:q` and buffer-switch paths, shared by both layouts), `fire_diff_event`, `accept_current_diff`, `deny_current_diff`. +- In `diff.lua`: `_resolve_diff_as_saved`, `_resolve_diff_as_rejected`, `open_diff_blocking`, `_setup_blocking_diff`, `close_diff_by_tab_name`, `close_pending_diffs`, `close_diffs_for_client`, `close_active_diffs`, `_cleanup_all_active_diffs`, `_reject_pending_diffs`, `cleanup_at_exit` (the `VimLeavePre` callback), `_register_diff_state`, `_cleanup_diff_state`, `register_diff_autocmds` (the `:w` and buffer-delete paths), `_write_proposal` (what a write of the proposal does), `_register_reject_when_hidden` (the `:q` and buffer-switch paths, shared by both layouts), `fire_diff_event`, `accept_current_diff`, `deny_current_diff`. - In `diff_inline.lua`: `setup_inline_diff`, `resolve_inline_as_saved`, `resolve_inline_as_rejected`. - In `server/init.lua`: `M.stop`, `on_disconnect`, `M._handle_request`, `M._setup_deferred_response`. - In `tools/init.lua` and `tools/open_diff.lua`: `M.handle_invoke`, and the openDiff `handler`. @@ -76,6 +76,7 @@ A changed file with no changed block or call site still needs a look. A wrapper | `CloseTabRejectsPending` | `close_diff_by_tab_name` rejects a pending diff before cleanup | | `CloseEachByName` | `close_active_diffs` closes each match through `close_diff_by_tab_name` | | `StopRejectsPending` | `M.stop` calls `close_pending_diffs`, then `stop_server`, then clears the senders | +| `ExitRejectsPending` | the `VimLeavePre` callback `cleanup_at_exit` calls `_reject_pending_diffs` before `_cleanup_all_active_diffs`, so a pending diff is answered, not removed | | `StopPendingOnly`, `DisconnectPendingOnly` | the `status == "pending"` filters in `close_pending_diffs` and `close_diffs_for_client` | | `DisconnectRejects` | `on_disconnect` schedules `close_diffs_for_client` | | `UserCloseAllPendingOnly` | `:ClaudeCodeCloseAllDiffs` calls `close_pending_diffs`, never `close_all_diffs` | @@ -121,6 +122,7 @@ Then decide which kind of failure it is: - The invariants run with 3 openDiff calls, 2 tab names and 2 clients, about 89,000 states. `DisconnectedCallsAnswered` runs with 2 calls, 2 tabs and 2 clients. It assumes weak fairness only on setup and on the scheduled disconnect cleanup. The user is never assumed to act, with one exception: the disconnect cleanup waits until Neovim allows window changes (`utils.when_windows_allowed`), so a user in the command-line window is assumed to leave it, and textlock is assumed to end. - In every run, a user `ClaudeCodeDiffOpened` handler (README "Diff Lifecycle Events") can accept or deny the diff during setup: `:ClaudeCodeDiffAccept`, `:ClaudeCodeDiffDeny`, or a `nested = true` autocmd that closes the proposal window. A default handler that closes the window resolves nothing, because WinClosed does not fire inside a non-nested autocmd. Other handler actions during setup are not modeled. +- `DiffCleanupAtExit` is the `VimLeavePre` callback of `diff.lua`: `_reject_pending_diffs`, then `_cleanup_all_active_diffs("shutdown")`. It models neither a `ClaudeCodeDiffClosed` handler (the reject pass has none unless a `keep_empty` reject runs a cleanup) nor a saved diff lost at exit: Neovim quits right after the step, and its buffers go with it. It can run in the same step as `ServerStops` in either order, because both are `VimLeavePre` autocmds. Each is one action here, and the model runs them in any order. Neovim's own exit `BufUnload` loop, which rejects before `VimLeavePre`, is the `UserRejects` action. - `SenderCleared` alone is caught by no rule, because the pending guards also stop a second answer. `--mutants` shows that `AtMostOneAnswer` breaks when both `SenderCleared` and `GuardRejected` are removed. - `SavedDiffKept` knows which client owns each diff, and, while `:ClaudeCodeCloseAllDiffs` runs, which files or file buffers hold the accepted text. The model has no file or buffer contents: the command's first pass leaves the accepted text in any subset of the saved diffs' files or buffers, because the code skips a diff whose file buffer already has unsaved changes, or whose buffer it cannot change. The plugin never writes the file. That the first pass leaves the file on disk and the user's unsaved changes alone is checked by the unit tests, not here. A saved diff counts as lost when the user, a disconnect cleanup or a server stop tears it down, or when a client that does not own it does so while the owner is disconnected. A close by another client while the owner is connected counts as safe, because the owner got `FILE_SAVED` and can still write the file. The code is stricter than the rule: it keeps another client's saved diff in both cases. Ownership compares client ids, so a new connection must never get an old id. With `ClientIdsUnique` off, `ClientIdReused` lets a new connection take the id of a disconnected client. - The split and unified layouts share the pending guards only because `resolve_inline_as_*` is reached through `_resolve_diff_as_*`. The inline functions have no guard of their own. diff --git a/.agents/skills/verify-diff-lifecycle/check.sh b/.agents/skills/verify-diff-lifecycle/check.sh index 4e980b15f..059af0d33 100755 --- a/.agents/skills/verify-diff-lifecycle/check.sh +++ b/.agents/skills/verify-diff-lifecycle/check.sh @@ -50,6 +50,8 @@ covered_block setup_deferred "$SERVER" '^function M\._setup_deferred_response\(' covered_block user_close_all "$MAIN" 'nvim_create_user_command\("ClaudeCodeCloseAllDiffs"' covered_block close_active "$DIFF" '^local function close_active_diffs\(' covered_block cleanup_all "$DIFF" '^function M\._cleanup_all_active_diffs\(' +covered_block reject_pending "$DIFF" '^function M\._reject_pending_diffs\(' +covered_block exit_cleanup "$DIFF" '^local function cleanup_at_exit\(' covered_block inline_saved "$INLINE" '^function M\.resolve_inline_as_saved\(' covered_block inline_rejected "$INLINE" '^function M\.resolve_inline_as_rejected\(' covered_block handle_invoke "$TOOLS" '^function M\.handle_invoke\(' @@ -137,6 +139,17 @@ fact[ClientIdsUnique]=$(as_bool client_ids_unique) fact[StopRejectsPending]=$(as_bool in_order 'diff\.close_pending_diffs\(' 'tcp_server\.stop_server\(' \ '_G\.claude_deferred_responses = \{\}' <<<"$server_stop") fact[StopPendingOnly]=$(as_bool grep -qE '^ *return diff_data\.status == "pending"$' <<<"$close_pending") +# At exit, a diff that no BufUnload rejected still gets its answer: the VimLeavePre autocmd rejects every pending diff +# before _cleanup_all_active_diffs removes any entry. +FACTS+=(ExitRejectsPending) +exit_rejects_pending() { + in_order '^function M\._reject_pending_diffs\(\)$' 'diff_data\.status == "pending"' \ + 'pcall\(M\._resolve_diff_as_rejected, entry\.tab_name\)' <<<"$reject_pending" && + in_order '^local function cleanup_at_exit\(\)$' '^ M\._reject_pending_diffs\(\)$' \ + '^ M\._cleanup_all_active_diffs\("shutdown"\)$' <<<"$exit_cleanup" && + in_order '^vim\.api\.nvim_create_autocmd\("VimLeavePre", \{$' '^ callback = cleanup_at_exit,$' <"$DIFF" +} +fact[ExitRejectsPending]=$(as_bool exit_rejects_pending) fact[DisconnectRejects]=$(as_bool in_order 'vim\.schedule\(function\(\)' 'diff\.close_diffs_for_client\(client_id' \ <<<"$on_disconnect") fact[DisconnectPendingOnly]=$(as_bool grep -qE \ @@ -208,7 +221,7 @@ CONSTRAINT=AnswerBound # it. SetupAnswerReturned=FALSE is caught only through that step, so its row also shows that the step is reachable. EXPECT=( [GuardSaved]=AnswerMatchesDiff [GuardRejected]=AnswerMatchesDiff - [ReplaceRejects]=NoStrandedCall [CloseTabRejectsPending]=NoStrandedCall [CloseEachByName]=NoStrandedCall [StopRejectsPending]=NoStrandedCall + [ReplaceRejects]=NoStrandedCall [CloseTabRejectsPending]=NoStrandedCall [CloseEachByName]=NoStrandedCall [StopRejectsPending]=NoStrandedCall [ExitRejectsPending]=NoStrandedCall [SetupAnswerReturned]=NoStrandedCall [StopPendingOnly]=SavedDiffKept [DisconnectPendingOnly]=SavedDiffKept [UserCloseAllPendingOnly]=SavedDiffKept [UserCloseKeepsAcceptedText]=SavedDiffKept diff --git a/.agents/skills/verify-diff-lifecycle/covered.lock b/.agents/skills/verify-diff-lifecycle/covered.lock index 72bef18b6..e2c3897b0 100644 --- a/.agents/skills/verify-diff-lifecycle/covered.lock +++ b/.agents/skills/verify-diff-lifecycle/covered.lock @@ -3,7 +3,7 @@ file lua/claudecode/config.lua 787fa0432d1d77d7 == lua/claudecode/cwd.lua file lua/claudecode/cwd.lua 55d26ea362173afa == lua/claudecode/diff.lua -file lua/claudecode/diff.lua a21129cdc802e417 +file lua/claudecode/diff.lua f6eb2eb21448cb11 block resolve_saved 46d5f3f732711fe1 block resolve_rejected c7a620816abff9a1 block open_blocking 92067df7f9d4d4d9 @@ -20,6 +20,8 @@ block put_in_buffer c5dedfca8261379b block held_for_other 3d1e2911d5310cc2 block close_active 446fbed632748300 block cleanup_all 43e7b595cf106229 +block reject_pending b00a72037446e3bf +block exit_cleanup 5e7246c612ddb96a block accept_current 7577efa33506569e block fire_event ef778bb39419d91a block register_state 8cf78b6711a9ecc6 @@ -28,7 +30,7 @@ block diff_autocmds 01fff2946c5a2149 block write_proposal 9baeff1cd3273193 block reject_when_hidden dd45932e5cd7090d block deny_current 94385d781b70d50c -site lua/claudecode/diff.lua: M._cleanup_all_active_diffs("shutdown") @ vim.api.nvim_create_autocmd("VimLeavePre", { +site lua/claudecode/diff.lua: M._cleanup_all_active_diffs("shutdown") @ local function cleanup_at_exit() site lua/claudecode/diff.lua: M._cleanup_diff_state(tab_name, "diff rejected (keep_empty)") @ function M._resolve_diff_as_rejected(tab_name) site lua/claudecode/diff.lua: M._cleanup_diff_state(tab_name, "diff tab closed after reject") @ function M.close_diff_by_tab_name(tab_name, client_id) site lua/claudecode/diff.lua: M._cleanup_diff_state(tab_name, "diff tab closed after reject") @ function M.close_diff_by_tab_name(tab_name, client_id) @@ -62,6 +64,7 @@ site lua/claudecode/diff.lua: for _, diff_data in pairs(active_diffs) do @ funct site lua/claudecode/diff.lua: for _, diff_data in pairs(active_diffs) do @ function M.close_saved_diffs(reason) site lua/claudecode/diff.lua: for _, other in pairs(active_diffs) do @ local function save_window_sizes(tab) site lua/claudecode/diff.lua: for tab_name, _ in pairs(active_diffs) do @ function M._cleanup_all_active_diffs(reason) +site lua/claudecode/diff.lua: for tab_name, diff_data in pairs(active_diffs) do @ function M._reject_pending_diffs() site lua/claudecode/diff.lua: for tab_name, diff_data in pairs(active_diffs) do @ local function close_active_diffs(filter_fn, reason, client_id) site lua/claudecode/diff.lua: function M._cleanup_all_active_diffs(reason) @ function M._cleanup_all_active_diffs(reason) site lua/claudecode/diff.lua: function M._cleanup_diff_state(tab_name, reason) @ function M._cleanup_diff_state(tab_name, reason) @@ -96,6 +99,7 @@ site lua/claudecode/diff.lua: local diff_data = active_diffs[tab_name] @ local f site lua/claudecode/diff.lua: local entry = active_diffs[tab_name] @ function M._register_reject_when_hidden(tab_name, new_buffer, autocmd_ids) site lua/claudecode/diff.lua: local existing_diff = active_diffs[tab_name] @ function M.open_diff_blocking(old_file_path, new_file_path, new_file_contents, tab_name, client_id) site lua/claudecode/diff.lua: local function close_active_diffs(filter_fn, reason, client_id) @ local function close_active_diffs(filter_fn, reason, client_id) +site lua/claudecode/diff.lua: local ok, err = pcall(M._resolve_diff_as_rejected, entry.tab_name) @ function M._reject_pending_diffs() site lua/claudecode/diff.lua: local state = active_diffs[tab_name] @ function M._setup_blocking_diff(params, resolution_callback) site lua/claudecode/diff.lua: resolution_callback = resolution_callback, @ function M._setup_blocking_diff(params, resolution_callback) site lua/claudecode/diff.lua: return active_diffs @ function M._get_active_diffs() diff --git a/.agents/skills/verify-diff-lifecycle/model/DiffLifecycle.tla b/.agents/skills/verify-diff-lifecycle/model/DiffLifecycle.tla index 68632c42d..b9f0a290c 100644 --- a/.agents/skills/verify-diff-lifecycle/model/DiffLifecycle.tla +++ b/.agents/skills/verify-diff-lifecycle/model/DiffLifecycle.tla @@ -33,6 +33,7 @@ CONSTANTS CloseTakesRejected, \* diff.lua close_active_diffs also closes an entry a handler rejected since the snapshot StopRejectsPending, \* server/init.lua M.stop closes pending diffs before clearing senders StopPendingOnly, \* diff.lua close_pending_diffs filters status == "pending" + ExitRejectsPending, \* diff.lua VimLeavePre autocmd rejects each pending diff before it cleans every diff up DisconnectRejects, \* server/init.lua on_disconnect schedules close_diffs_for_client DisconnectPendingOnly, \* diff.lua close_diffs_for_client filters status == "pending" UserCloseAllPendingOnly, \* init.lua :ClaudeCodeCloseAllDiffs calls close_pending_diffs @@ -339,6 +340,26 @@ ServerStops == SetW([v EXCEPT !.sender = [q \in Reqs |-> FALSE]]) /\ UNCHANGED <> +(* diff.lua _reject_pending_diffs: _resolve_diff_as_rejected for each diff that is still pending. *) +(* The order does not matter: each rejection touches only its own diff and its own call. *) +RECURSIVE RejectEach(_, _) +RejectEach(w, S) == + IF S = {} THEN w + ELSE LET t == CHOOSE x \in S : TRUE + IN RejectEach(IF w.active[t] # NoDiff /\ w.active[t].status = "pending" THEN Resolve(w, t, "rejected") ELSE w, + S \ {t}) + +(* diff.lua VimLeavePre autocmd: _reject_pending_diffs, then _cleanup_all_active_diffs("shutdown"), which *) +(* removes every entry in any status. Neovim fires BufUnload for each loaded buffer before VimLeavePre, and *) +(* the proposal buffers' autocmds reject their diffs there (the UserRejects action). That loop stops when an *) +(* autocmd deletes the buffer it fired for, so a pending diff can still reach this step. Neovim quits right *) +(* after, and its buffers go with it, so this removal is not a teardown that SavedDiffKept counts. *) +DiffCleanupAtExit == + /\ Idle /\ \E t \in Tabs : Open(t) + /\ LET rejected == IF ExitRejectsPending THEN RejectEach(W, Tabs) ELSE W + IN SetW([rejected EXCEPT !.active = [t \in Tabs |-> NoDiff]]) + /\ UNCHANGED <> + Next == \/ \E r \in Reqs, c \in Clients, t \in Tabs : ClaudeOpensDiff(r, c, t) \/ \E r \in Reqs, o \in Outcomes : UserHandlerResolves(r, o) @@ -352,6 +373,7 @@ Next == \/ \E c \in Clients : ClientIdReused(c) \/ \E c \in Clients : DisconnectCleanupRuns(c) \/ ServerStops + \/ DiffCleanupAtExit (* Setup is synchronous and the scheduled cleanup runs on a later tick. *) Spec == diff --git a/.agents/skills/verify-diff-lifecycle/model/statechart-view.ts b/.agents/skills/verify-diff-lifecycle/model/statechart-view.ts index 0bab1aaf1..4aed6a2f7 100644 --- a/.agents/skills/verify-diff-lifecycle/model/statechart-view.ts +++ b/.agents/skills/verify-diff-lifecycle/model/statechart-view.ts @@ -43,5 +43,6 @@ const labels: Record = { ClientIdReused: "a new Claude connection gets the old client id", DisconnectCleanupRuns: "the disconnect cleanup runs", ServerStops: "the server stops", + DiffCleanupAtExit: "Neovim exits (VimLeavePre)", }; export const step = (action: string) => labels[action] ?? action; diff --git a/lua/claudecode/README.md b/lua/claudecode/README.md index 91fd226b7..55de58bc1 100644 --- a/lua/claudecode/README.md +++ b/lua/claudecode/README.md @@ -143,6 +143,7 @@ Module map (direct files): - A write of the proposed buffer (`:w`, `:wq`, `:x`, `ZZ`, which run `BufWriteCmd`) never writes the file. It accepts a pending diff and does nothing else. `:w` leaves the windows open until `close_tab`, and `:wq`, `:x` and `ZZ` also close the proposal window. A write after the diff has its answer sends nothing and does not fail (the handler never returns `true`, which would delete the autocmd and make the next `:w` fail with E676). It warns when the buffer holds edits that Claude never gets [diff.lua:M._write_proposal] - A pending diff rejects at once when its proposed buffer is shown in no window, in the split and the unified layout: its last window closes (`:q`, `:close`, `c`, `:tabclose`), or that window switches to another buffer (`:edit`, `:buffer`, `:enew`, which only hide it). A copy of the proposal in another window or tab defers the reject until that copy leaves too. The switch path rejects on the next loop turn, and only when the diff is still the same one and still hidden. Both layouts keep the proposed buffer's `'bufhidden'` at `hide`: the buffer is modified, and `wipe` would make `:q` fail with E37 [diff.lua:M._register_reject_when_hidden] [diff_inline.lua:M.setup_inline_diff] - A diff that has its answer, by either one, clears `'modified'` on its proposed buffer. While the flag stayed set, `:wq` left the window open, and `:qa` stopped with E37 or E162 and moved a window onto the hidden buffer until `close_tab` deleted it [diff.lua:mark_proposal_unmodified] +- At exit, each diff that is still `pending` gets `DIFF_REJECTED` once. Neovim fires `BufUnload` for each loaded buffer before `VimLeavePre`, and a proposal's `BufUnload` autocmd rejects its diff, but that loop stops as soon as an autocmd deletes the buffer it fired for. The reject of a new-file diff with `on_new_file_reject = "keep_empty"` deletes its proposal buffer, so the diffs after it get no `BufUnload` (probed on Neovim 0.8.0, 0.9.5, 0.10.4, 0.11.5 and 0.12.3). The `VimLeavePre` autocmd therefore rejects every diff still `pending`, oldest first, before `_cleanup_all_active_diffs("shutdown")` removes any entry. A diff that already has its answer gets no second one [diff.lua:M._reject_pending_diffs] [diff.lua:cleanup_at_exit] - The cleanup of a diff closes the proposal window, and the window that the split layout opened for the file's old text, only while each still shows its buffer. A window that the user switched to another buffer stays with that buffer. `:edit` turns diff mode off by itself, except when the new buffer is in diff mode in another window, as the file's own buffer is. So the split layout's cleanup ends diff mode only in a kept window that shows the file's own buffer, and leaves a diff mode in a window that shows another buffer, which is the user's. One limit comes from Neovim: the default `closeoff` in `'diffopt'` turns diff mode off in the last window that has it when another diff window closes. That ends a user's diff mode in a kept original-side window when the proposal window closes, and in a kept proposal window when the cleanup closes the window that the plugin opened for the file. A user's diff mode over two or more windows stays. The unified layout needs no cleanup there: it sets `'signcolumn'` to `yes` with local scope, for the proposal buffer only, so the window shows the sign column it had before the diff once the user switches it, and keeps a value the user sets afterwards. Two cases still close the window: a diff in its own tab (`open_in_new_tab`, or a tab that already has a diff) closes the whole tab, and the fallback window that the plugin opens in a tab that holds only the Claude terminal always closes [diff.lua:M._close_proposal_window] [diff.lua:M._cleanup_diff_state] [diff_inline.lua:M.cleanup_inline_diff] [diff_inline.lua:M.setup_inline_diff] - `close_pending_diffs` and `close_diffs_for_client` close only `pending` diffs. `close_rejected_diffs` closes only `rejected` diffs. `close_all_diffs` closes every diff [diff.lua:M.close_pending_diffs] - A bulk close lists its diffs first, and each close fires `ClaudeCodeDiffClosed`. Before each close it reads the entry again: a diff a handler accepted since the list was made stays open, and one a handler rejected closes as listed. It closes the newest diff first, not in `pairs()` order, so when each close puts the cursor back, the cursor ends where it was before the oldest of them opened. `:ClaudeCodeCloseAllDiffs` runs three bulk closes, for pending, then rejected, then accepted diffs, so it closes newest first only within each status. With diffs of more than one status, the cursor can end in another window [diff.lua:close_active_diffs] @@ -159,7 +160,7 @@ Module map (direct files): - `terminal.setup` can replace the global `vim.paste` through `paste_fix.apply` [terminal.lua:M.setup] - The plugin broadcasts `at_mentioned` (`filePath`, `lineStart`, `lineEnd`) [init.lua:M._broadcast_at_mention] and `selection_changed` [selection.lua:M.send_selection_update] to every connected client - While Claude holds a visual selection, `selection.lua` attaches an `nvim_buf_attach` listener (`on_lines`, `on_reload`, `on_detach`) to its buffer, once per buffer. It detaches itself at the next `on_lines` after Claude holds no selection of that buffer, and a reload with `:edit` or `:edit!` detaches it, so it attaches again [selection.lua:watch_selection_buffer] -- Augroups: `ClaudeCodeShutdown`, which holds the `VimLeavePre` autocmd that stops on exit [init.lua:M.setup], `ClaudeCodeSelection` [selection.lua:M._create_autocommands], `ClaudeCodeMCPDiff` [diff.lua:get_autocmd_group], `ClaudeCodeDiffCleanup` [diff.lua:M._open_native_diff], and a module-level `VimLeavePre` autocmd in `diff.lua` that calls `M._cleanup_all_active_diffs("shutdown")` [diff.lua:M._cleanup_all_active_diffs] +- Augroups: `ClaudeCodeShutdown`, which holds the `VimLeavePre` autocmd that stops on exit [init.lua:M.setup], `ClaudeCodeSelection` [selection.lua:M._create_autocommands], `ClaudeCodeMCPDiff` [diff.lua:get_autocmd_group], `ClaudeCodeDiffCleanup` [diff.lua:M._open_native_diff], and a module-level `VimLeavePre` autocmd in `diff.lua` whose callback calls `M._reject_pending_diffs()` and then `M._cleanup_all_active_diffs("shutdown")` [diff.lua:cleanup_at_exit] - Diffs create scratch buffers, splits, and tabs. They resize the terminal window and fire `User` events. Cleanup closes these and reloads unchanged file buffers 100 ms after a save [diff.lua:M.close_diff_by_tab_name] - The native diff writes each temp file into its own session folder under `stdpath("cache")/claudecode_diffs`, or `claudecode_diffs_fallback` when it cannot create a directory in the first. The folder name includes the Neovim PID, so two running Neovim processes never share one. If both fail, `open_diff` returns `success = false` with an error that names both directories [diff.lua:create_temp_file]. A `BufDelete`/`BufWipeout` autocmd on the proposed buffer cleans up once, even when a wipe fires both events. It deletes the file and then its empty folder with `vim.fn.delete`, and warns once for each delete that fails. When the file delete fails, the folder is not empty, so its delete fails too and both warnings show [diff.lua:cleanup_temp_file]. `_cleanup_diff_state` does not [diff.lua:M._open_native_diff] - `diff_inline.lua` creates the `claudecode_inline_diff` namespace when the module loads. It defines the four `ClaudeCodeInlineDiff*` highlight groups (with `default = true`) [diff_inline.lua:setup_highlights] and sets extmarks [diff_inline.lua:M.render_diff_buffer] diff --git a/lua/claudecode/diff.lua b/lua/claudecode/diff.lua index 5875c295f..301f4229f 100644 --- a/lua/claudecode/diff.lua +++ b/lua/claudecode/diff.lua @@ -1993,12 +1993,39 @@ function M.open_diff_blocking(old_file_path, new_file_path, new_file_contents, t return user_action_result end +---Answer every diff that is still pending with DIFF_REJECTED, oldest first. A diff that already has its answer is +---skipped, so each openDiff gets one. One diff that fails to reject must not leave the others waiting. +function M._reject_pending_diffs() + local pending = {} + for tab_name, diff_data in pairs(active_diffs) do + if diff_data.status == "pending" then + pending[#pending + 1] = { tab_name = tab_name, open_order = diff_data.open_order } + end + end + table.sort(pending, function(a, b) + return a.open_order < b.open_order + end) + for _, entry in ipairs(pending) do + local ok, err = pcall(M._resolve_diff_as_rejected, entry.tab_name) + if not ok then + logger.warn("diff", "Could not reject the pending diff", entry.tab_name, "at exit:", tostring(err)) + end + end +end + +---Runs at VimLeavePre. 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, +---and the reject of a new-file diff with `on_new_file_reject = "keep_empty"` deletes its proposal buffer. The diffs +---behind that buffer then reach this point pending, so they are answered before the cleanup removes them. +local function cleanup_at_exit() + M._reject_pending_diffs() + M._cleanup_all_active_diffs("shutdown") +end + -- Set up global autocmds for shutdown handling vim.api.nvim_create_autocmd("VimLeavePre", { group = get_autocmd_group(), - callback = function() - M._cleanup_all_active_diffs("shutdown") - end, + callback = cleanup_at_exit, }) ---Close diff by tab name (used by close_tab tool) diff --git a/tests/integration/diff_exit_reject_spec.lua b/tests/integration/diff_exit_reject_spec.lua new file mode 100644 index 000000000..4afd56a8a --- /dev/null +++ b/tests/integration/diff_exit_reject_spec.lua @@ -0,0 +1,149 @@ +-- Real-Neovim check that `:qa!` answers every openDiff that is still pending, with DIFF_REJECTED, exactly once. +-- Neovim fires BufUnload for each loaded buffer at exit, and the proposal buffers' autocmds reject their diffs. That +-- loop stops once an autocmd deletes the buffer it fired for, and the reject of a new-file diff with the default +-- `on_new_file_reject = "keep_empty"` deletes its proposal buffer. The diffs behind it get no BufUnload, so the +-- VimLeavePre autocmd of diff.lua must answer them. A child Neovim does the exit, because it ends the process. +-- Mock vim cannot run an exit, so under busted this is pending. +if vim == nil or vim.fn.has("nvim") ~= 1 then + describe("openDiff at exit", function() + pending("needs a real Neovim") + end) + return +end + +describe("openDiff at exit", function() + local dir + + -- The child opens each step's diff in a coroutine, as the server does, and logs every answer its sender gets. + local child_script = [[ + local log_path = vim.env.EXIT_LOG + local function log(line) + local file = assert(io.open(log_path, "a")) + file:write(line, "\n") + file:close() + end + require("claudecode").setup({ + auto_start = false, + terminal = { provider = "none" }, + diff_opts = { layout = vim.env.EXIT_LAYOUT, on_new_file_reject = vim.env.EXIT_REJECT }, + }) + local diff = require("claudecode.diff") + _G.claude_deferred_responses = {} + for step in vim.env.EXIT_STEPS:gmatch("[^,]+") do + local name, kind = step:match("^(.-):(.*)$") + local path = vim.env.EXIT_DIR .. "/" .. name + if kind == "old" then + vim.fn.writefile({ "old line" }, path) + else + os.remove(path) + end + local co = coroutine.create(function() + return diff.open_diff_blocking(path, path, "new line\n", "tab_" .. name, nil) + end) + local ok, err = coroutine.resume(co) + log("resume " .. name .. " ok=" .. tostring(ok) .. " status=" .. coroutine.status(co) .. (ok and "" or " " .. tostring(err))) + _G.claude_deferred_responses[tostring(co)] = function(response) + log("answer " .. name .. " " .. tostring(response.content and response.content[1].text or vim.inspect(response))) + end + end + vim.defer_fn(function() + vim.cmd("qa!") + end, 100) + ]] + + ---Open the diffs of `steps` ("name:new" or "name:old", in order) in a child Neovim, quit it with :qa!, and return + ---the answers that its senders got and its exit code. + local function exit_with(steps, layout, reject) + local script = dir .. "/child.lua" + local log_path = dir .. "/answers.log" + vim.fn.writefile(vim.split(child_script, "\n"), script) + vim.fn.delete(log_path) + local job = vim.fn.jobstart({ + vim.v.progpath, + "--headless", + "-u", + "NONE", + "-i", + "NONE", + "--cmd", + "set rtp^=" .. vim.fn.getcwd(), + "-c", + "luafile " .. vim.fn.fnameescape(script), + }, { + env = { + EXIT_LOG = log_path, + EXIT_DIR = dir, + EXIT_LAYOUT = layout, + EXIT_REJECT = reject, + EXIT_STEPS = table.concat(steps, ","), + }, + }) + assert.is_true(job > 0, "could not start a child Neovim") + local code = vim.fn.jobwait({ job }, 20000)[1] + if code == -1 then + vim.fn.jobstop(job) + end + local lines = vim.fn.filereadable(log_path) == 1 and vim.fn.readfile(log_path) or {} + return lines, code + end + + local function answers_by_name(lines) + local answers = {} + for _, line in ipairs(lines) do + local name, text = line:match("^answer (%S+) (.*)$") + if name then + answers[name] = answers[name] or {} + table.insert(answers[name], text) + end + end + return answers + end + + before_each(function() + dir = vim.fn.tempname() + vim.fn.mkdir(dir, "p") + end) + + after_each(function() + vim.fn.delete(dir, "rf") + end) + + local scenarios = { + { name = "one existing file", steps = { "a.txt:old" } }, + { name = "a new file, then an existing file", steps = { "new1.txt:new", "b.txt:old" } }, + { name = "an existing file, then a new file", steps = { "a.txt:old", "new2.txt:new" } }, + { name = "two new files", steps = { "new1.txt:new", "new2.txt:new" } }, + { name = "a new file, an existing file, and a new file", steps = { "new1.txt:new", "b.txt:old", "new2.txt:new" } }, + } + local layouts = { "vertical", "unified" } + local rejects = { "keep_empty", "close_window" } + + for _, layout in ipairs(layouts) do + for _, reject in ipairs(rejects) do + for _, scenario in ipairs(scenarios) do + local title = ("answers each of %s with DIFF_REJECTED once (%s layout, on_new_file_reject = %s)"):format( + scenario.name, + layout, + reject + ) + -- The unified layout needs Neovim 0.9. The split layout bug is the same on 0.8. + local runs_here = layout ~= "unified" or vim.fn.has("nvim-0.9") == 1 + local define = runs_here and it or pending + define(title, function() + local lines, code = exit_with(scenario.steps, layout, reject) + + assert.equal(0, code, "the child exited with :qa!\n" .. table.concat(lines, "\n")) + local answers = answers_by_name(lines) + for _, step in ipairs(scenario.steps) do + local name = step:match("^(.-):") + assert.same( + { "DIFF_REJECTED" }, + answers[name], + ("openDiff of %s must get one DIFF_REJECTED. Log:\n%s"):format(name, table.concat(lines, "\n")) + ) + end + end) + end + end + end +end) diff --git a/tests/unit/diff_exit_reject_spec.lua b/tests/unit/diff_exit_reject_spec.lua new file mode 100644 index 000000000..b88ceb4fb --- /dev/null +++ b/tests/unit/diff_exit_reject_spec.lua @@ -0,0 +1,135 @@ +-- Neovim stops firing BufUnload at exit once an autocmd deletes the buffer it fired for (main.c, exit_loop in +-- getout), and a new-file reject with `on_new_file_reject = "keep_empty"` deletes its proposal buffer. So the +-- diffs behind that buffer get no BufUnload, reach VimLeavePre pending, and used to be removed unanswered. These +-- tests run the VimLeavePre autocmd of diff.lua over the states that exit can leave. The real exit, with real +-- buffers, is tests/integration/diff_exit_reject_spec.lua. +require("tests.busted_setup") + +describe("diffs still pending when Neovim exits", function() + local diff, dir, answers, senders + + ---The one VimLeavePre autocmd of diff.lua, run as Neovim runs it. + local function leave() + local group = _G.vim._autocmds["ClaudeCodeMCPDiff"] + assert(group, "the ClaudeCodeMCPDiff augroup was not created") + local callbacks = {} + for _, event in ipairs(group.events) do + if event.events == "VimLeavePre" then + callbacks[#callbacks + 1] = event.opts.callback + end + end + assert.equal(1, #callbacks, "expected one VimLeavePre autocmd in diff.lua") + callbacks[1]() + end + + ---Open a pending diff of a new file name. The sender records each answer, as the server's sender would. + local function open_pending(name) + local path = dir .. "/" .. name + local file = assert(io.open(path, "w")) + file:write("line 1\nline 2\n") + file:close() + local co = coroutine.create(function() + return diff.open_diff_blocking(path, path, "line 1\nline 2\nnew line\n", name, "client") + end) + local ok, err = coroutine.resume(co) + assert.is_true(ok, "the diff coroutine should start: " .. tostring(err)) + assert.equal("suspended", coroutine.status(co), name .. " should be pending") + _G.claude_deferred_responses[tostring(co)] = function(response) + answers[#answers + 1] = { name = name, text = response.content[1].text, tab = response.content[2].text } + end + senders[name] = tostring(co) + return co + end + + ---What Neovim's BufUnload autocmd of a proposal buffer does, and what the keep_empty reject then does. + local function buf_unload_with_keep_empty(name) + diff._resolve_diff_as_rejected(name) + diff._cleanup_diff_state(name, "diff rejected (keep_empty)") + end + + before_each(function() + package.loaded["claudecode.diff"] = nil + diff = require("claudecode.diff") + diff.setup({ diff_opts = {} }) + dir = require("tests.helpers.temp_dir").new() + answers, senders = {}, {} + _G.claude_deferred_responses = {} + end) + + after_each(function() + diff._cleanup_all_active_diffs("test cleanup") + require("tests.helpers.temp_dir").remove(dir) + _G.claude_deferred_responses = nil + end) + + it("answers a diff that never got its BufUnload with DIFF_REJECTED, and removes it", function() + open_pending("a.txt") + + leave() + + assert.same({ { name = "a.txt", text = "DIFF_REJECTED", tab = "a.txt" } }, answers) + assert.is_nil(next(diff._get_active_diffs())) + end) + + it("answers every pending diff when an earlier diff's reject deleted its buffer and stopped the exit loop", function() + open_pending("new1.txt") + open_pending("b.txt") + buf_unload_with_keep_empty("new1.txt") -- the only BufUnload that ran + + leave() + + local names = {} + for _, answer in ipairs(answers) do + assert.equal("DIFF_REJECTED", answer.text) + names[#names + 1] = answer.name + end + assert.same({ "new1.txt", "b.txt" }, names, "each diff gets one answer") + assert.is_nil(next(diff._get_active_diffs())) + end) + + it("answers the oldest pending diff first", function() + open_pending("first.txt") + open_pending("second.txt") + open_pending("third.txt") + + leave() + + local names = {} + for _, answer in ipairs(answers) do + names[#names + 1] = answer.name + end + assert.same({ "first.txt", "second.txt", "third.txt" }, names) + end) + + it("sends no second answer for a diff that its BufUnload already rejected", function() + open_pending("a.txt") + diff._resolve_diff_as_rejected("a.txt") -- the BufUnload autocmd, without a keep_empty cleanup + + leave() + + assert.equal(1, #answers, "one answer for one openDiff") + assert.is_nil(next(diff._get_active_diffs())) + end) + + it("sends no second answer to a diff that the user accepted with :w", function() + open_pending("a.txt") + diff._resolve_diff_as_saved("a.txt", diff._get_active_diffs()["a.txt"].new_buffer) + + leave() + + assert.equal(1, #answers, "one answer for one openDiff") + assert.equal("FILE_SAVED", answers[1].text) + end) + + it("still answers the other pending diffs when rejecting one raises", function() + open_pending("a.txt") + open_pending("b.txt") + diff._get_active_diffs()["a.txt"].resolution_callback = function() + error("boom") + end + + assert.has_no.errors(leave) + + assert.same({ { name = "b.txt", text = "DIFF_REJECTED", tab = "b.txt" } }, answers) + end) +end) From e94d6b172e39e8230fef34ddceb7f3a00790a887 Mon Sep 17 00:00:00 2001 From: Thomas Kosiewski Date: Thu, 12 Nov 2026 02:04:13 +0000 Subject: [PATCH 02/11] test(diff): drop an unused variable from the exit spec luacheck reports senders as mutated but never accessed. --- tests/unit/diff_exit_reject_spec.lua | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/tests/unit/diff_exit_reject_spec.lua b/tests/unit/diff_exit_reject_spec.lua index b88ceb4fb..4079a794e 100644 --- a/tests/unit/diff_exit_reject_spec.lua +++ b/tests/unit/diff_exit_reject_spec.lua @@ -6,7 +6,7 @@ require("tests.busted_setup") describe("diffs still pending when Neovim exits", function() - local diff, dir, answers, senders + local diff, dir, answers ---The one VimLeavePre autocmd of diff.lua, run as Neovim runs it. local function leave() @@ -37,7 +37,6 @@ describe("diffs still pending when Neovim exits", function() _G.claude_deferred_responses[tostring(co)] = function(response) answers[#answers + 1] = { name = name, text = response.content[1].text, tab = response.content[2].text } end - senders[name] = tostring(co) return co end @@ -52,7 +51,7 @@ describe("diffs still pending when Neovim exits", function() diff = require("claudecode.diff") diff.setup({ diff_opts = {} }) dir = require("tests.helpers.temp_dir").new() - answers, senders = {}, {} + answers = {} _G.claude_deferred_responses = {} end) From 1b89edc6effce441d7d4bcbfe58c0130c558f6c6 Mon Sep 17 00:00:00 2001 From: Thomas Kosiewski Date: Thu, 12 Nov 2026 03:35:15 +0000 Subject: [PATCH 03/11] fix(server): answer a call that an exit cut off during its setup A nested ClaudeCodeDiffOpened handler that runs :qa! during the setup of an openDiff exits Neovim with that call pending and no response sender yet, because the server stores the sender only after the tool handler yielded. The exit never unwinds the stack, so the early answer of the setup never left. Claude got the answers of the earlier calls, then Close 1001. _handle_request now records each request in M.state.in_flight while its handler runs. M.stop answers every request still recorded with -32603 before it stops the TCP server, and _handle_request sends nothing for a request that stop answered. The reject pass at exit then needs no sender of its own: a call in setup gets the error, a waiting call gets DIFF_REJECTED. The model gets the action ExitInSetup, the fact ExitAnswersInFlight and the rule ExitedCallsAnswered, so Idle no longer hides this case. With the fact FALSE, ExitedCallsAnswered breaks, and check.sh --mutants catches every guard. covered.lock records the new hashes. Verifier findings of the first round: - Order assertion: the unit spec opens c.txt, a.txt and b.txt, so a sort by tab name no longer passes. - SIGTERM, a buffer that deletes itself in its own BufUnload, and the unified layout in the exit pass each get an integration scenario. - Docs: the keep_empty reject deletes its proposal buffer only in a tab the diff did not open. SIGTERM and SIGHUP run VimLeavePre without the BufUnload loop, and SIGKILL or a crash runs no VimLeavePre. - SKILL.md: exit is modeled, and the two exit actions are separate actions. - The README sections the docs refresh wrote now state the fixed behavior. --- .agents/skills/verify-diff-lifecycle/SKILL.md | 9 +- .agents/skills/verify-diff-lifecycle/check.sh | 14 +- .../skills/verify-diff-lifecycle/covered.lock | 8 +- .../model/DiffLifecycle.tla | 29 ++- .../model/statechart-view.ts | 4 + README.md | 18 +- lua/claudecode/README.md | 2 +- lua/claudecode/diff.lua | 8 +- lua/claudecode/server/README.md | 5 +- lua/claudecode/server/init.lua | 21 +++ tests/integration/diff_exit_reject_spec.lua | 171 ++++++++++++++++-- tests/unit/diff_exit_during_setup_spec.lua | 131 ++++++++++++++ tests/unit/diff_exit_reject_spec.lua | 17 +- tests/unit/server/in_flight_request_spec.lua | 105 +++++++++++ 14 files changed, 492 insertions(+), 50 deletions(-) create mode 100644 tests/unit/diff_exit_during_setup_spec.lua create mode 100644 tests/unit/server/in_flight_request_spec.lua diff --git a/.agents/skills/verify-diff-lifecycle/SKILL.md b/.agents/skills/verify-diff-lifecycle/SKILL.md index 6da90183c..09fc50834 100644 --- a/.agents/skills/verify-diff-lifecycle/SKILL.md +++ b/.agents/skills/verify-diff-lifecycle/SKILL.md @@ -17,6 +17,7 @@ Claude calls `openDiff` and blocks until the plugin answers. If the plugin never | `SavedDiffKept` | Only Claude tears down a diff the user accepted with `:w`, and never after its own client is gone. The user can too, once the file or its buffer holds the accepted text. | The user's accepted edits vanish before Claude writes them. | | `DisconnectedCallsAnswered` | A disconnected client's waiting calls get answered without the user. | Orphaned diff windows linger (issue #248). | | `NoRejectedLeftover` | A bulk close also closes a diff from its list that a `ClaudeCodeDiffClosed` handler denied while the loop ran. | The denied diff's windows linger after `:ClaudeCodeCloseAllDiffs`, a disconnect, or a server stop. | +| `ExitedCallsAnswered` | A call that an exit of Neovim cut off during its setup has been answered, by the server stop. | Claude waits on a call whose connection closes with no answer. | ## Run @@ -77,6 +78,7 @@ A changed file with no changed block or call site still needs a look. A wrapper | `CloseEachByName` | `close_active_diffs` closes each match through `close_diff_by_tab_name` | | `StopRejectsPending` | `M.stop` calls `close_pending_diffs`, then `stop_server`, then clears the senders | | `ExitRejectsPending` | the `VimLeavePre` callback `cleanup_at_exit` calls `_reject_pending_diffs` before `_cleanup_all_active_diffs`, so a pending diff is answered, not removed | +| `ExitAnswersInFlight` | `M.stop` answers each request in `M.state.in_flight`, and `_handle_request` sends nothing after a stop that answered it, so a call cut off in setup is answered | | `StopPendingOnly`, `DisconnectPendingOnly` | the `status == "pending"` filters in `close_pending_diffs` and `close_diffs_for_client` | | `DisconnectRejects` | `on_disconnect` schedules `close_diffs_for_client` | | `UserCloseAllPendingOnly` | `:ClaudeCodeCloseAllDiffs` calls `close_pending_diffs`, never `close_all_diffs` | @@ -120,9 +122,10 @@ Then decide which kind of failure it is: ## Scope and bounds -- The invariants run with 3 openDiff calls, 2 tab names and 2 clients, about 89,000 states. `DisconnectedCallsAnswered` runs with 2 calls, 2 tabs and 2 clients. It assumes weak fairness only on setup and on the scheduled disconnect cleanup. The user is never assumed to act, with one exception: the disconnect cleanup waits until Neovim allows window changes (`utils.when_windows_allowed`), so a user in the command-line window is assumed to leave it, and textlock is assumed to end. +- The invariants run with 3 openDiff calls, 2 tab names and 2 clients, about 93,000 states. `DisconnectedCallsAnswered` runs with 2 calls, 2 tabs and 2 clients. It assumes weak fairness only on setup and on the scheduled disconnect cleanup. The user is never assumed to act, with one exception: the disconnect cleanup waits until Neovim allows window changes (`utils.when_windows_allowed`), so a user in the command-line window is assumed to leave it, and textlock is assumed to end. - In every run, a user `ClaudeCodeDiffOpened` handler (README "Diff Lifecycle Events") can accept or deny the diff during setup: `:ClaudeCodeDiffAccept`, `:ClaudeCodeDiffDeny`, or a `nested = true` autocmd that closes the proposal window. A default handler that closes the window resolves nothing, because WinClosed does not fire inside a non-nested autocmd. Other handler actions during setup are not modeled. -- `DiffCleanupAtExit` is the `VimLeavePre` callback of `diff.lua`: `_reject_pending_diffs`, then `_cleanup_all_active_diffs("shutdown")`. It models neither a `ClaudeCodeDiffClosed` handler (the reject pass has none unless a `keep_empty` reject runs a cleanup) nor a saved diff lost at exit: Neovim quits right after the step, and its buffers go with it. It can run in the same step as `ServerStops` in either order, because both are `VimLeavePre` autocmds. Each is one action here, and the model runs them in any order. Neovim's own exit `BufUnload` loop, which rejects before `VimLeavePre`, is the `UserRejects` action. +- `DiffCleanupAtExit` is the `VimLeavePre` callback of `diff.lua`: `_reject_pending_diffs`, then `_cleanup_all_active_diffs("shutdown")`. It models neither a `ClaudeCodeDiffClosed` handler (the reject pass has none unless a `keep_empty` reject runs a cleanup) nor a saved diff lost at exit: Neovim quits right after the step, and its buffers go with it. The model represents `DiffCleanupAtExit` and `ServerStops` as separate atomic actions and explores both orders, because both are `VimLeavePre` autocmds. Neovim's own exit `BufUnload` loop, which rejects before `VimLeavePre`, is the `UserRejects` action. +- `ExitInSetup` is an exit inside a `nested = true` `ClaudeCodeDiffOpened` handler, while one call is in setup, so it needs `OpenedEventInSetup`. The call has no sender and its setup never returns, so only the server stop answers it, with an error (`ExitAnswersInFlight`). The action runs both `VimLeavePre` autocmds in one order, diff cleanup first, then the server stop. The other order gives the same result, because each leaves the other nothing to do. A handler that runs `:ClaudeCodeStop` instead of an exit reaches the same `M.stop` code and is not a separate action. - `SenderCleared` alone is caught by no rule, because the pending guards also stop a second answer. `--mutants` shows that `AtMostOneAnswer` breaks when both `SenderCleared` and `GuardRejected` are removed. - `SavedDiffKept` knows which client owns each diff, and, while `:ClaudeCodeCloseAllDiffs` runs, which files or file buffers hold the accepted text. The model has no file or buffer contents: the command's first pass leaves the accepted text in any subset of the saved diffs' files or buffers, because the code skips a diff whose file buffer already has unsaved changes, or whose buffer it cannot change. The plugin never writes the file. That the first pass leaves the file on disk and the user's unsaved changes alone is checked by the unit tests, not here. A saved diff counts as lost when the user, a disconnect cleanup or a server stop tears it down, or when a client that does not own it does so while the owner is disconnected. A close by another client while the owner is connected counts as safe, because the owner got `FILE_SAVED` and can still write the file. The code is stricter than the rule: it keeps another client's saved diff in both cases. Ownership compares client ids, so a new connection must never get an old id. With `ClientIdsUnique` off, `ClientIdReused` lets a new connection take the id of a disconnected client. - The split and unified layouts share the pending guards only because `resolve_inline_as_*` is reached through `_resolve_diff_as_*`. The inline functions have no guard of their own. @@ -136,7 +139,7 @@ Then decide which kind of failure it is: - a cleanup that raises partway. Neovim refuses window changes in the command-line window (E11) and under textlock (E565), and a close that raises there leaves its diff registered. The disconnect cleanup starts only when Neovim allows window changes. The other closes (`close_tab`, a replacing openDiff, the server stop) do not wait, and in those states they still raise - the `keep_empty` new-file cleanup inside `_resolve_diff_as_rejected` (it runs after the answer) - setup errors, which answer at once - - Neovim exit (`VimLeavePre`) and the plugin's own `M.stop` in `init.lua`, beyond the server stop it calls + - the plugin's own `M.stop` in `init.lua`, beyond the server stop it calls - whether a sent response reaches a live socket - two servers in one Neovim - A pass covers this model at these bounds, for the facts `check.sh` reads, on `lua/` and `plugin/` exactly as recorded in `covered.lock`. Any other tree reads STALE. diff --git a/.agents/skills/verify-diff-lifecycle/check.sh b/.agents/skills/verify-diff-lifecycle/check.sh index 059af0d33..b809368b0 100755 --- a/.agents/skills/verify-diff-lifecycle/check.sh +++ b/.agents/skills/verify-diff-lifecycle/check.sh @@ -150,6 +150,16 @@ exit_rejects_pending() { in_order '^vim\.api\.nvim_create_autocmd\("VimLeavePre", \{$' '^ callback = cleanup_at_exit,$' <"$DIFF" } fact[ExitRejectsPending]=$(as_bool exit_rejects_pending) +# A request has no answer until its handler returns, and a blocking tool has no sender before that. When Neovim exits +# inside the handler, M.stop answers the request, and _handle_request sends nothing after a stop that answered it. +FACTS+=(ExitAnswersInFlight) +exit_answers_in_flight() { + in_order '^ M\.state\.in_flight\[in_flight\] = true$' 'pcall\(handler, client, params\)' \ + '^ if not M\.state\.in_flight\[in_flight\] then$' '^ return$' <<<"$handle_request" && + in_order '^ for in_flight in pairs\(M\.state\.in_flight\) do$' 'M\.send_response\(in_flight\.client, in_flight\.id' \ + 'tcp_server\.stop_server\(' <<<"$server_stop" +} +fact[ExitAnswersInFlight]=$(as_bool exit_answers_in_flight) fact[DisconnectRejects]=$(as_bool in_order 'vim\.schedule\(function\(\)' 'diff\.close_diffs_for_client\(client_id' \ <<<"$on_disconnect") fact[DisconnectPendingOnly]=$(as_bool grep -qE \ @@ -212,7 +222,7 @@ fact[OpenedEventInSetup]=$(as_bool opened_event_in_setup) mapfile -t COVERED < <(find lua plugin \( -name '*.lua' -o -name '*.vim' \) | sort) SITE_RE='(_resolve_diff_as_|resolve_inline_as_|_cleanup_diff_state|_cleanup_all_active_diffs|close_[a-z_]*diffs|close_diff_by_tab_name|resolution_callback|claude_deferred_responses|_register_diff_state|active_diffs|_get_active_diffs|\.status *= *[^=])' -RULES=(AtMostOneAnswer AnswerMatchesDiff NoStrandedCall SavedDiffKept NoRejectedLeftover) +RULES=(AtMostOneAnswer AnswerMatchesDiff NoStrandedCall SavedDiffKept NoRejectedLeftover ExitedCallsAnswered) LIVE=(DisconnectedCallsAnswered) CONSTRAINT=AnswerBound @@ -221,7 +231,7 @@ CONSTRAINT=AnswerBound # it. SetupAnswerReturned=FALSE is caught only through that step, so its row also shows that the step is reachable. EXPECT=( [GuardSaved]=AnswerMatchesDiff [GuardRejected]=AnswerMatchesDiff - [ReplaceRejects]=NoStrandedCall [CloseTabRejectsPending]=NoStrandedCall [CloseEachByName]=NoStrandedCall [StopRejectsPending]=NoStrandedCall [ExitRejectsPending]=NoStrandedCall + [ReplaceRejects]=NoStrandedCall [CloseTabRejectsPending]=NoStrandedCall [CloseEachByName]=NoStrandedCall [StopRejectsPending]=NoStrandedCall [ExitRejectsPending]=NoStrandedCall [ExitAnswersInFlight]=ExitedCallsAnswered [SetupAnswerReturned]=NoStrandedCall [StopPendingOnly]=SavedDiffKept [DisconnectPendingOnly]=SavedDiffKept [UserCloseAllPendingOnly]=SavedDiffKept [UserCloseKeepsAcceptedText]=SavedDiffKept diff --git a/.agents/skills/verify-diff-lifecycle/covered.lock b/.agents/skills/verify-diff-lifecycle/covered.lock index d86c6f3f6..6d51f1069 100644 --- a/.agents/skills/verify-diff-lifecycle/covered.lock +++ b/.agents/skills/verify-diff-lifecycle/covered.lock @@ -3,7 +3,7 @@ file lua/claudecode/config.lua 787fa0432d1d77d7 == lua/claudecode/cwd.lua file lua/claudecode/cwd.lua 55d26ea362173afa == lua/claudecode/diff.lua -file lua/claudecode/diff.lua f6eb2eb21448cb11 +file lua/claudecode/diff.lua 8a209b2c063625a1 block resolve_saved 46d5f3f732711fe1 block resolve_rejected c7a620816abff9a1 block open_blocking 92067df7f9d4d4d9 @@ -150,10 +150,10 @@ file lua/claudecode/server/frame.lua 3c50766875878d56 == lua/claudecode/server/handshake.lua file lua/claudecode/server/handshake.lua 3eca35d572094a55 == lua/claudecode/server/init.lua -file lua/claudecode/server/init.lua a9c0d153b61e3078 -block server_stop 4dae0bf675cf2cc1 +file lua/claudecode/server/init.lua ac973c02b06e4d54 +block server_stop 506e2e53358dc947 block on_disconnect a0ce86cede4af786 -block handle_request b74ff7dc2e55a2b8 +block handle_request dcc066af67f9f88e block setup_deferred f9381caaae0fe517 site lua/claudecode/server/init.lua: _G.claude_deferred_responses = {} @ function M._setup_deferred_response(deferred_info) site lua/claudecode/server/init.lua: _G.claude_deferred_responses = {} @ function M.stop() diff --git a/.agents/skills/verify-diff-lifecycle/model/DiffLifecycle.tla b/.agents/skills/verify-diff-lifecycle/model/DiffLifecycle.tla index b9f0a290c..96fee8bdd 100644 --- a/.agents/skills/verify-diff-lifecycle/model/DiffLifecycle.tla +++ b/.agents/skills/verify-diff-lifecycle/model/DiffLifecycle.tla @@ -34,6 +34,8 @@ CONSTANTS StopRejectsPending, \* server/init.lua M.stop closes pending diffs before clearing senders StopPendingOnly, \* diff.lua close_pending_diffs filters status == "pending" ExitRejectsPending, \* diff.lua VimLeavePre autocmd rejects each pending diff before it cleans every diff up + ExitAnswersInFlight, \* server/init.lua M.stop answers a request whose handler is still running, and + \* _handle_request sends nothing after that DisconnectRejects, \* server/init.lua on_disconnect schedules close_diffs_for_client DisconnectPendingOnly, \* diff.lua close_diffs_for_client filters status == "pending" UserCloseAllPendingOnly, \* init.lua :ClaudeCodeCloseAllDiffs calls close_pending_diffs @@ -57,7 +59,8 @@ Answer(status) == [] OTHER -> None VARIABLES - phase, \* per call: "unsent", "setup" (coroutine running), "waiting" (yielded), "done" + phase, \* per call: "unsent", "setup" (coroutine running), "waiting" (yielded), "done", "gone" (Neovim\ + \* exited during its setup, and nothing answered it) reqTab, \* per call: its tab_name reqClient, \* per call: the client that sent it active, \* diff.lua active_diffs: per tab_name, the call it answers and its status @@ -192,7 +195,7 @@ Idle == \A r \in Reqs : phase[r] # "setup" Open(t) == active[t] # NoDiff TypeOK == - /\ phase \in [Reqs -> {"unsent", "setup", "waiting", "done"}] + /\ phase \in [Reqs -> {"unsent", "setup", "waiting", "done", "gone"}] /\ reqTab \in [Reqs -> Tabs \cup {None}] /\ reqClient \in [Reqs -> Clients \cup {None}] /\ active \in [Tabs -> {NoDiff} \cup [req : Reqs, status : {"pending"} \cup Outcomes]] @@ -360,6 +363,23 @@ DiffCleanupAtExit == IN SetW([rejected EXCEPT !.active = [t \in Tabs |-> NoDiff]]) /\ UNCHANGED <> +(* Neovim exits inside a `nested = true` ClaudeCodeDiffOpened handler (it runs :qa!), during the setup of call r. *) +(* Both VimLeavePre autocmds run on top of the setup: the diff cleanup, then the server stop. Call r has no *) +(* sender yet, and its setup never returns its early answer, because the process ends. Only the server stop can *) +(* answer it, with an error. The model runs the two autocmds in this one order: the other order gives the same *) +(* result, because the stop's close_pending_diffs and the diff cleanup both find only what the other left. *) +ExitInSetup(r) == + /\ OpenedEventInSetup /\ phase[r] = "setup" /\ serverUp + /\ LET rejected == IF ExitRejectsPending THEN RejectEach(W, Tabs) ELSE W + stopped == [rejected EXCEPT !.active = [t \in Tabs |-> NoDiff], !.sender = [q \in Reqs |-> FALSE]] + answered == IF ExitAnswersInFlight + THEN [stopped EXCEPT !.phase[r] = "done", !.answers[r] = Append(@, "ERROR")] + ELSE [stopped EXCEPT !.phase[r] = "gone"] + IN SetW(answered) + /\ serverUp' = FALSE + /\ clientUp' = [c \in Clients |-> FALSE] + /\ UNCHANGED <> + Next == \/ \E r \in Reqs, c \in Clients, t \in Tabs : ClaudeOpensDiff(r, c, t) \/ \E r \in Reqs, o \in Outcomes : UserHandlerResolves(r, o) @@ -374,6 +394,7 @@ Next == \/ \E c \in Clients : DisconnectCleanupRuns(c) \/ ServerStops \/ DiffCleanupAtExit + \/ \E r \in Reqs : ExitInSetup(r) (* Setup is synchronous and the scheduled cleanup runs on a later tick. *) Spec == @@ -396,6 +417,10 @@ AnswerMatchesDiff == \A t \in Tabs : Open(t) /\ answers[active[t].req] # <<>> => answers[active[t].req][1] = Answer(active[t].status) +(* A call that Neovim's exit cut off during its setup has been answered, by *) +(* the server stop: no later step of its own can send anything. *) +ExitedCallsAnswered == \A r \in Reqs : phase[r] = "gone" => answers[r] # <<>> + (* A call that waits for the user still has a pending diff and a response *) (* sender, so a user action or a teardown can answer it. *) NoStrandedCall == diff --git a/.agents/skills/verify-diff-lifecycle/model/statechart-view.ts b/.agents/skills/verify-diff-lifecycle/model/statechart-view.ts index 4aed6a2f7..959047659 100644 --- a/.agents/skills/verify-diff-lifecycle/model/statechart-view.ts +++ b/.agents/skills/verify-diff-lifecycle/model/statechart-view.ts @@ -4,6 +4,8 @@ const first = (seq: any) => (seq && seq[1] !== undefined ? seq[1] : null); const call = (v: any, r: string) => { const phase = v.phase[r]; if (phase === "unsent") return "not called"; + if (phase === "gone") + return "cut off by Neovim's exit during setup, nothing can answer"; const diff = v.active[v.reqTab[r]]; const shown = diff.req !== r ? "its diff closed" : `diff ${diff.status}`; if (phase === "setup") return `opening, ${shown}`; @@ -44,5 +46,7 @@ const labels: Record = { DisconnectCleanupRuns: "the disconnect cleanup runs", ServerStops: "the server stops", DiffCleanupAtExit: "Neovim exits (VimLeavePre)", + ExitInSetup: + "Neovim exits inside a ClaudeCodeDiffOpened handler, during setup", }; export const step = (action: string) => labels[action] ?? action; diff --git a/README.md b/README.md index e1fa14220..77614dc5e 100644 --- a/README.md +++ b/README.md @@ -1140,14 +1140,16 @@ The plugin runs the CLI through a [terminal provider](./lua/claudecode/terminal/ pending diff [lua/claudecode/diff.lua:M.close_diff_by_tab_name]. A diff that resolves before `openDiff` yields, for example from a `ClaudeCodeDiffOpened` handler, is answered at once [lua/claudecode/diff.lua:M.open_diff_blocking]. When Neovim exits, it fires `BufUnload` for each loaded buffer before `VimLeavePre`. The `BufUnload` autocmd of a proposal buffer rejects - its pending diff [lua/claudecode/diff.lua:register_diff_autocmds], so with one pending diff the `VimLeavePre` cleanup - of `ClaudeCodeMCPDiff` finds none [lua/claudecode/diff.lua:M.\_cleanup_all_active_diffs]. With two or more pending - native diffs, one case breaks this. The reject of a new-file diff in the current tab, with - `diff_opts.on_new_file_reject = "keep_empty"` (the default), deletes its proposal buffer - [lua/claudecode/diff.lua:M.\_resolve_diff_as_rejected] [lua/claudecode/diff.lua:M.\_cleanup_diff_state]. When that - reject runs in the exit loop, Neovim stops firing `BufUnload` for the later buffers, and the `VimLeavePre` cleanup - removes the diffs that are still pending with no answer to Claude. This holds on Neovim 0.8.0 to 0.12.3. The - `"unified"` layout is not affected. + its pending diff [lua/claudecode/diff.lua:register_diff_autocmds]. That loop stops as soon as an autocmd deletes the + buffer it fired for. The reject of a new-file diff in a tab it did not open, with `diff_opts.on_new_file_reject = +"keep_empty"` (the default), deletes its proposal buffer [lua/claudecode/diff.lua:M.\_resolve_diff_as_rejected] + [lua/claudecode/diff.lua:M.\_cleanup_diff_state], so the later diffs get no `BufUnload`. SIGTERM and SIGHUP run + `VimLeavePre` without that loop, and SIGKILL or a crash runs no `VimLeavePre`. The `VimLeavePre` autocmd of + `ClaudeCodeMCPDiff` therefore rejects every diff that is still pending, oldest first, before it removes any + [lua/claudecode/diff.lua:cleanup_at_exit], so each pending diff gets one `DIFF_REJECTED` in every exit that runs + `VimLeavePre`. A call whose setup has not finished when Neovim exits has no response sender yet. This happens when a + `nested = true` `ClaudeCodeDiffOpened` handler runs `:qa!`. The server stop answers that call with a -32603 error + [lua/claudecode/server/init.lua:M.stop]. ### Global Side Effects diff --git a/lua/claudecode/README.md b/lua/claudecode/README.md index f15c11b7a..ca457b112 100644 --- a/lua/claudecode/README.md +++ b/lua/claudecode/README.md @@ -181,7 +181,7 @@ Module map (direct files): - A write of the proposed buffer (`:w`, `:wq`, `:x`, `ZZ`, which run `BufWriteCmd`) never writes the file. It accepts a pending diff and does nothing else. `:w` leaves the windows open until `close_tab`, and `:wq`, `:x` and `ZZ` also close the proposal window. A write after the diff has its answer sends nothing and does not fail (the handler never returns `true`, which would delete the autocmd and make the next `:w` fail with E676). It warns when the buffer holds edits that Claude never gets [diff.lua:M._write_proposal] - A pending diff rejects at once when its proposed buffer is shown in no window, in the split and the unified layout: its last window closes (`:q`, `:close`, `c`, `:tabclose`), or that window switches to another buffer (`:edit`, `:buffer`, `:enew`, which only hide it). A copy of the proposal in another window or tab defers the reject until that copy leaves too. The switch path rejects on the next loop turn, and only when the diff is still the same one and still hidden. Both layouts keep the proposed buffer's `'bufhidden'` at `hide`: the buffer is modified, and `wipe` would make `:q` fail with E37 [diff.lua:M._register_reject_when_hidden] [diff_inline.lua:M.setup_inline_diff] - A diff that has its answer, by either one, clears `'modified'` on its proposed buffer. While the flag stayed set, `:wq` left the window open, and `:qa` stopped with E37 or E162 and moved a window onto the hidden buffer until `close_tab` deleted it [diff.lua:mark_proposal_unmodified] -- At exit, each diff that is still `pending` gets `DIFF_REJECTED` once. Neovim fires `BufUnload` for each loaded buffer before `VimLeavePre`, and a proposal's `BufUnload` autocmd rejects its diff, but that loop stops as soon as an autocmd deletes the buffer it fired for. The reject of a new-file diff with `on_new_file_reject = "keep_empty"` deletes its proposal buffer, so the diffs after it get no `BufUnload` (probed on Neovim 0.8.0, 0.9.5, 0.10.4, 0.11.5 and 0.12.3). The `VimLeavePre` autocmd therefore rejects every diff still `pending`, oldest first, before `_cleanup_all_active_diffs("shutdown")` removes any entry. A diff that already has its answer gets no second one [diff.lua:M._reject_pending_diffs] [diff.lua:cleanup_at_exit] +- At exit, each diff that is still `pending` gets `DIFF_REJECTED` once. Neovim fires `BufUnload` for each loaded buffer before `VimLeavePre`, and a proposal's `BufUnload` autocmd rejects its diff, but that loop stops as soon as an autocmd deletes the buffer it fired for. The reject of a new-file diff with `on_new_file_reject = "keep_empty"`, in a tab that the diff did not open, deletes its proposal buffer, and another plugin's buffer can delete itself in its own `BufUnload`. The diffs after either get no `BufUnload` (probed on Neovim 0.8.0, 0.9.5, 0.10.4, 0.11.5 and 0.12.3). SIGTERM and SIGHUP run `VimLeavePre` without that loop, and SIGKILL or a crash runs no `VimLeavePre`. The `VimLeavePre` autocmd therefore rejects every diff still `pending`, oldest first, before `_cleanup_all_active_diffs("shutdown")` removes any entry, so each pending diff gets its answer in every exit path that runs `VimLeavePre`. A diff that already has its answer gets no second one. A call whose setup has not finished has no response sender yet. When a `nested = true` `ClaudeCodeDiffOpened` handler runs `:qa!` during that setup, `server.stop` answers the call with -32603 [diff.lua:M._reject_pending_diffs] [diff.lua:cleanup_at_exit] [server/init.lua:M.stop] - The cleanup of a diff closes the proposal window, and the window that the split layout opened for the file's old text, only while each still shows its buffer. A window that the user switched to another buffer stays with that buffer. `:edit` turns diff mode off by itself, except when the new buffer is in diff mode in another window, as the file's own buffer is. So the split layout's cleanup ends diff mode only in a kept window that shows the file's own buffer, and leaves a diff mode in a window that shows another buffer, which is the user's. One limit comes from Neovim: the default `closeoff` in `'diffopt'` turns diff mode off in the last window that has it when another diff window closes. That ends a user's diff mode in a kept original-side window when the proposal window closes, and in a kept proposal window when the cleanup closes the window that the plugin opened for the file. A user's diff mode over two or more windows stays. The unified layout needs no cleanup there: it sets `'signcolumn'` to `yes` with local scope, for the proposal buffer only, so the window shows the sign column it had before the diff once the user switches it, and keeps a value the user sets afterwards. Two cases still close the window: a diff in its own tab (`open_in_new_tab`, or a tab that already has a diff) closes the whole tab, and the fallback window that the plugin opens in a tab that holds only the Claude terminal always closes [diff.lua:M._close_proposal_window] [diff.lua:M._cleanup_diff_state] [diff_inline.lua:M.cleanup_inline_diff] [diff_inline.lua:M.setup_inline_diff] - Rejecting a new-file diff that is not in its own tab cleans it up at once when `diff_opts.on_new_file_reject` is `"keep_empty"` (the default), before `close_tab`. A `unified` diff gets no such cleanup [diff.lua:M._resolve_diff_as_rejected] [diff_inline.lua:M.resolve_inline_as_rejected] - `BufDelete`, `BufUnload`, and `BufWipeout` on the proposed buffer reject a pending diff. `WinClosed` rejects it only when no other window in any tab shows the proposed buffer. The original file's buffer is not watched. The unified layout registers the same autocmds [diff.lua:register_diff_autocmds] [diff_inline.lua:M.setup_inline_diff] diff --git a/lua/claudecode/diff.lua b/lua/claudecode/diff.lua index 301f4229f..ed5d24721 100644 --- a/lua/claudecode/diff.lua +++ b/lua/claudecode/diff.lua @@ -2014,9 +2014,11 @@ function M._reject_pending_diffs() end ---Runs at VimLeavePre. 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, ----and the reject of a new-file diff with `on_new_file_reject = "keep_empty"` deletes its proposal buffer. The diffs ----behind that buffer then reach this point pending, so they are answered before the cleanup removes them. +---buffer's BufUnload autocmd rejects its diff. That loop stops as soon as an autocmd deletes the buffer it fired for. +---The reject of a new-file diff with `on_new_file_reject = "keep_empty"`, in a tab that the diff did not open, deletes +---its proposal buffer, and another plugin's buffer can delete itself in its own BufUnload. The diffs behind that +---buffer then reach this point pending. SIGTERM and SIGHUP run VimLeavePre without the loop, so every diff reaches it +---pending. SIGKILL or a crash runs no VimLeavePre. The pending diffs are answered before the cleanup removes them. local function cleanup_at_exit() M._reject_pending_diffs() M._cleanup_all_active_diffs("shutdown") diff --git a/lua/claudecode/server/README.md b/lua/claudecode/server/README.md index 48ff50fd7..d0e7d410e 100644 --- a/lua/claudecode/server/README.md +++ b/lua/claudecode/server/README.md @@ -20,7 +20,7 @@ Layers, top to bottom: - `M.state`: `{ server, port, auth_token, handlers, ping_timer }` [server/init.lua:M.state] - `M.start(config, auth_token) -> ok, port|err`: registers handlers, calls `tools.setup(M)`, creates the TCP server, and starts the ping timer. It returns "Unknown server creation error" when `create_server` fails without a message [server/init.lua:M.start] -- `M.stop() -> ok, err?`: stops the ping timer, closes pending diffs, stops the TCP server, and clears deferred responses [server/init.lua:M.stop] +- `M.stop() -> ok, err?`: stops the ping timer, closes pending diffs, answers each request whose handler is still running, stops the TCP server, and clears deferred responses [server/init.lua:M.stop] - `M.register_handlers()`: handles `initialize`, `notifications/initialized`, `prompts/list`, `tools/list`, and `tools/call` [server/init.lua:M.register_handlers] - `initialize` returns `protocolVersion = "2024-11-05"` (`MCP_PROTOCOL_VERSION`), `capabilities` (logging, `prompts.listChanged`, `tools.listChanged`), `serverInfo.name = "claudecode-neovim"`, and `serverInfo.version = require("claudecode").version:string()` [server/init.lua:MCP_PROTOCOL_VERSION] [server/init.lua:M.register_handlers] - `tools/call` checks the shape of its params first, then delegates to `tools.handle_invoke` and returns a `_deferred` table (`{ _deferred = true, coroutine, client, params }`) for blocking tools @@ -96,6 +96,7 @@ Layers, top to bottom: - A message with a truthy `id` is a request. Any other message is a notification. A valid notification never gets a response. The -32700 and -32600 checks run before the `id` check, so a malformed notification gets an error reply [server/init.lua:M._handle_message] - Notification handlers run in `pcall`. Their errors and return values are discarded, and an unknown notification is ignored without a reply [server/init.lua:M._handle_notification] - A deferred response is keyed by `tostring(coroutine)` in `_G.claude_deferred_responses`. It replies to `result.client` with the original request id. A result with `content` is sent as `result`, one with `error` is sent as `error`, and a result with neither becomes -32603 [server/init.lua:M._setup_deferred_response] +- `_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" (`data`: "The server stopped before the call finished") before it stops the TCP server, and `_handle_request` sends nothing for that request afterwards. A blocking tool has no response sender before its handler yields, so this is the only answer to a call when Neovim exits inside the handler, for example from a `nested = true` `ClaudeCodeDiffOpened` handler that runs `:qa!` [server/init.lua:M._handle_request] [server/init.lua:M.stop] - A ping timer tick pings every connected client and never closes one for a missing answer. `client.ping_unanswered_since` holds the loop time of the oldest ping sent since bytes were last read from the client. The read callback clears it on any bytes, not only a pong: Claude shows it is alive by writing anything, and a pong that waits behind more unread data than the socket holds is not in the first poll after a block. The tick calls `vim.loop.update_time()` before it reads the clock, because libuv caches the loop time when a pass of the main loop starts and runs the timers of the pass with it, and a callback that ran before the tick in the pass can have blocked since, or the machine can have slept in it. A stale time would hide a wake and would stamp a ping older than it is. Each tick that pinged a client arms `after_next_poll` (below), and its callback runs `judge_silence` after the next poll. libuv runs the check handle right after the poll phase that reads the sockets, so every byte that reached a socket before the loop time of the check was read. The check closes each client that the tick pinged when its oldest ping is `interval * 2.5` old, but only if its loop time is at most `interval * 1.5` after the tick. After a longer block Neovim and Claude can have slept together (a suspended machine), and Claude has had no time to answer. A tick cannot decide this, because libuv can run a tick before it reads the pong that Claude sent: after a block in a check callback or an I/O read callback, and, with libuv 1.44 (bundled in Neovim 0.8), after a block in any callback. libuv 1.44 also runs the timers again at the end of a pass, so two ticks can run with no read between them. `vim.schedule` cannot decide it either, because Neovim runs its callbacks between two passes, so it can run one before the read. Drop bound: a block of the main loop, of any length and in any callback, does not close a Claude that writes something within `interval * 2.5` (75 s by default) of each ping. A tick never closes a client, a check judges only after a poll within `interval * 1.5` of its tick, and any bytes clear the stamp. A client that writes data but never answers a ping is not closed by this rule: it is alive, and the plugin does not judge what it writes (the stall rule below closes it when it stops reading, but only once the kernel's socket buffers are full and libuv's write queue backs up, because a queue that is empty counts as progress). A block longer than `interval * 1.5` restarts the count: the next tick sees the gap, takes it for a wake and clears every stamp. So a block delays the close of a silent Claude, and a run of such blocks can postpone it. Without a long block, a Claude that sends nothing is closed at the first read of its socket after the third tick that follows its first unanswered ping, which is 90 s by default after that ping. The Close frame of a dropped client carries 1011 "Keepalive timeout" (RFC 6455 reserves 1006, so no Close frame carries it), `on_disconnect` gets 1006 "Connection timeout", and the check logs `Closed client_: no reply for s` at info level. A peer that stopped reading does not get that Close frame: once its receive window is full, every write to it (replies, pings, and the Close frame) waits in libuv's queue, and closing the socket drops the queue [server/tcp.lua:M.start_ping_timer] [server/tcp.lua:judge_silence] [server/tcp.lua:drop_client] - A Claude that answers each ping but never reads would keep that queue, which is Neovim's memory, without limit, so the server also closes a connected client that answers and whose write queue made no progress at two ticks in a row. Each tick calls `is_client_reading` for each connected client, which calls `sample_write_progress`: it compares `queued_bytes - get_write_queue_size()` with its value at the last progress, and an empty queue counts as progress. A tick that finds progress clears `client.write_stalled`. The first tick that finds none sets it. The second asks for a confirmation, and a confirmation that still finds no progress drops the client: the Close frame carries 1011 "Not reading", `on_disconnect` gets 1006 "Write queue stalled", and the server logs `Closed client_: not reading for s, KB queued` at info level. The confirmation runs right after the next poll. libuv reports that a socket is writable again only in a poll phase, so a tick that Neovim runs after a main-loop block inside an I/O callback can find no progress although Claude read during the block, whether the tick is late or on time. `after_next_poll` arms a check handle, which libuv runs right after each poll phase in every version, and an idle handle, which keeps that poll from waiting for a timer or a socket. The check callback closes both handles, removes them from `server.pending_polls`, and judges the queue once more: progress clears `write_stalled`, and no progress drops the client. A client that left or started to leave in the meantime, because the poll read its EOF, a read error, its Close frame or a protocol error, is skipped without a log line. `stop_server` closes the handles of a confirmation that has not run, which `server.pending_polls` lists. If libuv cannot create a handle, the tick leaves the client for the next tick. The look is a sample at one moment, not proof. libuv takes its snapshot of the ready sockets when a poll starts, so a callback that blocks inside that poll, after the snapshot, hides what Claude reads during the block from the check callback that follows. A client that was stalled for the whole window and resumes during such a block is dropped. That is a deadline race, as with any timeout: the client was stalled when the window ended, and what it read afterwards is not reported yet. Verifier probes closed a reader that resumed 0.1 s into a 0.5 s to 1.5 s block inside the confirming poll, on Neovim 0.12.3 and 0.8.0, at 3 s, 5 s and 30 s intervals. A block after the tick and before that poll, or in a scheduled callback, keeps the reader. The bound is two to three ticks after the last progress, which is 60 s to 90 s at the default interval, plus the delay of late ticks, each of which comes at most `interval * 1.5` after the tick before it. A tick that comes more than `interval * 1.5` after the tick before it counts as a wake from sleep. A wake tick starts a fresh stall window: it clears the count and judges nothing, so it delays the drop by one tick when the count was clear, and by two ticks when the wake tick takes the place of the confirming tick. The next two ticks always count, wake ticks or not, because wake ticks that come again and again are a loop that stays blocked, not sleeps. So whatever the pattern of late ticks, a stalled client is closed at the second tick after a fresh window started. A wake tick three or more ticks after the last fresh start starts a new window. The bound is never the size of the queue, so a Claude that reads slowly, or is sent one message of tens of MB, stays connected while libuv writes any of it. libuv writes more only after the kernel reports the socket writable again, which took 1.2 to 1.5 MB of reading on a Linux loopback socket with default buffers (measured), so a Claude that reads less than about 1.4 MB per window counts as stalled while a backlog exists, and one that reads just faster than that still lets the queue grow as long as Neovim queues more than it reads. `write` queues a frame of more than 1 MiB in pieces of 1 MiB, because libuv on Windows lowers the queue only when a whole write request is done, so a large frame shows its progress per piece there too, and the callback of the last piece reports the frame. [server/tcp.lua:M.start_ping_timer] [server/tcp.lua:judge_stall] [server/tcp.lua:after_next_poll] [server/tcp.lua:M.stop_server] [server/client.lua:M.is_client_reading] [server/client.lua:M.sample_write_progress] [server/client.lua:write] - On one tick a client counts as answered when no ping of it waited for `interval * 2`. Only a client that answered is confirmed for a stall. A client that did not answer is dropped by the keepalive check above, whatever its write queue shows. A client that answers while its queue is stalled is not saved: its bytes clear the stamp, the next tick finds it answered, and then asks for the confirmation. Every connected client is sampled on every tick except one that starts a fresh stall window, answering or not, so "two ticks in a row" means consecutive ticks. A tick gap above `interval * 1.5` counts as a wake from sleep and clears `ping_unanswered_since` of each connected client. A wake tick that starts a fresh stall window also resets `last_drain`, clears `write_stalled`, and does not sample the queue. `last_drain` feeds only the `not reading for s` of the log line, so the reset keeps the sleep out of it. A wake tick within two ticks after the start of a fresh window clears the stamps only, and counts like any tick. A client that is not connected is not pinged, so a main loop that blocks during the handshake does not count against it [server/tcp.lua:M.start_ping_timer] [server/client.lua:M.process_data] @@ -114,7 +115,7 @@ Layers, top to bottom: - `M.start` changes the global tool registry (`tools.setup`), opens a listening TCP handle, and starts a `vim.loop` ping timer [server/init.lua:M.start] - When a client completed its WebSocket handshake (`on_open`, called from the 101 write callback in `client.lua`), the server schedules `claudecode.process_mention_queue(true)`. `on_connect` runs at the TCP accept, before the handshake, and only logs [server/init.lua:M.start] [server/client.lua:M.process_data] - On disconnect, the server schedules `diff.close_diffs_for_client(client.id, "client disconnected")`, only when `package.loaded["claudecode.diff"]` is set and the client completed its handshake. A client without a completed handshake ran no tool, so it owns no diff. The cleanup runs through `utils.when_windows_allowed`: in the command-line window or under textlock it waits, and it runs once Neovim allows window changes again. One 50 ms timer checks for every waiting cleanup [server/init.lua:M.start] [lua/claudecode/utils.lua:M.when_windows_allowed] -- `M.stop` calls `diff.close_pending_diffs("server stopping")` when `package.loaded["claudecode.diff"]` is set, resets `_G.claude_deferred_responses`, and clears `M.state.auth_token` [server/init.lua:M.stop] +- `M.stop` calls `diff.close_pending_diffs("server stopping")` when `package.loaded["claudecode.diff"]` is set, answers the requests in `M.state.in_flight`, resets `_G.claude_deferred_responses`, and clears `M.state.auth_token` [server/init.lua:M.stop] - `M.start` logs only the auth token length, not the token [server/init.lua:M.start] - `tools/call` debug lines name the tool and list the argument and result keys with their sizes (string bytes, table entry counts), never their values, so file contents and selected text stay out of `:messages`. An error result logs its code and message. A rejected call logs one line, `Rejected tools/call: ()`, and none of the others [server/init.lua:M.register_handlers] - `client.create_client` increments `_G.claudecode_clients_created` [server/client.lua:M.create_client] diff --git a/lua/claudecode/server/init.lua b/lua/claudecode/server/init.lua index 8dbded052..7866a0562 100644 --- a/lua/claudecode/server/init.lua +++ b/lua/claudecode/server/init.lua @@ -18,12 +18,14 @@ local M = {} ---@field auth_token string|nil The authentication token for validating connections ---@field handlers table Message handlers by method name ---@field ping_timer table|nil Timer for sending pings +---@field in_flight table The requests whose handler is running, each as `{ client, id }` M.state = { server = nil, port = nil, auth_token = nil, handlers = {}, ping_timer = nil, + in_flight = {}, } ---Initialize the WebSocket server @@ -136,6 +138,16 @@ function M.stop() diff.close_pending_diffs("server stopping") end + -- A request whose handler is still running has no sender yet, so nothing above can answer it. + for in_flight in pairs(M.state.in_flight) do + M.state.in_flight[in_flight] = nil + M.send_response(in_flight.client, in_flight.id, nil, { + code = -32603, + message = "Internal error", + data = "The server stopped before the call finished", + }) + end + tcp_server.stop_server(M.state.server) -- CRITICAL: Clear global deferred responses to prevent memory leaks and hanging @@ -198,7 +210,16 @@ function M._handle_request(client, request) return end + -- A request has no answer until its handler returns, and a blocking tool has no sender before that. When Neovim + -- exits inside the handler (a ClaudeCodeDiffOpened handler that runs :qa!), the handler never returns, so `stop` + -- answers the request, and this function sends nothing after it. + local in_flight = { client = client, id = id } + M.state.in_flight[in_flight] = true local success, result, error_data = pcall(handler, client, params) + if not M.state.in_flight[in_flight] then + return + end + M.state.in_flight[in_flight] = nil if success then -- Check if this is a deferred response (blocking tool) if result and result._deferred then diff --git a/tests/integration/diff_exit_reject_spec.lua b/tests/integration/diff_exit_reject_spec.lua index 4afd56a8a..60bd45008 100644 --- a/tests/integration/diff_exit_reject_spec.lua +++ b/tests/integration/diff_exit_reject_spec.lua @@ -1,8 +1,10 @@ --- Real-Neovim check that `:qa!` answers every openDiff that is still pending, with DIFF_REJECTED, exactly once. +-- Real-Neovim check that :qa! (or SIGTERM) answers every openDiff call that is still open, exactly once: a pending diff +-- gets DIFF_REJECTED, and a call whose setup a ClaudeCodeDiffOpened handler cut off by quitting gets a -32603 error. -- Neovim fires BufUnload for each loaded buffer at exit, and the proposal buffers' autocmds reject their diffs. That --- loop stops once an autocmd deletes the buffer it fired for, and the reject of a new-file diff with the default --- `on_new_file_reject = "keep_empty"` deletes its proposal buffer. The diffs behind it get no BufUnload, so the --- VimLeavePre autocmd of diff.lua must answer them. A child Neovim does the exit, because it ends the process. +-- loop stops once an autocmd deletes the buffer it fired for: the reject of a new-file diff with the default +-- `on_new_file_reject = "keep_empty"`, in a tab the diff did not open, deletes its proposal buffer, and another +-- plugin's buffer can delete itself. SIGTERM runs VimLeavePre without the loop. The diffs behind it get no BufUnload, +-- so the VimLeavePre autocmd of diff.lua must answer them. A child Neovim does the exit, because it ends the process. -- Mock vim cannot run an exit, so under busted this is pending. if vim == nil or vim.fn.has("nvim") ~= 1 then describe("openDiff at exit", function() @@ -28,6 +30,16 @@ describe("openDiff at exit", function() diff_opts = { layout = vim.env.EXIT_LAYOUT, on_new_file_reject = vim.env.EXIT_REJECT }, }) local diff = require("claudecode.diff") + if vim.env.EXIT_DELETER == "1" then + local other = vim.api.nvim_create_buf(true, false) + vim.api.nvim_buf_set_lines(other, 0, -1, false, { "another plugin's buffer" }) + vim.api.nvim_create_autocmd("BufUnload", { + buffer = other, + callback = function(args) + vim.api.nvim_buf_delete(args.buf, { force = true }) + end, + }) + end _G.claude_deferred_responses = {} for step in vim.env.EXIT_STEPS:gmatch("[^,]+") do local name, kind = step:match("^(.-):(.*)$") @@ -47,17 +59,73 @@ describe("openDiff at exit", function() end end vim.defer_fn(function() + if vim.env.EXIT_QUIT == "sigterm" then + vim.loop.kill(vim.fn.getpid(), 15) + return + end vim.cmd("qa!") end, 100) ]] - ---Open the diffs of `steps` ("name:new" or "name:old", in order) in a child Neovim, quit it with :qa!, and return - ---the answers that its senders got and its exit code. - local function exit_with(steps, layout, reject) + -- The same steps as requests, through the server's own message handler. The server answers through + -- `send_response`, which the child replaces with a log line, as it has no client on a socket. + local setup_exit_script = [[ + local log_path = vim.env.EXIT_LOG + local function log(line) + local file = assert(io.open(log_path, "a")) + file:write(line, "\n") + file:close() + end + require("claudecode").setup({ + auto_start = true, + terminal = { provider = "none" }, + diff_opts = { layout = vim.env.EXIT_LAYOUT }, + }) + local server = require("claudecode.server.init") + server.send_response = function(_, id, result, error_data) + log("response " .. id .. " " .. (result and result.content[1].text or ("error " .. error_data.code))) + end + vim.api.nvim_create_autocmd("User", { + pattern = "ClaudeCodeDiffOpened", + nested = true, + callback = function(args) + if args.data.tab_name == "tab_" .. vim.env.EXIT_QUIT_IN then + vim.cmd("qa!") + end + end, + }) + local id = 0 + for step in vim.env.EXIT_STEPS:gmatch("[^,]+") do + local name, kind = step:match("^(.-):(.*)$") + local path = vim.env.EXIT_DIR .. "/" .. name + if kind == "old" then + vim.fn.writefile({ "old line" }, path) + else + os.remove(path) + end + id = id + 1 + server._handle_message({ id = "stub" }, vim.json.encode({ + jsonrpc = "2.0", + id = id, + method = "tools/call", + params = { + name = "openDiff", + arguments = { old_file_path = path, new_file_path = path, new_file_contents = "new line\n", tab_name = "tab_" .. name }, + }, + })) + end + ]] + + ---Run `source` in a child Neovim with `env`, and return the lines it logged and its exit code. + local function run_child(source, env) local script = dir .. "/child.lua" local log_path = dir .. "/answers.log" - vim.fn.writefile(vim.split(child_script, "\n"), script) + vim.fn.writefile(vim.split(source, "\n"), script) vim.fn.delete(log_path) + env.EXIT_LOG = log_path + env.EXIT_DIR = dir + -- The child starts a server in some scenarios: its lock file goes to this folder, not to the user's. + env.CLAUDE_CONFIG_DIR = dir .. "/claude" local job = vim.fn.jobstart({ vim.v.progpath, "--headless", @@ -69,15 +137,7 @@ describe("openDiff at exit", function() "set rtp^=" .. vim.fn.getcwd(), "-c", "luafile " .. vim.fn.fnameescape(script), - }, { - env = { - EXIT_LOG = log_path, - EXIT_DIR = dir, - EXIT_LAYOUT = layout, - EXIT_REJECT = reject, - EXIT_STEPS = table.concat(steps, ","), - }, - }) + }, { env = env }) assert.is_true(job > 0, "could not start a child Neovim") local code = vim.fn.jobwait({ job }, 20000)[1] if code == -1 then @@ -87,6 +147,29 @@ describe("openDiff at exit", function() return lines, code end + ---Open the diffs of `steps` ("name:new" or "name:old", in order) in a child Neovim, end it with :qa! (or SIGTERM), + ---and return the answers that its senders got and its exit code. + local function exit_with(steps, layout, reject, deleter, quit) + return run_child(child_script, { + EXIT_LAYOUT = layout, + EXIT_REJECT = reject, + EXIT_STEPS = table.concat(steps, ","), + EXIT_DELETER = deleter and "1" or "0", + EXIT_QUIT = quit or "qa", + }) + end + + ---Send the openDiff of each step to the child's server as a request, and have a `nested = true` ClaudeCodeDiffOpened + ---handler run :qa! during the setup of the step named `quit_in`. The call has no response sender yet, so the child + ---logs what the server sends for each request id. + local function exit_in_opened_handler(steps, quit_in, layout) + return run_child(setup_exit_script, { + EXIT_LAYOUT = layout, + EXIT_STEPS = table.concat(steps, ","), + EXIT_QUIT_IN = quit_in, + }) + end + local function answers_by_name(lines) local answers = {} for _, line in ipairs(lines) do @@ -118,6 +201,60 @@ describe("openDiff at exit", function() local layouts = { "vertical", "unified" } local rejects = { "keep_empty", "close_window" } + for _, layout in ipairs(layouts) do + local runs_here = layout ~= "unified" or vim.fn.has("nvim-0.9") == 1 + local define = runs_here and it or pending + define(("answers each pending diff when SIGTERM ends Neovim (%s layout)"):format(layout), function() + local lines = exit_with({ "new1.txt:new", "b.txt:old" }, layout, "keep_empty", false, "sigterm") + local answers = answers_by_name(lines) + assert.same({ "DIFF_REJECTED" }, answers["new1.txt"], table.concat(lines, "\n")) + assert.same({ "DIFF_REJECTED" }, answers["b.txt"], table.concat(lines, "\n")) + end) + define( + ("answers one diff that another plugin's buffer kept from its BufUnload (%s layout)"):format(layout), + function() + local lines, code = exit_with({ "a.txt:old" }, layout, "keep_empty", true) + assert.equal(0, code, "the child exited with :qa!\n" .. table.concat(lines, "\n")) + assert.same({ "DIFF_REJECTED" }, answers_by_name(lines)["a.txt"], table.concat(lines, "\n")) + end + ) + end + + ---The answers that the server sent, by request id. + local function responses_by_id(lines) + local responses = {} + for _, line in ipairs(lines) do + local id, text = line:match("^response (%d+) (.*)$") + if id then + id = tonumber(id) + responses[id] = responses[id] or {} + table.insert(responses[id], text) + end + end + return responses + end + + for _, layout in ipairs(layouts) do + local runs_here = layout ~= "unified" or vim.fn.has("nvim-0.9") == 1 + local define = runs_here and it or pending + define( + ("answers the call whose setup was running when a handler quit Neovim (%s layout)"):format(layout), + function() + local lines = exit_in_opened_handler({ "new1.txt:new", "b.txt:old" }, "b.txt", layout) + + local responses = responses_by_id(lines) + local log = table.concat(lines, "\n") + assert.same({ "DIFF_REJECTED" }, responses[1], log) + assert.same({ "error -32603" }, responses[2], log) + end + ) + define(("answers the only call when a handler quit Neovim during its setup (%s layout)"):format(layout), function() + local lines = exit_in_opened_handler({ "a.txt:old" }, "a.txt", layout) + + assert.same({ "error -32603" }, responses_by_id(lines)[1], table.concat(lines, "\n")) + end) + end + for _, layout in ipairs(layouts) do for _, reject in ipairs(rejects) do for _, scenario in ipairs(scenarios) do diff --git a/tests/unit/diff_exit_during_setup_spec.lua b/tests/unit/diff_exit_during_setup_spec.lua new file mode 100644 index 000000000..eea98328a --- /dev/null +++ b/tests/unit/diff_exit_during_setup_spec.lua @@ -0,0 +1,131 @@ +-- Neovim exits inside a `nested = true` ClaudeCodeDiffOpened handler, while the second openDiff is still in setup. +-- That call is pending in the diff table, but it has no response sender yet, because the server stores one only after +-- the tool handler yielded. The exit never unwinds the stack, so the setup never returns its early answer. The server's +-- `stop` must answer the call, whichever of the two VimLeavePre autocmds (diff cleanup and server stop) runs first. +-- The real exit, with a child Neovim, is tests/integration/diff_exit_reject_spec.lua. +require("tests.busted_setup") +local temp_dir = require("tests.helpers.temp_dir") + +describe("Neovim exits inside a ClaudeCodeDiffOpened handler", function() + local diff, server, tools, tcp + local dir, saved_stop_server, saved_exec_autocmds, responses, exited + local client = { id = "client-setup-exit" } + + ---The VimLeavePre autocmd of diff.lua, run as Neovim runs it. + local function diff_leave_pre() + local callbacks = {} + for _, event in ipairs(vim._autocmds["ClaudeCodeMCPDiff"].events) do + if event.events == "VimLeavePre" then + callbacks[#callbacks + 1] = event.opts.callback + end + end + assert.equals(1, #callbacks, "expected one VimLeavePre autocmd in diff.lua") + callbacks[1]() + end + + ---Stands in for `callback = function() vim.cmd("qa!") end`: both VimLeavePre autocmds run inside the handler, and + ---nothing runs after them, because the process is gone. + local function exit_in_opened_handler(tab_name, order) + vim.api.nvim_exec_autocmds = function(event, opts) + if event == "User" and opts.pattern == "ClaudeCodeDiffOpened" and opts.data.tab_name == tab_name then + for _, step in ipairs(order) do + step() + end + exited = true + end + return saved_exec_autocmds(event, opts) + end + end + + local function open_diff(id, name) + local path = dir .. "/" .. name + local file = assert(io.open(path, "w")) + file:write("old line\n") + file:close() + server._handle_request(client, { + jsonrpc = "2.0", + id = id, + method = "tools/call", + params = { + name = "openDiff", + arguments = { old_file_path = path, new_file_path = path, new_file_contents = "new line\n", tab_name = name }, + }, + }) + end + + local function replies_for(id) + local found = {} + for _, response in ipairs(responses) do + if response.id == id then + found[#found + 1] = response + end + end + return found + end + + before_each(function() + for _, name in ipairs({ "claudecode.diff", "claudecode.server.init" }) do + package.loaded[name] = nil + end + diff = require("claudecode.diff") + server = require("claudecode.server.init") + tools = require("claudecode.tools.init") + tcp = require("claudecode.server.tcp") + diff.setup({ terminal = {}, diff_opts = {} }) + server.register_handlers() + tools.setup(server) + saved_stop_server = tcp.stop_server + tcp.stop_server = function() end + server.state.server = {} -- a running server, as far as stop can tell + package.loaded["claudecode.terminal"] = { + get_active_terminal_bufnr = function() + return nil + end, + ensure_visible = function() end, + } + saved_exec_autocmds = vim.api.nvim_exec_autocmds + dir = temp_dir.new() + exited, responses = false, {} + -- Once the process is gone, nothing it would send later leaves it. + server.send_response = function(_, id, result, error_data) + if not exited then + responses[#responses + 1] = { id = id, result = result, error = error_data } + end + end + end) + + after_each(function() + vim.api.nvim_exec_autocmds = saved_exec_autocmds + tcp.stop_server = saved_stop_server + diff._cleanup_all_active_diffs("test cleanup") + package.loaded["claudecode.terminal"] = nil + temp_dir.remove(dir) + end) + + local orders = { + { name = "the diff cleanup first, then the server", steps = { diff_leave_pre, "stop" } }, + { name = "the server first, then the diff cleanup", steps = { "stop", diff_leave_pre } }, + } + + for _, case in ipairs(orders) do + it("answers both calls once, with " .. case.name, function() + local steps = {} + for _, step in ipairs(case.steps) do + steps[#steps + 1] = step == "stop" and function() + server.stop() + end or step + end + open_diff(1, "first.txt") -- setup is over: the diff waits, and its sender exists + exit_in_opened_handler("second.txt", steps) + + open_diff(2, "second.txt") -- the exit happens in this call's setup + + local first, second = replies_for(1), replies_for(2) + assert.equals(1, #first, "the waiting call gets one answer") + assert.equals("DIFF_REJECTED", first[1].result.content[1].text) + assert.equals(1, #second, "the call in setup gets one answer") + assert.equals(-32603, second[1].error.code) + assert.equals(2, #responses) + end) + end +end) diff --git a/tests/unit/diff_exit_reject_spec.lua b/tests/unit/diff_exit_reject_spec.lua index 4079a794e..dc356959d 100644 --- a/tests/unit/diff_exit_reject_spec.lua +++ b/tests/unit/diff_exit_reject_spec.lua @@ -1,8 +1,8 @@ -- Neovim stops firing BufUnload at exit once an autocmd deletes the buffer it fired for (main.c, exit_loop in --- getout), and a new-file reject with `on_new_file_reject = "keep_empty"` deletes its proposal buffer. So the --- diffs behind that buffer get no BufUnload, reach VimLeavePre pending, and used to be removed unanswered. These --- tests run the VimLeavePre autocmd of diff.lua over the states that exit can leave. The real exit, with real --- buffers, is tests/integration/diff_exit_reject_spec.lua. +-- getout). A new-file reject with `on_new_file_reject = "keep_empty"`, in a tab the diff did not open, deletes its +-- proposal buffer, and SIGTERM runs VimLeavePre without the loop. So diffs can reach VimLeavePre pending, and they +-- used to be removed unanswered. These tests run the VimLeavePre autocmd of diff.lua over the states exit can leave. +-- The real exit, with real buffers, is tests/integration/diff_exit_reject_spec.lua. require("tests.busted_setup") describe("diffs still pending when Neovim exits", function() @@ -87,9 +87,10 @@ describe("diffs still pending when Neovim exits", function() end) it("answers the oldest pending diff first", function() - open_pending("first.txt") - open_pending("second.txt") - open_pending("third.txt") + -- Open order differs from name order, so a sort by tab name answers in another order. + open_pending("c.txt") + open_pending("a.txt") + open_pending("b.txt") leave() @@ -97,7 +98,7 @@ describe("diffs still pending when Neovim exits", function() for _, answer in ipairs(answers) do names[#names + 1] = answer.name end - assert.same({ "first.txt", "second.txt", "third.txt" }, names) + assert.same({ "c.txt", "a.txt", "b.txt" }, names) end) it("sends no second answer for a diff that its BufUnload already rejected", function() diff --git a/tests/unit/server/in_flight_request_spec.lua b/tests/unit/server/in_flight_request_spec.lua new file mode 100644 index 000000000..008d0ee99 --- /dev/null +++ b/tests/unit/server/in_flight_request_spec.lua @@ -0,0 +1,105 @@ +-- A request has no answer until its handler returns, and a blocking tool has no response sender before that. When +-- Neovim exits inside a handler, the handler never returns, so `server.stop` must answer the request: Claude gets one +-- answer for every call that reached the server, and `_handle_request` sends nothing after a stop that answered it. +require("tests.busted_setup") + +describe("a request whose handler is still running when the server stops", function() + local server, tcp, saved_stop_server, replies + local client = { id = "in-flight-client" } + + local function request(id, method) + return { jsonrpc = "2.0", id = id, method = method or "slow", params = {} } + end + + before_each(function() + package.loaded["claudecode.server.init"] = nil + server = require("claudecode.server.init") + tcp = require("claudecode.server.tcp") + saved_stop_server = tcp.stop_server + tcp.stop_server = function() end + server.state.server = {} -- a running server, as far as stop can tell + replies = {} + server.send_response = function(_, id, result, error_data) + replies[#replies + 1] = { id = id, result = result, error = error_data } + end + end) + + after_each(function() + tcp.stop_server = saved_stop_server + end) + + local function replies_for(id) + local found = {} + for _, reply in ipairs(replies) do + if reply.id == id then + found[#found + 1] = reply + end + end + return found + end + + it("answers the request once with -32603, and sends nothing when the handler returns", function() + server.state.handlers.slow = function() + server.stop() -- Neovim exits here: VimLeavePre runs inside the handler + return { content = { { type = "text", text = "too late" } } } + end + + server._handle_request(client, request(7)) + + local answers = replies_for(7) + assert.equals(1, #answers, "one answer for one request") + assert.equals(-32603, answers[1].error.code) + assert.is_nil(answers[1].result) + assert.is_nil(next(server.state.in_flight)) + end) + + it("answers every request whose handler is running, the outer one too", function() + server.state.handlers.inner = function() + server.stop() + end + server.state.handlers.outer = function() + server._handle_request(client, request(2, "inner")) + end + + server._handle_request(client, request(1, "outer")) + + for _, id in ipairs({ 1, 2 }) do + local answers = replies_for(id) + assert.equals(1, #answers, "request " .. id .. " gets one answer") + assert.equals(-32603, answers[1].error.code) + end + assert.equals(2, #replies) + end) + + it("answers a finished request through its own return, and leaves nothing in flight", function() + server.state.handlers.slow = function() + return { content = { { type = "text", text = "done" } } } + end + + server._handle_request(client, request(3)) + server.stop() + + assert.equals(1, #replies) + assert.equals("done", replies[1].result.content[1].text) + assert.is_nil(next(server.state.in_flight)) + end) + + it("answers a request whose handler raised once, and leaves nothing in flight", function() + server.state.handlers.slow = function() + error("boom") + end + + server._handle_request(client, request(4)) + server.stop() + + assert.equals(1, #replies) + assert.equals(-32603, replies[1].error.code) + assert.is_nil(next(server.state.in_flight)) + end) + + it("sends nothing when no handler is running", function() + assert.is_true(server.stop()) + + assert.same({}, replies) + end) +end) From cbc8b2f1a46f69878e88d98c45c5e32308614d45 Mon Sep 17 00:00:00 2001 From: Thomas Kosiewski Date: Thu, 12 Nov 2026 06:15:30 +0000 Subject: [PATCH 04/11] docs(server): state what an exit answers, and test the order of the stop The verifiers of the second round found two README lines that the PR made false, three wording slips and one test gap. - server README: M.state lists in_flight. - README.md and lua/claudecode/README.md: not every exit answers each pending diff. A quit from inside an autocmd that is not nested runs no VimLeavePre, like SIGKILL or a crash: Claude gets no answer, its socket closes with 1006, and the lock file stays. A pending diff whose setup has not finished gets -32603, not DIFF_REJECTED. Each sentence was checked against a probe on Neovim 0.8.0 to 0.12.3. - server/init.lua: the comment names a nested ClaudeCodeDiffOpened handler. - SKILL.md: a handler that runs :ClaudeCodeStop is not modeled. - A new unit test records what reaches the socket and asserts that stop sends the -32603 answer before it stops the TCP server. A move of that answer after stop_server or after M.state.server = nil passed every test and left the call unanswered on a real socket. covered.lock records the new hash of server/init.lua. --- .agents/skills/verify-diff-lifecycle/SKILL.md | 2 +- .../skills/verify-diff-lifecycle/covered.lock | 4 +-- README.md | 9 ++--- lua/claudecode/README.md | 2 +- lua/claudecode/server/README.md | 2 +- lua/claudecode/server/init.lua | 2 +- tests/unit/server/in_flight_request_spec.lua | 36 +++++++++++++++++++ 7 files changed, 47 insertions(+), 10 deletions(-) diff --git a/.agents/skills/verify-diff-lifecycle/SKILL.md b/.agents/skills/verify-diff-lifecycle/SKILL.md index 09fc50834..367eae788 100644 --- a/.agents/skills/verify-diff-lifecycle/SKILL.md +++ b/.agents/skills/verify-diff-lifecycle/SKILL.md @@ -125,7 +125,7 @@ Then decide which kind of failure it is: - The invariants run with 3 openDiff calls, 2 tab names and 2 clients, about 93,000 states. `DisconnectedCallsAnswered` runs with 2 calls, 2 tabs and 2 clients. It assumes weak fairness only on setup and on the scheduled disconnect cleanup. The user is never assumed to act, with one exception: the disconnect cleanup waits until Neovim allows window changes (`utils.when_windows_allowed`), so a user in the command-line window is assumed to leave it, and textlock is assumed to end. - In every run, a user `ClaudeCodeDiffOpened` handler (README "Diff Lifecycle Events") can accept or deny the diff during setup: `:ClaudeCodeDiffAccept`, `:ClaudeCodeDiffDeny`, or a `nested = true` autocmd that closes the proposal window. A default handler that closes the window resolves nothing, because WinClosed does not fire inside a non-nested autocmd. Other handler actions during setup are not modeled. - `DiffCleanupAtExit` is the `VimLeavePre` callback of `diff.lua`: `_reject_pending_diffs`, then `_cleanup_all_active_diffs("shutdown")`. It models neither a `ClaudeCodeDiffClosed` handler (the reject pass has none unless a `keep_empty` reject runs a cleanup) nor a saved diff lost at exit: Neovim quits right after the step, and its buffers go with it. The model represents `DiffCleanupAtExit` and `ServerStops` as separate atomic actions and explores both orders, because both are `VimLeavePre` autocmds. Neovim's own exit `BufUnload` loop, which rejects before `VimLeavePre`, is the `UserRejects` action. -- `ExitInSetup` is an exit inside a `nested = true` `ClaudeCodeDiffOpened` handler, while one call is in setup, so it needs `OpenedEventInSetup`. The call has no sender and its setup never returns, so only the server stop answers it, with an error (`ExitAnswersInFlight`). The action runs both `VimLeavePre` autocmds in one order, diff cleanup first, then the server stop. The other order gives the same result, because each leaves the other nothing to do. A handler that runs `:ClaudeCodeStop` instead of an exit reaches the same `M.stop` code and is not a separate action. +- `ExitInSetup` is an exit inside a `nested = true` `ClaudeCodeDiffOpened` handler, while one call is in setup, so it needs `OpenedEventInSetup`. The call has no sender and its setup never returns, so only the server stop answers it, with an error (`ExitAnswersInFlight`). The action runs both `VimLeavePre` autocmds in one order, diff cleanup first, then the server stop. The other order gives the same result, because each leaves the other nothing to do. A handler that runs `:ClaudeCodeStop` instead of an exit is not modeled: the handler returns after the stop, and `tests/unit/server/in_flight_request_spec.lua` checks that `_handle_request` then sends nothing. - `SenderCleared` alone is caught by no rule, because the pending guards also stop a second answer. `--mutants` shows that `AtMostOneAnswer` breaks when both `SenderCleared` and `GuardRejected` are removed. - `SavedDiffKept` knows which client owns each diff, and, while `:ClaudeCodeCloseAllDiffs` runs, which files or file buffers hold the accepted text. The model has no file or buffer contents: the command's first pass leaves the accepted text in any subset of the saved diffs' files or buffers, because the code skips a diff whose file buffer already has unsaved changes, or whose buffer it cannot change. The plugin never writes the file. That the first pass leaves the file on disk and the user's unsaved changes alone is checked by the unit tests, not here. A saved diff counts as lost when the user, a disconnect cleanup or a server stop tears it down, or when a client that does not own it does so while the owner is disconnected. A close by another client while the owner is connected counts as safe, because the owner got `FILE_SAVED` and can still write the file. The code is stricter than the rule: it keeps another client's saved diff in both cases. Ownership compares client ids, so a new connection must never get an old id. With `ClientIdsUnique` off, `ClientIdReused` lets a new connection take the id of a disconnected client. - The split and unified layouts share the pending guards only because `resolve_inline_as_*` is reached through `_resolve_diff_as_*`. The inline functions have no guard of their own. diff --git a/.agents/skills/verify-diff-lifecycle/covered.lock b/.agents/skills/verify-diff-lifecycle/covered.lock index a0270162e..0a7bd0730 100644 --- a/.agents/skills/verify-diff-lifecycle/covered.lock +++ b/.agents/skills/verify-diff-lifecycle/covered.lock @@ -150,10 +150,10 @@ file lua/claudecode/server/frame.lua 3c50766875878d56 == lua/claudecode/server/handshake.lua file lua/claudecode/server/handshake.lua 3eca35d572094a55 == lua/claudecode/server/init.lua -file lua/claudecode/server/init.lua 7a2c6c65948073e5 +file lua/claudecode/server/init.lua 8a3d1b16b91dd5a7 block server_stop 506e2e53358dc947 block on_disconnect a0ce86cede4af786 -block handle_request dcc066af67f9f88e +block handle_request 9cdbba664a85319f block setup_deferred f9381caaae0fe517 site lua/claudecode/server/init.lua: _G.claude_deferred_responses = {} @ function M._setup_deferred_response(deferred_info) site lua/claudecode/server/init.lua: _G.claude_deferred_responses = {} @ function M.stop() diff --git a/README.md b/README.md index a3f21f0f6..01227b9aa 100644 --- a/README.md +++ b/README.md @@ -1146,10 +1146,11 @@ The plugin runs the CLI through a [terminal provider](./lua/claudecode/terminal/ buffer it fired for. The reject of a new-file diff in a tab it did not open, with `diff_opts.on_new_file_reject = "keep_empty"` (the default), deletes its proposal buffer [lua/claudecode/diff.lua:M.\_resolve_diff_as_rejected] [lua/claudecode/diff.lua:M.\_cleanup_diff_state], so the later diffs get no `BufUnload`. SIGTERM and SIGHUP run - `VimLeavePre` without that loop, and SIGKILL or a crash runs no `VimLeavePre`. The `VimLeavePre` autocmd of - `ClaudeCodeMCPDiff` therefore rejects every diff that is still pending, oldest first, before it removes any - [lua/claudecode/diff.lua:cleanup_at_exit], so each pending diff gets one `DIFF_REJECTED` in every exit that runs - `VimLeavePre`. A call whose setup has not finished when Neovim exits has no response sender yet. This happens when a + `VimLeavePre` without that loop. SIGKILL, a crash, or a quit from inside an autocmd that is not `nested` runs no + `VimLeavePre`: Claude then gets no answer, its socket closes with 1006, and the lock file stays. In an exit that runs + `VimLeavePre`, the autocmd of `ClaudeCodeMCPDiff` rejects every diff that is still pending, oldest first, before it + removes any [lua/claudecode/diff.lua:cleanup_at_exit], so each pending diff whose `openDiff` setup has finished gets + one `DIFF_REJECTED`. A call whose setup has not finished when Neovim exits has no response sender yet. This happens when a `nested = true` `ClaudeCodeDiffOpened` handler runs `:qa!`. The server stop answers that call with a -32603 error [lua/claudecode/server/init.lua:M.stop]. diff --git a/lua/claudecode/README.md b/lua/claudecode/README.md index 9f2fe0544..73f36cf72 100644 --- a/lua/claudecode/README.md +++ b/lua/claudecode/README.md @@ -182,7 +182,7 @@ Module map (direct files): - In the `vertical` and `horizontal` layouts the saved content is the lines of the proposed buffer, joined with line breaks, plus a final line break only when the buffer's `'eol'` is on. The diff sets `'eol'` from the proposal when it creates the buffer, so a proposal without a final line break is saved without one, and the user's edits keep that ending [diff.lua:M._setup_blocking_diff] [diff.lua:M._resolve_diff_as_saved] - A pending diff rejects at once when its proposed buffer is shown in no window, in the split and the unified layout: its last window closes (`:q`, `:close`, `c`, `:tabclose`), or that window switches to another buffer (`:edit`, `:buffer`, `:enew`, which only hide it). A copy of the proposal in another window or tab defers the reject until that copy leaves too. The switch path rejects on the next loop turn, and only when the diff is still the same one and still hidden. Both layouts keep the proposed buffer's `'bufhidden'` at `hide`: the buffer is modified, and `wipe` would make `:q` fail with E37 [diff.lua:M._register_reject_when_hidden] [diff_inline.lua:M.setup_inline_diff] - A diff that has its answer, by either one, clears `'modified'` on its proposed buffer. While the flag stayed set, `:wq` left the window open, and `:qa` stopped with E37 or E162 and moved a window onto the hidden buffer until `close_tab` deleted it [diff.lua:mark_proposal_unmodified] -- At exit, each diff that is still `pending` gets `DIFF_REJECTED` once. Neovim fires `BufUnload` for each loaded buffer before `VimLeavePre`, and a proposal's `BufUnload` autocmd rejects its diff, but that loop stops as soon as an autocmd deletes the buffer it fired for. The reject of a new-file diff with `on_new_file_reject = "keep_empty"`, in a tab that the diff did not open, deletes its proposal buffer, and another plugin's buffer can delete itself in its own `BufUnload`. The diffs after either get no `BufUnload` (probed on Neovim 0.8.0, 0.9.5, 0.10.4, 0.11.5 and 0.12.3). SIGTERM and SIGHUP run `VimLeavePre` without that loop, and SIGKILL or a crash runs no `VimLeavePre`. The `VimLeavePre` autocmd therefore rejects every diff still `pending`, oldest first, before `_cleanup_all_active_diffs("shutdown")` removes any entry, so each pending diff gets its answer in every exit path that runs `VimLeavePre`. A diff that already has its answer gets no second one. A call whose setup has not finished has no response sender yet. When a `nested = true` `ClaudeCodeDiffOpened` handler runs `:qa!` during that setup, `server.stop` answers the call with -32603 [diff.lua:M._reject_pending_diffs] [diff.lua:cleanup_at_exit] [server/init.lua:M.stop] +- An exit that runs `VimLeavePre` answers each `openDiff` call that is still open once: `DIFF_REJECTED` for a diff that is pending, or a -32603 error for a call whose setup has not finished (below). A quit from inside an autocmd that is not `nested` runs no `VimLeavePre`, and neither do SIGKILL or a crash. Claude then gets no answer, its socket closes with 1006, and the lock file stays. Neovim fires `BufUnload` for each loaded buffer before `VimLeavePre`, and a proposal's `BufUnload` autocmd rejects its diff, but that loop stops as soon as an autocmd deletes the buffer it fired for. The reject of a new-file diff with `on_new_file_reject = "keep_empty"`, in a tab that the diff did not open, deletes its proposal buffer, and another plugin's buffer can delete itself in its own `BufUnload`. The diffs after either get no `BufUnload` (probed on Neovim 0.8.0, 0.9.5, 0.10.4, 0.11.5 and 0.12.3). SIGTERM and SIGHUP run `VimLeavePre` without that loop. The `VimLeavePre` autocmd therefore rejects every diff still `pending`, oldest first, before `_cleanup_all_active_diffs("shutdown")` removes any entry, so each pending diff whose setup has finished gets its `DIFF_REJECTED` in every exit that runs `VimLeavePre`. A diff that already has its answer gets no second one. A call whose setup has not finished has no response sender yet. When a `nested = true` `ClaudeCodeDiffOpened` handler runs `:qa!` during that setup, `server.stop` answers the call with -32603 [diff.lua:M._reject_pending_diffs] [diff.lua:cleanup_at_exit] [server/init.lua:M.stop] - The cleanup of a diff closes the proposal window, and the window that the split layout opened for the file's old text, only while each still shows its buffer. A window that the user switched to another buffer stays with that buffer. `:edit` turns diff mode off by itself, except when the new buffer is in diff mode in another window, as the file's own buffer is. So the split layout's cleanup ends diff mode only in a kept window that shows the file's own buffer, and leaves a diff mode in a window that shows another buffer, which is the user's. One limit comes from Neovim: the default `closeoff` in `'diffopt'` turns diff mode off in the last window that has it when another diff window closes. That ends a user's diff mode in a kept original-side window when the proposal window closes, and in a kept proposal window when the cleanup closes the window that the plugin opened for the file. A user's diff mode over two or more windows stays. The unified layout needs no cleanup there: it sets `'signcolumn'` to `yes` with local scope, for the proposal buffer only, so the window shows the sign column it had before the diff once the user switches it, and keeps a value the user sets afterwards. Two cases still close the window: a diff in its own tab (`open_in_new_tab`, or a tab that already has a diff) closes the whole tab, and the fallback window that the plugin opens in a tab that holds only the Claude terminal always closes [diff.lua:M._close_proposal_window] [diff.lua:M._cleanup_diff_state] [diff_inline.lua:M.cleanup_inline_diff] [diff_inline.lua:M.setup_inline_diff] - Rejecting a new-file diff that is not in its own tab cleans it up at once when `diff_opts.on_new_file_reject` is `"keep_empty"` (the default), before `close_tab`. A `unified` diff gets no such cleanup [diff.lua:M._resolve_diff_as_rejected] [diff_inline.lua:M.resolve_inline_as_rejected] - `BufDelete`, `BufUnload`, and `BufWipeout` on the proposed buffer reject a pending diff. `WinClosed` rejects it only when no other window in any tab shows the proposed buffer. The original file's buffer is not watched. The unified layout registers the same autocmds [diff.lua:register_diff_autocmds] [diff_inline.lua:M.setup_inline_diff] diff --git a/lua/claudecode/server/README.md b/lua/claudecode/server/README.md index cf3a63e14..37e4bc1e5 100644 --- a/lua/claudecode/server/README.md +++ b/lua/claudecode/server/README.md @@ -18,7 +18,7 @@ Layers, top to bottom: ### Lifecycle and messaging (`init.lua`) -- `M.state`: `{ server, port, auth_token, handlers, ping_timer }` [server/init.lua:M.state] +- `M.state`: `{ server, port, auth_token, handlers, ping_timer, in_flight }`, where `in_flight` holds the requests whose handler is running [server/init.lua:M.state] - `M.start(config, auth_token) -> ok, port|err`: registers handlers, calls `tools.setup(M)`, creates the TCP server, and starts the ping timer. It returns "Unknown server creation error" when `create_server` fails without a message [server/init.lua:M.start] - `M.stop() -> ok, err?`: stops the ping timer, closes pending diffs, answers each request whose handler is still running, stops the TCP server, and clears deferred responses [server/init.lua:M.stop] - `M.register_handlers()`: handles `initialize`, `notifications/initialized`, `prompts/list`, `tools/list`, and `tools/call` [server/init.lua:M.register_handlers] diff --git a/lua/claudecode/server/init.lua b/lua/claudecode/server/init.lua index e32cd74ad..53731f1f0 100644 --- a/lua/claudecode/server/init.lua +++ b/lua/claudecode/server/init.lua @@ -216,7 +216,7 @@ function M._handle_request(client, request) end -- A request has no answer until its handler returns, and a blocking tool has no sender before that. When Neovim - -- exits inside the handler (a ClaudeCodeDiffOpened handler that runs :qa!), the handler never returns, so `stop` + -- exits inside the handler (a `nested = true` ClaudeCodeDiffOpened handler that runs :qa!), the handler never returns, so `stop` -- answers the request, and this function sends nothing after it. local in_flight = { client = client, id = id } M.state.in_flight[in_flight] = true diff --git a/tests/unit/server/in_flight_request_spec.lua b/tests/unit/server/in_flight_request_spec.lua index 008d0ee99..2a230c582 100644 --- a/tests/unit/server/in_flight_request_spec.lua +++ b/tests/unit/server/in_flight_request_spec.lua @@ -103,3 +103,39 @@ describe("a request whose handler is still running when the server stops", funct assert.same({}, replies) end) end) + +-- The answer must leave before the TCP server stops. After `tcp_server.stop_server` the clients are closing, and after +-- `M.state.server = nil` `send_response` sends nothing, so an answer moved there never reaches Claude. The describe +-- above replaces `send_response` and `stop_server`, so it cannot see the order. This one records what reaches the socket. +describe("server.stop with a request in flight, as the socket sees it", function() + local server, tcp, saved, order + + before_each(function() + package.loaded["claudecode.server.init"] = nil + server = require("claudecode.server.init") + tcp = require("claudecode.server.tcp") + saved = { send_to_client = tcp.send_to_client, stop_server = tcp.stop_server } + order = {} + tcp.send_to_client = function(_, client_id, message) + local answered = message:find("-32603", 1, true) and message:find('"id":9', 1, true) + order[#order + 1] = client_id .. (answered and " answered 9 with -32603" or (" sent " .. message)) + end + tcp.stop_server = function() + order[#order + 1] = "stop_server" + end + server.state.server = {} -- a running server, as far as stop can tell + server.state.handlers.slow = function() + server.stop() + end + end) + + after_each(function() + tcp.send_to_client, tcp.stop_server = saved.send_to_client, saved.stop_server + end) + + it("sends the -32603 answer before it stops the TCP server", function() + server._handle_request({ id = "c1" }, { jsonrpc = "2.0", id = 9, method = "slow", params = {} }) + + assert.same({ "c1 answered 9 with -32603", "stop_server" }, order) + end) +end) From f2862a70b1275f2f3717ed606406aa7ff6f9104b Mon Sep 17 00:00:00 2001 From: Thomas Kosiewski Date: Thu, 12 Nov 2026 06:42:20 +0000 Subject: [PATCH 05/11] docs: name both VimLeavePre autocmds in the Resource Cleanup section ARCHITECTURE.md named only the ClaudeCodeShutdown autocmd of init.lua. A second VimLeavePre autocmd, in the ClaudeCodeMCPDiff group of diff.lua, now answers every pending openDiff call with DIFF_REJECTED before it closes the diffs. The section now names both, says what each one does, and gives the order they run in. A probe fixed the order: 15 runs (Neovim 0.8.0, 0.9.5, 0.10.4, 0.11.5, 0.12.3; default, a second setup call, and diff.lua loaded before setup) with tracer autocmds before and after setup gave BufUnload loop, tracer 1, the diff.lua autocmd (_reject_pending_diffs, then _cleanup_all_active_diffs), init.lua stop (lockfile.remove, tcp.stop_server), tracer 2. The Security Considerations bullet names both autocmds, and says that a SIGKILL, a crash, or a quit from inside a non-nested autocmd skips both. --- ARCHITECTURE.md | 25 +++++++++++++++++++++++-- 1 file changed, 23 insertions(+), 2 deletions(-) diff --git a/ARCHITECTURE.md b/ARCHITECTURE.md index 0031d3ed5..17d16a9bc 100644 --- a/ARCHITECTURE.md +++ b/ARCHITECTURE.md @@ -173,7 +173,26 @@ return true, result ### Resource Cleanup -The `VimLeavePre` autocmd that `setup` creates runs `stop` only when the server runs. Without a server, it only clears the @-mention queue [lua/claudecode/init.lua:M.setup] [lua/claudecode/init.lua:clear_mention_queue]: +At exit, Neovim fires `BufUnload` for each loaded buffer, and then runs the `VimLeavePre` autocmds. The plugin registers two of them, and they run in this order. Neovim runs the autocmds of one event in the order that they were registered. `diff.lua` registers its autocmd when it first loads, and `M.setup` loads `diff.lua` before it registers its own [lua/claudecode/init.lua:M.setup]. + +First, the autocmd of `diff.lua`, in the `ClaudeCodeMCPDiff` group, answers every `openDiff` call that is still pending with `DIFF_REJECTED`, oldest first, while the server still runs. Then it closes every diff and clears its state. It runs whether or not the server runs [lua/claudecode/diff.lua:cleanup_at_exit] [lua/claudecode/diff.lua:M._reject_pending_diffs] [lua/claudecode/diff.lua:M._cleanup_all_active_diffs]: + +```lua +-- diff.lua, module level (shortened) +local function cleanup_at_exit() + M._reject_pending_diffs() -- DIFF_REJECTED for each pending diff, oldest first + M._cleanup_all_active_diffs("shutdown") -- close each diff and clear its state +end + +vim.api.nvim_create_autocmd("VimLeavePre", { + group = get_autocmd_group(), -- "ClaudeCodeMCPDiff" + callback = cleanup_at_exit, +}) +``` + +The `BufUnload` loop that runs before both autocmds already rejects most diffs, because the `BufUnload` autocmd of a proposal buffer rejects its diff [lua/claudecode/diff.lua:register_diff_autocmds]. That loop stops when an autocmd deletes the buffer that it fired for, and SIGTERM and SIGHUP run `VimLeavePre` without the loop. `_reject_pending_diffs` answers the diffs that the loop did not reach. A diff that already has its answer gets no second one. + +Second, the autocmd of `init.lua`, in the `ClaudeCodeShutdown` group, runs `stop` only when the server runs. Without a server, it only clears the @-mention queue [lua/claudecode/init.lua:M.setup] [lua/claudecode/init.lua:clear_mention_queue]. `stop` removes the lock file, stops the server, and clears the queue [lua/claudecode/init.lua:M.stop]. The server's own `stop` answers a call whose handler is still running with the error -32603, and the TCP server then closes each client with the close code 1001 [lua/claudecode/server/init.lua:M.stop] [lua/claudecode/server/tcp.lua:M.stop_server]: ```lua -- init.lua, M.setup (shortened) @@ -190,6 +209,8 @@ vim.api.nvim_create_autocmd("VimLeavePre", { }) ``` +SIGKILL, a crash, and a quit from inside an autocmd that is not `nested` run neither autocmd. Claude then sees the socket drop (close code 1006) with no answer for a pending diff, and the lock file stays. + ## Module Structure ``` @@ -271,6 +292,6 @@ vv netrw # Test with built-in netrw - **Debounced Updates**: 100 ms debounce on selection changes, except leaving visual mode, editing or consuming the held text, and moving the cursor away from a held selection, which report at once - **Localhost Only**: Server binds to 127.0.0.1 -- **Resource Cleanup**: A `VimLeavePre` autocmd runs `stop` when the server runs. `stop` stops the server and tries to remove the lock file [lua/claudecode/init.lua:M.setup] [lua/claudecode/init.lua:M.stop]. A SIGKILL skips it and leaves the lock file +- **Resource Cleanup**: At exit, two `VimLeavePre` autocmds run. The one in `diff.lua` runs first and answers each pending diff. The one in `init.lua` runs `stop` when the server runs, and `stop` stops the server and tries to remove the lock file [lua/claudecode/diff.lua:cleanup_at_exit] [lua/claudecode/init.lua:M.setup] [lua/claudecode/init.lua:M.stop]. A SIGKILL, a crash, or a quit from inside an autocmd that is not `nested` skips both and leaves the lock file - **In-memory State**: The plugin keeps, among other things, the latest selection [lua/claudecode/selection.lua:M.state], the proposed text of each open diff [lua/claudecode/diff.lua:M._register_diff_state], an input buffer for each connected client [lua/claudecode/server/client.lua:M.create_client], and the @-mentions that wait to go out [lua/claudecode/init.lua:mention_queue]. `lockfile.lua` reads `CLAUDE_CONFIG_DIR` once, when it loads [lua/claudecode/lockfile.lua:get_lock_dir] - **Async and Blocking I/O**: Sockets and timers use libuv callbacks. The TCP server accepts connections and reads through callbacks, writes with libuv's asynchronous `write`, and runs its ping timer from a libuv timer [lua/claudecode/server/tcp.lua:M._bind_and_listen] [lua/claudecode/server/tcp.lua:M._handle_new_connection] [lua/claudecode/server/client.lua:write] [lua/claudecode/server/tcp.lua:M.start_ping_timer]. The selection debounce and the @-mention queue use timers [lua/claudecode/selection.lua:M.debounce_update] [lua/claudecode/init.lua:start_timer]. File I/O is synchronous and blocks Neovim until it returns: `lockfile.lua` calls `fs_open`, `fs_write` and `fs_close` without callbacks and uses `os.rename` and `os.remove` [lua/claudecode/lockfile.lua:M.create] [lua/claudecode/lockfile.lua:M.remove], and `diff.lua` reads and writes files with `io.open` [lua/claudecode/diff.lua:read_file] [lua/claudecode/diff.lua:create_temp_file] From 41797477d0a5983d9621fcda31c57f58c9ce17d5 Mon Sep 17 00:00:00 2001 From: Thomas Kosiewski Date: Thu, 12 Nov 2026 09:11:45 +0000 Subject: [PATCH 06/11] docs: state the exit answers once, in the root README Three rounds of review found a false claim about what Claude gets at an exit, each time in another file, because each doc restated the rule in its own words. Round c found two more: ARCHITECTURE.md said the diff.lua autocmd answers every pending call with DIFF_REJECTED, but a call that the exit cuts off during its setup gets a -32603 error from the server stop (its reject stays in setup_result and sends nothing). README.md said that every pending diff is rejected before any is removed, but a keep_empty reject removes its own diff, so a ClaudeCodeDiffClosed handler can see a later diff still pending. The root README now has one bullet, "Exit answers", that states what each kind of call gets, in each kind of exit, with only probed claims. The other docs describe the mechanism they own and link to it: ARCHITECTURE.md (which autocmd runs what, in which order), lua/claudecode/README.md (the BufUnload loop and the two passes), lua/claudecode/server/README.md (what M.stop sends), SKILL.md and check.sh (what the model reads), and two test headers. Two comments in diff.lua said the same wrong thing and now describe the mechanism. The change to diff.lua is comments only: the lines that are not comments are identical to the previous head. covered.lock records the new file hash. --- .agents/skills/verify-diff-lifecycle/SKILL.md | 6 +-- .agents/skills/verify-diff-lifecycle/check.sh | 4 +- .../skills/verify-diff-lifecycle/covered.lock | 2 +- ARCHITECTURE.md | 12 +++--- README.md | 40 +++++++++++++------ lua/claudecode/README.md | 2 +- lua/claudecode/diff.lua | 10 +++-- lua/claudecode/server/README.md | 2 +- tests/integration/diff_exit_reject_spec.lua | 5 +-- tests/unit/server/in_flight_request_spec.lua | 5 ++- 10 files changed, 53 insertions(+), 35 deletions(-) diff --git a/.agents/skills/verify-diff-lifecycle/SKILL.md b/.agents/skills/verify-diff-lifecycle/SKILL.md index 367eae788..a7a65c42b 100644 --- a/.agents/skills/verify-diff-lifecycle/SKILL.md +++ b/.agents/skills/verify-diff-lifecycle/SKILL.md @@ -77,8 +77,8 @@ A changed file with no changed block or call site still needs a look. A wrapper | `CloseTabRejectsPending` | `close_diff_by_tab_name` rejects a pending diff before cleanup | | `CloseEachByName` | `close_active_diffs` closes each match through `close_diff_by_tab_name` | | `StopRejectsPending` | `M.stop` calls `close_pending_diffs`, then `stop_server`, then clears the senders | -| `ExitRejectsPending` | the `VimLeavePre` callback `cleanup_at_exit` calls `_reject_pending_diffs` before `_cleanup_all_active_diffs`, so a pending diff is answered, not removed | -| `ExitAnswersInFlight` | `M.stop` answers each request in `M.state.in_flight`, and `_handle_request` sends nothing after a stop that answered it, so a call cut off in setup is answered | +| `ExitRejectsPending` | the `VimLeavePre` callback `cleanup_at_exit` calls `_reject_pending_diffs` before `_cleanup_all_active_diffs`, so the final cleanup pass finds no pending diff | +| `ExitAnswersInFlight` | `M.stop` answers each request in `M.state.in_flight`, and `_handle_request` sends nothing after a stop that answered it | | `StopPendingOnly`, `DisconnectPendingOnly` | the `status == "pending"` filters in `close_pending_diffs` and `close_diffs_for_client` | | `DisconnectRejects` | `on_disconnect` schedules `close_diffs_for_client` | | `UserCloseAllPendingOnly` | `:ClaudeCodeCloseAllDiffs` calls `close_pending_diffs`, never `close_all_diffs` | @@ -125,7 +125,7 @@ Then decide which kind of failure it is: - The invariants run with 3 openDiff calls, 2 tab names and 2 clients, about 93,000 states. `DisconnectedCallsAnswered` runs with 2 calls, 2 tabs and 2 clients. It assumes weak fairness only on setup and on the scheduled disconnect cleanup. The user is never assumed to act, with one exception: the disconnect cleanup waits until Neovim allows window changes (`utils.when_windows_allowed`), so a user in the command-line window is assumed to leave it, and textlock is assumed to end. - In every run, a user `ClaudeCodeDiffOpened` handler (README "Diff Lifecycle Events") can accept or deny the diff during setup: `:ClaudeCodeDiffAccept`, `:ClaudeCodeDiffDeny`, or a `nested = true` autocmd that closes the proposal window. A default handler that closes the window resolves nothing, because WinClosed does not fire inside a non-nested autocmd. Other handler actions during setup are not modeled. - `DiffCleanupAtExit` is the `VimLeavePre` callback of `diff.lua`: `_reject_pending_diffs`, then `_cleanup_all_active_diffs("shutdown")`. It models neither a `ClaudeCodeDiffClosed` handler (the reject pass has none unless a `keep_empty` reject runs a cleanup) nor a saved diff lost at exit: Neovim quits right after the step, and its buffers go with it. The model represents `DiffCleanupAtExit` and `ServerStops` as separate atomic actions and explores both orders, because both are `VimLeavePre` autocmds. Neovim's own exit `BufUnload` loop, which rejects before `VimLeavePre`, is the `UserRejects` action. -- `ExitInSetup` is an exit inside a `nested = true` `ClaudeCodeDiffOpened` handler, while one call is in setup, so it needs `OpenedEventInSetup`. The call has no sender and its setup never returns, so only the server stop answers it, with an error (`ExitAnswersInFlight`). The action runs both `VimLeavePre` autocmds in one order, diff cleanup first, then the server stop. The other order gives the same result, because each leaves the other nothing to do. A handler that runs `:ClaudeCodeStop` instead of an exit is not modeled: the handler returns after the stop, and `tests/unit/server/in_flight_request_spec.lua` checks that `_handle_request` then sends nothing. +- `ExitInSetup` is an exit inside a `nested = true` `ClaudeCodeDiffOpened` handler, while one call is in setup, so it needs `OpenedEventInSetup`. The call has no sender and its setup never returns, so only the server stop can answer it (`ExitAnswersInFlight`). The action runs both `VimLeavePre` autocmds in one order, diff cleanup first, then the server stop. The other order gives the same result, because each leaves the other nothing to do. A handler that runs `:ClaudeCodeStop` instead of an exit is not modeled: the handler returns after the stop, and `tests/unit/server/in_flight_request_spec.lua` checks that `_handle_request` then sends nothing. What Claude receives at an exit is in the root `README.md`, under Global Invariants. - `SenderCleared` alone is caught by no rule, because the pending guards also stop a second answer. `--mutants` shows that `AtMostOneAnswer` breaks when both `SenderCleared` and `GuardRejected` are removed. - `SavedDiffKept` knows which client owns each diff, and, while `:ClaudeCodeCloseAllDiffs` runs, which files or file buffers hold the accepted text. The model has no file or buffer contents: the command's first pass leaves the accepted text in any subset of the saved diffs' files or buffers, because the code skips a diff whose file buffer already has unsaved changes, or whose buffer it cannot change. The plugin never writes the file. That the first pass leaves the file on disk and the user's unsaved changes alone is checked by the unit tests, not here. A saved diff counts as lost when the user, a disconnect cleanup or a server stop tears it down, or when a client that does not own it does so while the owner is disconnected. A close by another client while the owner is connected counts as safe, because the owner got `FILE_SAVED` and can still write the file. The code is stricter than the rule: it keeps another client's saved diff in both cases. Ownership compares client ids, so a new connection must never get an old id. With `ClientIdsUnique` off, `ClientIdReused` lets a new connection take the id of a disconnected client. - The split and unified layouts share the pending guards only because `resolve_inline_as_*` is reached through `_resolve_diff_as_*`. The inline functions have no guard of their own. diff --git a/.agents/skills/verify-diff-lifecycle/check.sh b/.agents/skills/verify-diff-lifecycle/check.sh index b809368b0..5bc39b276 100755 --- a/.agents/skills/verify-diff-lifecycle/check.sh +++ b/.agents/skills/verify-diff-lifecycle/check.sh @@ -139,8 +139,8 @@ fact[ClientIdsUnique]=$(as_bool client_ids_unique) fact[StopRejectsPending]=$(as_bool in_order 'diff\.close_pending_diffs\(' 'tcp_server\.stop_server\(' \ '_G\.claude_deferred_responses = \{\}' <<<"$server_stop") fact[StopPendingOnly]=$(as_bool grep -qE '^ *return diff_data\.status == "pending"$' <<<"$close_pending") -# At exit, a diff that no BufUnload rejected still gets its answer: the VimLeavePre autocmd rejects every pending diff -# before _cleanup_all_active_diffs removes any entry. +# The VimLeavePre callback runs _reject_pending_diffs before _cleanup_all_active_diffs, so a diff that no BufUnload +# rejected is rejected before the final cleanup pass. FACTS+=(ExitRejectsPending) exit_rejects_pending() { in_order '^function M\._reject_pending_diffs\(\)$' 'diff_data\.status == "pending"' \ diff --git a/.agents/skills/verify-diff-lifecycle/covered.lock b/.agents/skills/verify-diff-lifecycle/covered.lock index b168d38a6..defeaf2bb 100644 --- a/.agents/skills/verify-diff-lifecycle/covered.lock +++ b/.agents/skills/verify-diff-lifecycle/covered.lock @@ -3,7 +3,7 @@ file lua/claudecode/config.lua 787fa0432d1d77d7 == lua/claudecode/cwd.lua file lua/claudecode/cwd.lua 35b7af3572d13018 == lua/claudecode/diff.lua -file lua/claudecode/diff.lua 2dafd67f00f45cde +file lua/claudecode/diff.lua 90add626421c266e block resolve_saved 46d5f3f732711fe1 block resolve_rejected c7a620816abff9a1 block open_blocking 92067df7f9d4d4d9 diff --git a/ARCHITECTURE.md b/ARCHITECTURE.md index 17d16a9bc..e2221fdac 100644 --- a/ARCHITECTURE.md +++ b/ARCHITECTURE.md @@ -175,12 +175,12 @@ return true, result At exit, Neovim fires `BufUnload` for each loaded buffer, and then runs the `VimLeavePre` autocmds. The plugin registers two of them, and they run in this order. Neovim runs the autocmds of one event in the order that they were registered. `diff.lua` registers its autocmd when it first loads, and `M.setup` loads `diff.lua` before it registers its own [lua/claudecode/init.lua:M.setup]. -First, the autocmd of `diff.lua`, in the `ClaudeCodeMCPDiff` group, answers every `openDiff` call that is still pending with `DIFF_REJECTED`, oldest first, while the server still runs. Then it closes every diff and clears its state. It runs whether or not the server runs [lua/claudecode/diff.lua:cleanup_at_exit] [lua/claudecode/diff.lua:M._reject_pending_diffs] [lua/claudecode/diff.lua:M._cleanup_all_active_diffs]: +First, the autocmd of `diff.lua`, in the `ClaudeCodeMCPDiff` group, rejects each diff whose status is `pending`, oldest first, while the server still runs. Then it closes every diff and clears its state. It runs whether or not the server runs [lua/claudecode/diff.lua:cleanup_at_exit] [lua/claudecode/diff.lua:M._reject_pending_diffs] [lua/claudecode/diff.lua:M._cleanup_all_active_diffs]: ```lua -- diff.lua, module level (shortened) local function cleanup_at_exit() - M._reject_pending_diffs() -- DIFF_REJECTED for each pending diff, oldest first + M._reject_pending_diffs() -- reject each diff that is pending, oldest first M._cleanup_all_active_diffs("shutdown") -- close each diff and clear its state end @@ -190,9 +190,9 @@ vim.api.nvim_create_autocmd("VimLeavePre", { }) ``` -The `BufUnload` loop that runs before both autocmds already rejects most diffs, because the `BufUnload` autocmd of a proposal buffer rejects its diff [lua/claudecode/diff.lua:register_diff_autocmds]. That loop stops when an autocmd deletes the buffer that it fired for, and SIGTERM and SIGHUP run `VimLeavePre` without the loop. `_reject_pending_diffs` answers the diffs that the loop did not reach. A diff that already has its answer gets no second one. +The `BufUnload` loop that runs before both autocmds already rejects most diffs, because the `BufUnload` autocmd of a proposal buffer rejects its diff [lua/claudecode/diff.lua:register_diff_autocmds]. That loop stops when an autocmd deletes the buffer that it fired for, and SIGTERM and SIGHUP run `VimLeavePre` without the loop. `_reject_pending_diffs` rejects the diffs that the loop did not reach, and it skips a diff whose status is not `pending`. A reject can remove its own diff (a new-file diff with `on_new_file_reject = "keep_empty"`), so a diff can be gone before the next one is rejected. -Second, the autocmd of `init.lua`, in the `ClaudeCodeShutdown` group, runs `stop` only when the server runs. Without a server, it only clears the @-mention queue [lua/claudecode/init.lua:M.setup] [lua/claudecode/init.lua:clear_mention_queue]. `stop` removes the lock file, stops the server, and clears the queue [lua/claudecode/init.lua:M.stop]. The server's own `stop` answers a call whose handler is still running with the error -32603, and the TCP server then closes each client with the close code 1001 [lua/claudecode/server/init.lua:M.stop] [lua/claudecode/server/tcp.lua:M.stop_server]: +Second, the autocmd of `init.lua`, in the `ClaudeCodeShutdown` group, runs `stop` only when the server runs. Without a server, it only clears the @-mention queue [lua/claudecode/init.lua:M.setup] [lua/claudecode/init.lua:clear_mention_queue]. `stop` removes the lock file, stops the server, and clears the queue [lua/claudecode/init.lua:M.stop]. The server's own `stop` sends a response for each request whose handler is still running, and the TCP server then closes each client with the close code 1001 [lua/claudecode/server/init.lua:M.stop] [lua/claudecode/server/tcp.lua:M.stop_server]: ```lua -- init.lua, M.setup (shortened) @@ -209,7 +209,7 @@ vim.api.nvim_create_autocmd("VimLeavePre", { }) ``` -SIGKILL, a crash, and a quit from inside an autocmd that is not `nested` run neither autocmd. Claude then sees the socket drop (close code 1006) with no answer for a pending diff, and the lock file stays. +SIGKILL, a crash, and a quit from inside an autocmd that is not `nested` run neither autocmd. What Claude receives for each kind of call, in each kind of exit, is in the root [README.md](./README.md), under Global Invariants. ## Module Structure @@ -292,6 +292,6 @@ vv netrw # Test with built-in netrw - **Debounced Updates**: 100 ms debounce on selection changes, except leaving visual mode, editing or consuming the held text, and moving the cursor away from a held selection, which report at once - **Localhost Only**: Server binds to 127.0.0.1 -- **Resource Cleanup**: At exit, two `VimLeavePre` autocmds run. The one in `diff.lua` runs first and answers each pending diff. The one in `init.lua` runs `stop` when the server runs, and `stop` stops the server and tries to remove the lock file [lua/claudecode/diff.lua:cleanup_at_exit] [lua/claudecode/init.lua:M.setup] [lua/claudecode/init.lua:M.stop]. A SIGKILL, a crash, or a quit from inside an autocmd that is not `nested` skips both and leaves the lock file +- **Resource Cleanup**: At exit, two `VimLeavePre` autocmds run. The one in `diff.lua` runs first and rejects each pending diff. The one in `init.lua` runs `stop` when the server runs, and `stop` stops the server and tries to remove the lock file [lua/claudecode/diff.lua:cleanup_at_exit] [lua/claudecode/init.lua:M.setup] [lua/claudecode/init.lua:M.stop]. A SIGKILL, a crash, or a quit from inside an autocmd that is not `nested` skips both and leaves the lock file - **In-memory State**: The plugin keeps, among other things, the latest selection [lua/claudecode/selection.lua:M.state], the proposed text of each open diff [lua/claudecode/diff.lua:M._register_diff_state], an input buffer for each connected client [lua/claudecode/server/client.lua:M.create_client], and the @-mentions that wait to go out [lua/claudecode/init.lua:mention_queue]. `lockfile.lua` reads `CLAUDE_CONFIG_DIR` once, when it loads [lua/claudecode/lockfile.lua:get_lock_dir] - **Async and Blocking I/O**: Sockets and timers use libuv callbacks. The TCP server accepts connections and reads through callbacks, writes with libuv's asynchronous `write`, and runs its ping timer from a libuv timer [lua/claudecode/server/tcp.lua:M._bind_and_listen] [lua/claudecode/server/tcp.lua:M._handle_new_connection] [lua/claudecode/server/client.lua:write] [lua/claudecode/server/tcp.lua:M.start_ping_timer]. The selection debounce and the @-mention queue use timers [lua/claudecode/selection.lua:M.debounce_update] [lua/claudecode/init.lua:start_timer]. File I/O is synchronous and blocks Neovim until it returns: `lockfile.lua` calls `fs_open`, `fs_write` and `fs_close` without callbacks and uses `os.rename` and `os.remove` [lua/claudecode/lockfile.lua:M.create] [lua/claudecode/lockfile.lua:M.remove], and `diff.lua` reads and writes files with `io.open` [lua/claudecode/diff.lua:read_file] [lua/claudecode/diff.lua:create_temp_file] diff --git a/README.md b/README.md index 7018098e2..3f0df0e03 100644 --- a/README.md +++ b/README.md @@ -1140,19 +1140,33 @@ The plugin runs the CLI through a [terminal provider](./lua/claudecode/terminal/ [lua/claudecode/server/init.lua:M.start], or when a new diff reuses its `tab_name` [lua/claudecode/diff.lua:M.open_diff_blocking]. Claude's `close_tab` and `closeAllDiffTabs` calls also reject a pending diff [lua/claudecode/diff.lua:M.close_diff_by_tab_name]. A diff that resolves before `openDiff` yields, for example from a - `ClaudeCodeDiffOpened` handler, is answered at once [lua/claudecode/diff.lua:M.open_diff_blocking]. When Neovim exits, - it fires `BufUnload` for each loaded buffer before `VimLeavePre`. The `BufUnload` autocmd of a proposal buffer rejects - its pending diff [lua/claudecode/diff.lua:register_diff_autocmds]. That loop stops as soon as an autocmd deletes the - buffer it fired for. The reject of a new-file diff in a tab it did not open, with `diff_opts.on_new_file_reject = -"keep_empty"` (the default), deletes its proposal buffer [lua/claudecode/diff.lua:M.\_resolve_diff_as_rejected] - [lua/claudecode/diff.lua:M.\_cleanup_diff_state], so the later diffs get no `BufUnload`. SIGTERM and SIGHUP run - `VimLeavePre` without that loop. SIGKILL, a crash, or a quit from inside an autocmd that is not `nested` runs no - `VimLeavePre`: Claude then gets no answer, its socket closes with 1006, and the lock file stays. In an exit that runs - `VimLeavePre`, the autocmd of `ClaudeCodeMCPDiff` rejects every diff that is still pending, oldest first, before it - removes any [lua/claudecode/diff.lua:cleanup_at_exit], so each pending diff whose `openDiff` setup has finished gets - one `DIFF_REJECTED`. A call whose setup has not finished when Neovim exits has no response sender yet. This happens when a - `nested = true` `ClaudeCodeDiffOpened` handler runs `:qa!`. The server stop answers that call with a -32603 error - [lua/claudecode/server/init.lua:M.stop]. + `ClaudeCodeDiffOpened` handler, is answered at once [lua/claudecode/diff.lua:M.open_diff_blocking]. +- Exit answers. This bullet is the one place that states what Claude gets when Neovim exits. The other docs describe + how the code runs the exit and link here. A call is waiting when its `openDiff` setup has finished and nobody has + answered its diff. Each fact below was probed with a real WebSocket client on Neovim 0.8.0 and 0.12.3. The facts + about `:qa!`, SIGTERM and a quit from a non-nested autocmd also hold on 0.9.5, 0.10.4 and 0.11.5. SIGKILL stands in + for a crash, which was not probed: + - An exit that runs `VimLeavePre` (`:qa!`, SIGTERM or SIGHUP) gives each waiting call one `DIFF_REJECTED` + [lua/claudecode/diff.lua:M._reject_pending_diffs]. A call that already has its answer, such as a `FILE_SAVED`, + gets no second one. + - A call that the exit cuts off during its `openDiff` setup gets one -32603 error from the server stop. It has no + response sender yet, so nothing else can answer it. This happens when a `nested = true` `ClaudeCodeDiffOpened` + handler runs `:qa!` during the setup. A server stop that is not an exit, for example `:ClaudeCodeStop` from such a + handler, gives the same error [lua/claudecode/server/init.lua:M.stop] + [lua/claudecode/server/init.lua:M.\_handle_request]. + - The answers go out before the server closes each client with close code 1001. `stop` also tries to remove the lock + file [lua/claudecode/init.lua:M.stop]. + - SIGKILL, a crash, and `:qa!` from inside an autocmd that is not `nested` run no `VimLeavePre`. Claude gets no + answer, its socket closes with 1006, and the lock file stays. + - In an exit that runs `VimLeavePre`, the autocmd of `ClaudeCodeMCPDiff` rejects every diff with the status + `pending` first and removes every diff after that [lua/claudecode/diff.lua:cleanup_at_exit]. A reject can remove + its own diff at once. With the default `on_new_file_reject = "keep_empty"`, a new-file diff in the `vertical` + layout that is not in its own tab is removed when it is rejected, so its `ClaudeCodeDiffClosed` handler runs while + a later diff is still `pending` [lua/claudecode/diff.lua:M.\_resolve_diff_as_rejected]. With `"close_window"`, + both diffs have their answers before the first `ClaudeCodeDiffClosed` fires (probed with one new-file and one + existing-file diff, for `:qa!` and SIGTERM). + - How the code runs the exit: [lua/claudecode/README.md](./lua/claudecode/README.md) and the Resource Cleanup + section of [ARCHITECTURE.md](./ARCHITECTURE.md). ### Global Side Effects diff --git a/lua/claudecode/README.md b/lua/claudecode/README.md index 09a760c31..1df3cecb6 100644 --- a/lua/claudecode/README.md +++ b/lua/claudecode/README.md @@ -182,7 +182,7 @@ Module map (direct files): - In the `vertical` and `horizontal` layouts the saved content is the lines of the proposal buffer, joined with line breaks, plus a final line break only when the buffer's `'eol'` is on. The diff sets `'eol'` from the proposal when it creates the buffer, so a proposal without a final line break is saved without one, and the user's edits keep that ending [diff.lua:M._setup_blocking_diff] [diff.lua:M._resolve_diff_as_saved] - A pending diff rejects at once when its proposal buffer is shown in no window, in the split and the unified layout: its last window closes (`:q`, `:close`, `c`, `:tabclose`), or that window switches to another buffer (`:edit`, `:buffer`, `:enew`, which only hide it). A copy of the proposal in another window or tab defers the reject until that copy leaves too. The switch path rejects on the next loop turn, and only when the diff is still the same one and still hidden. Both layouts keep the proposal buffer's `'bufhidden'` at `hide`: the buffer is modified, and `wipe` would make `:q` fail with E37 [diff.lua:M._register_reject_when_hidden] [diff_inline.lua:M.setup_inline_diff] - A diff that has its answer, by either one, clears `'modified'` on its proposal buffer. While the flag stayed set, `:wq` left the window open, and `:qa` stopped with E37 or E162 and moved a window onto the hidden buffer until `close_tab` deleted it [diff.lua:mark_proposal_unmodified] -- An exit that runs `VimLeavePre` answers each `openDiff` call that is still open once: `DIFF_REJECTED` for a diff that is pending, or a -32603 error for a call whose setup has not finished (below). A quit from inside an autocmd that is not `nested` runs no `VimLeavePre`, and neither do SIGKILL or a crash. Claude then gets no answer, its socket closes with 1006, and the lock file stays. Neovim fires `BufUnload` for each loaded buffer before `VimLeavePre`, and a proposal's `BufUnload` autocmd rejects its diff, but that loop stops as soon as an autocmd deletes the buffer it fired for. The reject of a new-file diff with `on_new_file_reject = "keep_empty"`, in a tab that the diff did not open, deletes its proposal buffer, and another plugin's buffer can delete itself in its own `BufUnload`. The diffs after either get no `BufUnload` (probed on Neovim 0.8.0, 0.9.5, 0.10.4, 0.11.5 and 0.12.3). SIGTERM and SIGHUP run `VimLeavePre` without that loop. The `VimLeavePre` autocmd therefore rejects every diff still `pending`, oldest first, before `_cleanup_all_active_diffs("shutdown")` removes any entry, so each pending diff whose setup has finished gets its `DIFF_REJECTED` in every exit that runs `VimLeavePre`. A diff that already has its answer gets no second one. A call whose setup has not finished has no response sender yet. When a `nested = true` `ClaudeCodeDiffOpened` handler runs `:qa!` during that setup, `server.stop` answers the call with -32603 [diff.lua:M._reject_pending_diffs] [diff.lua:cleanup_at_exit] [server/init.lua:M.stop] +- At exit, Neovim fires `BufUnload` for each loaded buffer before `VimLeavePre`, and a proposal's `BufUnload` autocmd rejects its diff, but that loop stops as soon as an autocmd deletes the buffer it fired for. The reject of a new-file diff with `on_new_file_reject = "keep_empty"`, in a tab that the diff did not open, deletes its proposal buffer, and another plugin's buffer can delete itself in its own `BufUnload`. The diffs after either get no `BufUnload` (probed on Neovim 0.8.0, 0.9.5, 0.10.4, 0.11.5 and 0.12.3). SIGTERM and SIGHUP run `VimLeavePre` without that loop. A quit from inside an autocmd that is not `nested`, SIGKILL and a crash run no `VimLeavePre`. The `VimLeavePre` autocmd of `ClaudeCodeMCPDiff` therefore runs `_reject_pending_diffs` and then `_cleanup_all_active_diffs("shutdown")`. `_reject_pending_diffs` rejects each diff with the status `pending`, oldest first, each under `pcall`, and skips a diff with another status. A reject can remove its own diff (the `keep_empty` case above), so a diff can be gone before the next one is rejected. The reject of a diff whose `openDiff` setup has not finished stays in `setup_result`, because the call has no response sender yet, so the server's `stop` sends that call's response. The README at the repository root, under Global Invariants, lists what Claude receives for each kind of call [diff.lua:M._reject_pending_diffs] [diff.lua:cleanup_at_exit] [diff.lua:M.open_diff_blocking] [server/init.lua:M.stop] - The cleanup of a diff closes the proposal window, and the window that the split layout opened for the file's old text, only while each still shows its buffer. A window that the user switched to another buffer stays with that buffer. `:edit` turns diff mode off by itself, except when the new buffer is in diff mode in another window, as the file's own buffer is. So the split layout's cleanup ends diff mode only in a kept window that shows the file's own buffer, and leaves a diff mode in a window that shows another buffer, which is the user's. One limit comes from Neovim: the default `closeoff` in `'diffopt'` turns diff mode off in the last window that has it when another diff window closes. That ends a user's diff mode in a kept original-side window when the proposal window closes, and in a kept proposal window when the cleanup closes the window that the plugin opened for the file. A user's diff mode over two or more windows stays. The unified layout needs no cleanup there: it sets `'signcolumn'` to `yes` with local scope, for the proposal buffer only, so the window shows the sign column it had before the diff once the user switches it, and keeps a value the user sets afterwards. Two cases still close the window: a diff in its own tab (`open_in_new_tab`, or a tab that already has a diff) closes the whole tab, and the fallback window that the plugin opens in a tab that holds only the Claude terminal always closes [diff.lua:M._close_proposal_window] [diff.lua:M._cleanup_diff_state] [diff_inline.lua:M.cleanup_inline_diff] [diff_inline.lua:M.setup_inline_diff] - Rejecting a new-file diff that is not in its own tab cleans it up at once when `diff_opts.on_new_file_reject` is `"keep_empty"` (the default), before `close_tab`. A `unified` diff gets no such cleanup [diff.lua:M._resolve_diff_as_rejected] [diff_inline.lua:M.resolve_inline_as_rejected] - `BufDelete`, `BufUnload`, and `BufWipeout` on the proposal buffer reject a pending diff. `WinClosed` rejects it only when no other window in any tab shows the proposal buffer. The original file's buffer is not watched. The unified layout registers the same autocmds [diff.lua:register_diff_autocmds] [diff_inline.lua:M.setup_inline_diff] diff --git a/lua/claudecode/diff.lua b/lua/claudecode/diff.lua index a5ab4c86d..d51b9d7e6 100644 --- a/lua/claudecode/diff.lua +++ b/lua/claudecode/diff.lua @@ -2003,8 +2003,11 @@ function M.open_diff_blocking(old_file_path, new_file_path, new_file_contents, t return user_action_result end ----Answer every diff that is still pending with DIFF_REJECTED, oldest first. A diff that already has its answer is ----skipped, so each openDiff gets one. One diff that fails to reject must not leave the others waiting. +---Resolve each diff whose status is `pending` as rejected, oldest first, each under `pcall`, so one diff that fails to +---reject cannot leave the others waiting. A diff with another status is skipped. A reject can remove its own diff +---(`keep_empty`), so a diff can be gone before the next one is rejected. The reject of a diff whose openDiff setup is +---still running stays in `setup_result` (see `open_diff_blocking`) and sends nothing: the call has no response sender +---yet, and the server's `stop` sends its response. function M._reject_pending_diffs() local pending = {} for tab_name, diff_data in pairs(active_diffs) do @@ -2028,7 +2031,8 @@ end ---The reject of a new-file diff with `on_new_file_reject = "keep_empty"`, in a tab that the diff did not open, deletes ---its proposal buffer, and another plugin's buffer can delete itself in its own BufUnload. The diffs behind that ---buffer then reach this point pending. SIGTERM and SIGHUP run VimLeavePre without the loop, so every diff reaches it ----pending. SIGKILL or a crash runs no VimLeavePre. The pending diffs are answered before the cleanup removes them. +---pending. SIGKILL, a crash, or a quit from inside an autocmd that is not `nested` runs no VimLeavePre. The reject +---pass runs first. The cleanup pass of every diff runs after it. local function cleanup_at_exit() M._reject_pending_diffs() M._cleanup_all_active_diffs("shutdown") diff --git a/lua/claudecode/server/README.md b/lua/claudecode/server/README.md index 37e4bc1e5..007139e75 100644 --- a/lua/claudecode/server/README.md +++ b/lua/claudecode/server/README.md @@ -96,7 +96,7 @@ Layers, top to bottom: - A message with a truthy `id` is a request. Any other message is a notification. A valid notification never gets a response. The -32700 and -32600 checks run before the `id` check, so a malformed notification gets an error reply [server/init.lua:M._handle_message] - Notification handlers run in `pcall`. Their errors and return values are discarded, and an unknown notification is ignored without a reply [server/init.lua:M._handle_notification] - A deferred response is keyed by `tostring(coroutine)` in `_G.claude_deferred_responses`. It replies to `result.client` with the original request id. A result with `content` is sent as `result`, one with `error` is sent as `error`, and a result with neither becomes -32603 [server/init.lua:M._setup_deferred_response] -- `_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" (`data`: "The server stopped before the call finished") before it stops the TCP server, and `_handle_request` sends nothing for that request afterwards. A blocking tool has no response sender before its handler yields, so this is the only answer to a call when Neovim exits inside the handler, for example from a `nested = true` `ClaudeCodeDiffOpened` handler that runs `:qa!` [server/init.lua:M._handle_request] [server/init.lua:M.stop] +- `_handle_request` records each request in `M.state.in_flight` while its handler runs. `M.stop` sends -32603 "Internal error" (`data`: "The server stopped before the call finished") for every request still recorded, before it stops the TCP server, and `_handle_request` sends nothing for that request afterwards. A blocking tool has no response sender before its handler yields, so nothing else can answer a request whose handler is still running [server/init.lua:M._handle_request] [server/init.lua:M.stop]. The README at the repository root, under Global Invariants, lists what Claude receives at an exit. - A ping timer tick pings every connected client and never closes one for a missing answer. `client.ping_unanswered_since` holds the loop time of the oldest ping sent since bytes were last read from the client. The read callback clears it on any bytes, not only a pong: Claude shows it is alive by writing anything, and a pong that waits behind more unread data than the socket holds is not in the first poll after a block. The tick calls `vim.loop.update_time()` before it reads the clock, because libuv caches the loop time when a pass of the main loop starts and runs the timers of the pass with it, and a callback that ran before the tick in the pass can have blocked since, or the machine can have slept in it. A stale time would hide a wake and would stamp a ping older than it is. Each tick that pinged a client arms `after_next_poll` (below), and its callback runs `judge_silence` after the next poll. libuv runs the check handle right after the poll phase that reads the sockets, so every byte that reached a socket before the loop time of the check was read. The check closes each client that the tick pinged when its oldest ping is `interval * 2.5` old, but only if its loop time is at most `interval * 1.5` after the tick. After a longer block Neovim and Claude can have slept together (a suspended machine), and Claude has had no time to answer. A tick cannot decide this, because libuv can run a tick before it reads the pong that Claude sent: after a block in a check callback or an I/O read callback, and, with libuv 1.44 (bundled in Neovim 0.8), after a block in any callback. libuv 1.44 also runs the timers again at the end of a pass, so two ticks can run with no read between them. `vim.schedule` cannot decide it either, because Neovim runs its callbacks between two passes, so it can run one before the read. Drop bound: a block of the main loop, of any length and in any callback, does not close a Claude that writes something within `interval * 2.5` (75 s by default) of each ping. A tick never closes a client, a check judges only after a poll within `interval * 1.5` of its tick, and any bytes clear the stamp. A client that writes data but never answers a ping is not closed by this rule: it is alive, and the plugin does not judge what it writes (the stall rule below closes it when it stops reading, but only once the kernel's socket buffers are full and libuv's write queue backs up, because a queue that is empty counts as progress). A block longer than `interval * 1.5` restarts the count: the next tick sees the gap, takes it for a wake and clears every stamp. So a block delays the close of a silent Claude, and a run of such blocks can postpone it. Without a long block, a Claude that sends nothing is closed at the first read of its socket after the third tick that follows its first unanswered ping, which is 90 s by default after that ping. The Close frame of a dropped client carries 1011 "Keepalive timeout" (RFC 6455 reserves 1006, so no Close frame carries it), `on_disconnect` gets 1006 "Connection timeout", and the check logs `Closed client_: no reply for s` at info level. A peer that stopped reading does not get that Close frame: once its receive window is full, every write to it (replies, pings, and the Close frame) waits in libuv's queue, and closing the socket drops the queue [server/tcp.lua:M.start_ping_timer] [server/tcp.lua:judge_silence] [server/tcp.lua:drop_client] - A Claude that answers each ping but never reads would keep that queue, which is Neovim's memory, without limit, so the server also closes a connected client that answers and whose write queue made no progress at two ticks in a row. Each tick calls `is_client_reading` for each connected client, which calls `sample_write_progress`: it compares `queued_bytes - get_write_queue_size()` with its value at the last progress, and an empty queue counts as progress. A tick that finds progress clears `client.write_stalled`. The first tick that finds none sets it. The second asks for a confirmation, and a confirmation that still finds no progress drops the client: the Close frame carries 1011 "Not reading", `on_disconnect` gets 1006 "Write queue stalled", and the server logs `Closed client_: not reading for s, KB queued` at info level. The confirmation runs right after the next poll. libuv reports that a socket is writable again only in a poll phase, so a tick that Neovim runs after a main-loop block inside an I/O callback can find no progress although Claude read during the block, whether the tick is late or on time. `after_next_poll` arms a check handle, which libuv runs right after each poll phase in every version, and an idle handle, which keeps that poll from waiting for a timer or a socket. The check callback closes both handles, removes them from `server.pending_polls`, and judges the queue once more: progress clears `write_stalled`, and no progress drops the client. A client that left or started to leave in the meantime, because the poll read its EOF, a read error, its Close frame or a protocol error, is skipped without a log line. `stop_server` closes the handles of a confirmation that has not run, which `server.pending_polls` lists. If libuv cannot create a handle, the tick leaves the client for the next tick. The look is a sample at one moment, not proof. libuv takes its snapshot of the ready sockets when a poll starts, so a callback that blocks inside that poll, after the snapshot, hides what Claude reads during the block from the check callback that follows. A client that was stalled for the whole window and resumes during such a block is dropped. That is a deadline race, as with any timeout: the client was stalled when the window ended, and what it read afterwards is not reported yet. Verifier probes closed a reader that resumed 0.1 s into a 0.5 s to 1.5 s block inside the confirming poll, on Neovim 0.12.3 and 0.8.0, at 3 s, 5 s and 30 s intervals. A block after the tick and before that poll, or in a scheduled callback, keeps the reader. The bound is two to three ticks after the last progress, which is 60 s to 90 s at the default interval, plus the delay of late ticks, each of which comes at most `interval * 1.5` after the tick before it. A tick that comes more than `interval * 1.5` after the tick before it counts as a wake from sleep. A wake tick starts a fresh stall window: it clears the count and judges nothing, so it delays the drop by one tick when the count was clear, and by two ticks when the wake tick takes the place of the confirming tick. The next two ticks always count, wake ticks or not, because wake ticks that come again and again are a loop that stays blocked, not sleeps. So whatever the pattern of late ticks, a stalled client is closed at the second tick after a fresh window started. A wake tick three or more ticks after the last fresh start starts a new window. The bound is never the size of the queue, so a Claude that reads slowly, or is sent one message of tens of MB, stays connected while libuv writes any of it. libuv writes more only after the kernel reports the socket writable again, which took 1.2 to 1.5 MB of reading on a Linux loopback socket with default buffers (measured), so a Claude that reads less than about 1.4 MB per window counts as stalled while a backlog exists, and one that reads just faster than that still lets the queue grow as long as Neovim queues more than it reads. `write` queues a frame of more than 1 MiB in pieces of 1 MiB, because libuv on Windows lowers the queue only when a whole write request is done, so a large frame shows its progress per piece there too, and the callback of the last piece reports the frame. [server/tcp.lua:M.start_ping_timer] [server/tcp.lua:judge_stall] [server/tcp.lua:after_next_poll] [server/tcp.lua:M.stop_server] [server/client.lua:M.is_client_reading] [server/client.lua:M.sample_write_progress] [server/client.lua:write] - On one tick a client counts as answered when no ping of it waited for `interval * 2`. Only a client that answered is confirmed for a stall. A client that did not answer is dropped by the keepalive check above, whatever its write queue shows. A client that answers while its queue is stalled is not saved: its bytes clear the stamp, the next tick finds it answered, and then asks for the confirmation. Every connected client is sampled on every tick except one that starts a fresh stall window, answering or not, so "two ticks in a row" means consecutive ticks. A tick gap above `interval * 1.5` counts as a wake from sleep and clears `ping_unanswered_since` of each connected client. A wake tick that starts a fresh stall window also resets `last_drain`, clears `write_stalled`, and does not sample the queue. `last_drain` feeds only the `not reading for s` of the log line, so the reset keeps the sleep out of it. A wake tick within two ticks after the start of a fresh window clears the stamps only, and counts like any tick. A client that is not connected is not pinged, so a main loop that blocks during the handshake does not count against it [server/tcp.lua:M.start_ping_timer] [server/client.lua:M.process_data] diff --git a/tests/integration/diff_exit_reject_spec.lua b/tests/integration/diff_exit_reject_spec.lua index 60bd45008..42fe1515d 100644 --- a/tests/integration/diff_exit_reject_spec.lua +++ b/tests/integration/diff_exit_reject_spec.lua @@ -1,10 +1,9 @@ --- Real-Neovim check that :qa! (or SIGTERM) answers every openDiff call that is still open, exactly once: a pending diff --- gets DIFF_REJECTED, and a call whose setup a ClaudeCodeDiffOpened handler cut off by quitting gets a -32603 error. +-- Real-Neovim check of the exit answers that README.md lists under Global Invariants, in "Exit answers". -- Neovim fires BufUnload for each loaded buffer at exit, and the proposal buffers' autocmds reject their diffs. That -- loop stops once an autocmd deletes the buffer it fired for: the reject of a new-file diff with the default -- `on_new_file_reject = "keep_empty"`, in a tab the diff did not open, deletes its proposal buffer, and another -- plugin's buffer can delete itself. SIGTERM runs VimLeavePre without the loop. The diffs behind it get no BufUnload, --- so the VimLeavePre autocmd of diff.lua must answer them. A child Neovim does the exit, because it ends the process. +-- so the VimLeavePre autocmd of diff.lua must reject them. A child Neovim does the exit, because it ends the process. -- Mock vim cannot run an exit, so under busted this is pending. if vim == nil or vim.fn.has("nvim") ~= 1 then describe("openDiff at exit", function() diff --git a/tests/unit/server/in_flight_request_spec.lua b/tests/unit/server/in_flight_request_spec.lua index 2a230c582..ab303cfb1 100644 --- a/tests/unit/server/in_flight_request_spec.lua +++ b/tests/unit/server/in_flight_request_spec.lua @@ -1,6 +1,7 @@ -- A request has no answer until its handler returns, and a blocking tool has no response sender before that. When --- Neovim exits inside a handler, the handler never returns, so `server.stop` must answer the request: Claude gets one --- answer for every call that reached the server, and `_handle_request` sends nothing after a stop that answered it. +-- Neovim exits inside a handler, the handler never returns, so `server.stop` must answer the request, and +-- `_handle_request` sends nothing after a stop that answered it. README.md lists what Claude receives, under Global +-- Invariants, in "Exit answers". require("tests.busted_setup") describe("a request whose handler is still running when the server stops", function() From ddb52530cb079b5ebaf7baeedad48dd143e179d3 Mon Sep 17 00:00:00 2001 From: Thomas Kosiewski Date: Thu, 12 Nov 2026 09:41:28 +0000 Subject: [PATCH 07/11] docs(verify): say that a stop answers the openDiff calls that wait for the user A call still in setup gets a -32603 error from the stop. The root README lists it under Exit answers. --- .agents/skills/verify-claudecode-nvim/features/server.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.agents/skills/verify-claudecode-nvim/features/server.md b/.agents/skills/verify-claudecode-nvim/features/server.md index 5341141e1..347370fdf 100644 --- a/.agents/skills/verify-claudecode-nvim/features/server.md +++ b/.agents/skills/verify-claudecode-nvim/features/server.md @@ -6,7 +6,7 @@ claudecode.nvim runs a WebSocket server on `127.0.0.1` and writes a lock file, ` - `server-status` `:ClaudeCodeStatus` prints `Claude Code integration is running on port ` or `Claude Code integration is not running`. - `server-lock` the lock file is mode `0600` in a `0700` folder and holds nvim's `pid`, `ideName` `Neovim`, `transport` `ws`, the cwd in `workspaceFolders`, and a 32-character hex `authToken`. -- `server-stop` `:ClaudeCodeStop` deletes `.lock`, answers each pending openDiff with `DIFF_REJECTED`, closes each client with code 1001, and prints `Claude Code integration stopped`. +- `server-stop` `:ClaudeCodeStop` deletes `.lock`, answers each openDiff that waits for the user with `DIFF_REJECTED`, closes each client with code 1001, and prints `Claude Code integration stopped`. - `server-start` `:ClaudeCodeStart` listens on a new random port, writes a new lock file with a new token, and prints `Claude Code integration started on port `. - `server-twice` Start while running warns `Claude Code integration is already running on port `. Stop while stopped warns `Claude Code integration is not running`. - `server-exit` quitting Neovim deletes the lock file. From 68fa74d6d3591300671bad56fc5596df21f03414 Mon Sep 17 00:00:00 2001 From: Thomas Kosiewski Date: Thu, 12 Nov 2026 10:29:06 +0000 Subject: [PATCH 08/11] refactor(diff): make the exit reject pass a local function AGENTS.md (Code Shape, #552) puts a function on a module table only when another module, the documented API or Neovim calls it. reject_pending_diffs has one caller, the module-local cleanup_at_exit, and no test calls it, so it is now a local function. The behavior is identical. The names change in check.sh (three regexes), SKILL.md, the model comments, the READMEs and ARCHITECTURE.md. covered.lock records the new hashes of the renamed blocks. --- .agents/skills/verify-diff-lifecycle/SKILL.md | 6 +++--- .agents/skills/verify-diff-lifecycle/check.sh | 8 ++++---- .agents/skills/verify-diff-lifecycle/covered.lock | 10 +++++----- .../verify-diff-lifecycle/model/DiffLifecycle.tla | 4 ++-- ARCHITECTURE.md | 6 +++--- README.md | 2 +- lua/claudecode/README.md | 4 ++-- lua/claudecode/diff.lua | 4 ++-- 8 files changed, 22 insertions(+), 22 deletions(-) diff --git a/.agents/skills/verify-diff-lifecycle/SKILL.md b/.agents/skills/verify-diff-lifecycle/SKILL.md index a7a65c42b..54dcb1284 100644 --- a/.agents/skills/verify-diff-lifecycle/SKILL.md +++ b/.agents/skills/verify-diff-lifecycle/SKILL.md @@ -55,7 +55,7 @@ The record has one group of lines per file: a `== ` header, the file's has Covered blocks: -- In `diff.lua`: `_resolve_diff_as_saved`, `_resolve_diff_as_rejected`, `open_diff_blocking`, `_setup_blocking_diff`, `close_diff_by_tab_name`, `close_pending_diffs`, `close_diffs_for_client`, `close_active_diffs`, `_cleanup_all_active_diffs`, `_reject_pending_diffs`, `cleanup_at_exit` (the `VimLeavePre` callback), `_register_diff_state`, `_cleanup_diff_state`, `register_diff_autocmds` (the `:w` and buffer-delete paths), `_write_proposal` (what a write of the proposal does), `_register_reject_when_hidden` (the `:q` and buffer-switch paths, shared by both layouts), `fire_diff_event`, `accept_current_diff`, `deny_current_diff`. +- In `diff.lua`: `_resolve_diff_as_saved`, `_resolve_diff_as_rejected`, `open_diff_blocking`, `_setup_blocking_diff`, `close_diff_by_tab_name`, `close_pending_diffs`, `close_diffs_for_client`, `close_active_diffs`, `_cleanup_all_active_diffs`, `reject_pending_diffs`, `cleanup_at_exit` (the `VimLeavePre` callback), `_register_diff_state`, `_cleanup_diff_state`, `register_diff_autocmds` (the `:w` and buffer-delete paths), `_write_proposal` (what a write of the proposal does), `_register_reject_when_hidden` (the `:q` and buffer-switch paths, shared by both layouts), `fire_diff_event`, `accept_current_diff`, `deny_current_diff`. - In `diff_inline.lua`: `setup_inline_diff`, `resolve_inline_as_saved`, `resolve_inline_as_rejected`. - In `server/init.lua`: `M.stop`, `on_disconnect`, `M._handle_request`, `M._setup_deferred_response`. - In `tools/init.lua` and `tools/open_diff.lua`: `M.handle_invoke`, and the openDiff `handler`. @@ -77,7 +77,7 @@ A changed file with no changed block or call site still needs a look. A wrapper | `CloseTabRejectsPending` | `close_diff_by_tab_name` rejects a pending diff before cleanup | | `CloseEachByName` | `close_active_diffs` closes each match through `close_diff_by_tab_name` | | `StopRejectsPending` | `M.stop` calls `close_pending_diffs`, then `stop_server`, then clears the senders | -| `ExitRejectsPending` | the `VimLeavePre` callback `cleanup_at_exit` calls `_reject_pending_diffs` before `_cleanup_all_active_diffs`, so the final cleanup pass finds no pending diff | +| `ExitRejectsPending` | the `VimLeavePre` callback `cleanup_at_exit` calls `reject_pending_diffs` before `_cleanup_all_active_diffs`, so the final cleanup pass finds no pending diff | | `ExitAnswersInFlight` | `M.stop` answers each request in `M.state.in_flight`, and `_handle_request` sends nothing after a stop that answered it | | `StopPendingOnly`, `DisconnectPendingOnly` | the `status == "pending"` filters in `close_pending_diffs` and `close_diffs_for_client` | | `DisconnectRejects` | `on_disconnect` schedules `close_diffs_for_client` | @@ -124,7 +124,7 @@ Then decide which kind of failure it is: - The invariants run with 3 openDiff calls, 2 tab names and 2 clients, about 93,000 states. `DisconnectedCallsAnswered` runs with 2 calls, 2 tabs and 2 clients. It assumes weak fairness only on setup and on the scheduled disconnect cleanup. The user is never assumed to act, with one exception: the disconnect cleanup waits until Neovim allows window changes (`utils.when_windows_allowed`), so a user in the command-line window is assumed to leave it, and textlock is assumed to end. - In every run, a user `ClaudeCodeDiffOpened` handler (README "Diff Lifecycle Events") can accept or deny the diff during setup: `:ClaudeCodeDiffAccept`, `:ClaudeCodeDiffDeny`, or a `nested = true` autocmd that closes the proposal window. A default handler that closes the window resolves nothing, because WinClosed does not fire inside a non-nested autocmd. Other handler actions during setup are not modeled. -- `DiffCleanupAtExit` is the `VimLeavePre` callback of `diff.lua`: `_reject_pending_diffs`, then `_cleanup_all_active_diffs("shutdown")`. It models neither a `ClaudeCodeDiffClosed` handler (the reject pass has none unless a `keep_empty` reject runs a cleanup) nor a saved diff lost at exit: Neovim quits right after the step, and its buffers go with it. The model represents `DiffCleanupAtExit` and `ServerStops` as separate atomic actions and explores both orders, because both are `VimLeavePre` autocmds. Neovim's own exit `BufUnload` loop, which rejects before `VimLeavePre`, is the `UserRejects` action. +- `DiffCleanupAtExit` is the `VimLeavePre` callback of `diff.lua`: `reject_pending_diffs`, then `_cleanup_all_active_diffs("shutdown")`. It models neither a `ClaudeCodeDiffClosed` handler (the reject pass has none unless a `keep_empty` reject runs a cleanup) nor a saved diff lost at exit: Neovim quits right after the step, and its buffers go with it. The model represents `DiffCleanupAtExit` and `ServerStops` as separate atomic actions and explores both orders, because both are `VimLeavePre` autocmds. Neovim's own exit `BufUnload` loop, which rejects before `VimLeavePre`, is the `UserRejects` action. - `ExitInSetup` is an exit inside a `nested = true` `ClaudeCodeDiffOpened` handler, while one call is in setup, so it needs `OpenedEventInSetup`. The call has no sender and its setup never returns, so only the server stop can answer it (`ExitAnswersInFlight`). The action runs both `VimLeavePre` autocmds in one order, diff cleanup first, then the server stop. The other order gives the same result, because each leaves the other nothing to do. A handler that runs `:ClaudeCodeStop` instead of an exit is not modeled: the handler returns after the stop, and `tests/unit/server/in_flight_request_spec.lua` checks that `_handle_request` then sends nothing. What Claude receives at an exit is in the root `README.md`, under Global Invariants. - `SenderCleared` alone is caught by no rule, because the pending guards also stop a second answer. `--mutants` shows that `AtMostOneAnswer` breaks when both `SenderCleared` and `GuardRejected` are removed. - `SavedDiffKept` knows which client owns each diff, and, while `:ClaudeCodeCloseAllDiffs` runs, which files or file buffers hold the accepted text. The model has no file or buffer contents: the command's first pass leaves the accepted text in any subset of the saved diffs' files or buffers, because the code skips a diff whose file buffer already has unsaved changes, or whose buffer it cannot change. The plugin never writes the file. That the first pass leaves the file on disk and the user's unsaved changes alone is checked by the unit tests, not here. A saved diff counts as lost when the user, a disconnect cleanup or a server stop tears it down, or when a client that does not own it does so while the owner is disconnected. A close by another client while the owner is connected counts as safe, because the owner got `FILE_SAVED` and can still write the file. The code is stricter than the rule: it keeps another client's saved diff in both cases. Ownership compares client ids, so a new connection must never get an old id. With `ClientIdsUnique` off, `ClientIdReused` lets a new connection take the id of a disconnected client. diff --git a/.agents/skills/verify-diff-lifecycle/check.sh b/.agents/skills/verify-diff-lifecycle/check.sh index 5bc39b276..bb8d22482 100755 --- a/.agents/skills/verify-diff-lifecycle/check.sh +++ b/.agents/skills/verify-diff-lifecycle/check.sh @@ -50,7 +50,7 @@ covered_block setup_deferred "$SERVER" '^function M\._setup_deferred_response\(' covered_block user_close_all "$MAIN" 'nvim_create_user_command\("ClaudeCodeCloseAllDiffs"' covered_block close_active "$DIFF" '^local function close_active_diffs\(' covered_block cleanup_all "$DIFF" '^function M\._cleanup_all_active_diffs\(' -covered_block reject_pending "$DIFF" '^function M\._reject_pending_diffs\(' +covered_block reject_pending "$DIFF" '^local function reject_pending_diffs\(' covered_block exit_cleanup "$DIFF" '^local function cleanup_at_exit\(' covered_block inline_saved "$INLINE" '^function M\.resolve_inline_as_saved\(' covered_block inline_rejected "$INLINE" '^function M\.resolve_inline_as_rejected\(' @@ -139,13 +139,13 @@ fact[ClientIdsUnique]=$(as_bool client_ids_unique) fact[StopRejectsPending]=$(as_bool in_order 'diff\.close_pending_diffs\(' 'tcp_server\.stop_server\(' \ '_G\.claude_deferred_responses = \{\}' <<<"$server_stop") fact[StopPendingOnly]=$(as_bool grep -qE '^ *return diff_data\.status == "pending"$' <<<"$close_pending") -# The VimLeavePre callback runs _reject_pending_diffs before _cleanup_all_active_diffs, so a diff that no BufUnload +# The VimLeavePre callback runs reject_pending_diffs before _cleanup_all_active_diffs, so a diff that no BufUnload # rejected is rejected before the final cleanup pass. FACTS+=(ExitRejectsPending) exit_rejects_pending() { - in_order '^function M\._reject_pending_diffs\(\)$' 'diff_data\.status == "pending"' \ + in_order '^local function reject_pending_diffs\(\)$' 'diff_data\.status == "pending"' \ 'pcall\(M\._resolve_diff_as_rejected, entry\.tab_name\)' <<<"$reject_pending" && - in_order '^local function cleanup_at_exit\(\)$' '^ M\._reject_pending_diffs\(\)$' \ + in_order '^local function cleanup_at_exit\(\)$' '^ reject_pending_diffs\(\)$' \ '^ M\._cleanup_all_active_diffs\("shutdown"\)$' <<<"$exit_cleanup" && in_order '^vim\.api\.nvim_create_autocmd\("VimLeavePre", \{$' '^ callback = cleanup_at_exit,$' <"$DIFF" } diff --git a/.agents/skills/verify-diff-lifecycle/covered.lock b/.agents/skills/verify-diff-lifecycle/covered.lock index 4beb96701..bb0d4df36 100644 --- a/.agents/skills/verify-diff-lifecycle/covered.lock +++ b/.agents/skills/verify-diff-lifecycle/covered.lock @@ -3,7 +3,7 @@ file lua/claudecode/config.lua 3afb1bdeff27bfb6 == lua/claudecode/cwd.lua file lua/claudecode/cwd.lua 35b7af3572d13018 == lua/claudecode/diff.lua -file lua/claudecode/diff.lua dd40f39fbda748ee +file lua/claudecode/diff.lua 94c8ed00f46ea9fa block resolve_saved 4d34b0c7e34d561f block resolve_rejected c7a620816abff9a1 block open_blocking 92067df7f9d4d4d9 @@ -20,8 +20,8 @@ block put_in_buffer c5dedfca8261379b block held_for_other 3d1e2911d5310cc2 block close_active 446fbed632748300 block cleanup_all 43e7b595cf106229 -block reject_pending b00a72037446e3bf -block exit_cleanup 5e7246c612ddb96a +block reject_pending 4dc1ccb4a92cc822 +block exit_cleanup 46793014076b4e79 block accept_current 7577efa33506569e block fire_event ef778bb39419d91a block register_state 8cf78b6711a9ecc6 @@ -64,8 +64,8 @@ site lua/claudecode/diff.lua: for _, diff_data in pairs(active_diffs) do @ funct site lua/claudecode/diff.lua: for _, diff_data in pairs(active_diffs) do @ function M.close_saved_diffs(reason) site lua/claudecode/diff.lua: for _, other in pairs(active_diffs) do @ local function save_window_sizes(tab) site lua/claudecode/diff.lua: for tab_name, _ in pairs(active_diffs) do @ function M._cleanup_all_active_diffs(reason) -site lua/claudecode/diff.lua: for tab_name, diff_data in pairs(active_diffs) do @ function M._reject_pending_diffs() site lua/claudecode/diff.lua: for tab_name, diff_data in pairs(active_diffs) do @ local function close_active_diffs(filter_fn, reason, client_id) +site lua/claudecode/diff.lua: for tab_name, diff_data in pairs(active_diffs) do @ local function reject_pending_diffs() site lua/claudecode/diff.lua: function M._cleanup_all_active_diffs(reason) @ function M._cleanup_all_active_diffs(reason) site lua/claudecode/diff.lua: function M._cleanup_diff_state(tab_name, reason) @ function M._cleanup_diff_state(tab_name, reason) site lua/claudecode/diff.lua: function M._get_active_diffs() @ function M._get_active_diffs() @@ -99,7 +99,7 @@ site lua/claudecode/diff.lua: local diff_data = active_diffs[tab_name] @ local f site lua/claudecode/diff.lua: local entry = active_diffs[tab_name] @ function M._register_reject_when_hidden(tab_name, new_buffer, autocmd_ids) site lua/claudecode/diff.lua: local existing_diff = active_diffs[tab_name] @ function M.open_diff_blocking(old_file_path, new_file_path, new_file_contents, tab_name, client_id) site lua/claudecode/diff.lua: local function close_active_diffs(filter_fn, reason, client_id) @ local function close_active_diffs(filter_fn, reason, client_id) -site lua/claudecode/diff.lua: local ok, err = pcall(M._resolve_diff_as_rejected, entry.tab_name) @ function M._reject_pending_diffs() +site lua/claudecode/diff.lua: local ok, err = pcall(M._resolve_diff_as_rejected, entry.tab_name) @ local function reject_pending_diffs() site lua/claudecode/diff.lua: local state = active_diffs[tab_name] @ function M._setup_blocking_diff(params, resolution_callback) site lua/claudecode/diff.lua: resolution_callback = resolution_callback, @ function M._setup_blocking_diff(params, resolution_callback) site lua/claudecode/diff.lua: return active_diffs @ function M._get_active_diffs() diff --git a/.agents/skills/verify-diff-lifecycle/model/DiffLifecycle.tla b/.agents/skills/verify-diff-lifecycle/model/DiffLifecycle.tla index 96fee8bdd..d379def4f 100644 --- a/.agents/skills/verify-diff-lifecycle/model/DiffLifecycle.tla +++ b/.agents/skills/verify-diff-lifecycle/model/DiffLifecycle.tla @@ -343,7 +343,7 @@ ServerStops == SetW([v EXCEPT !.sender = [q \in Reqs |-> FALSE]]) /\ UNCHANGED <> -(* diff.lua _reject_pending_diffs: _resolve_diff_as_rejected for each diff that is still pending. *) +(* diff.lua reject_pending_diffs: _resolve_diff_as_rejected for each diff that is still pending. *) (* The order does not matter: each rejection touches only its own diff and its own call. *) RECURSIVE RejectEach(_, _) RejectEach(w, S) == @@ -352,7 +352,7 @@ RejectEach(w, S) == IN RejectEach(IF w.active[t] # NoDiff /\ w.active[t].status = "pending" THEN Resolve(w, t, "rejected") ELSE w, S \ {t}) -(* diff.lua VimLeavePre autocmd: _reject_pending_diffs, then _cleanup_all_active_diffs("shutdown"), which *) +(* diff.lua VimLeavePre autocmd: reject_pending_diffs, then _cleanup_all_active_diffs("shutdown"), which *) (* removes every entry in any status. Neovim fires BufUnload for each loaded buffer before VimLeavePre, and *) (* the proposal buffers' autocmds reject their diffs there (the UserRejects action). That loop stops when an *) (* autocmd deletes the buffer it fired for, so a pending diff can still reach this step. Neovim quits right *) diff --git a/ARCHITECTURE.md b/ARCHITECTURE.md index e2221fdac..e3533e01f 100644 --- a/ARCHITECTURE.md +++ b/ARCHITECTURE.md @@ -175,12 +175,12 @@ return true, result At exit, Neovim fires `BufUnload` for each loaded buffer, and then runs the `VimLeavePre` autocmds. The plugin registers two of them, and they run in this order. Neovim runs the autocmds of one event in the order that they were registered. `diff.lua` registers its autocmd when it first loads, and `M.setup` loads `diff.lua` before it registers its own [lua/claudecode/init.lua:M.setup]. -First, the autocmd of `diff.lua`, in the `ClaudeCodeMCPDiff` group, rejects each diff whose status is `pending`, oldest first, while the server still runs. Then it closes every diff and clears its state. It runs whether or not the server runs [lua/claudecode/diff.lua:cleanup_at_exit] [lua/claudecode/diff.lua:M._reject_pending_diffs] [lua/claudecode/diff.lua:M._cleanup_all_active_diffs]: +First, the autocmd of `diff.lua`, in the `ClaudeCodeMCPDiff` group, rejects each diff whose status is `pending`, oldest first, while the server still runs. Then it closes every diff and clears its state. It runs whether or not the server runs [lua/claudecode/diff.lua:cleanup_at_exit] [lua/claudecode/diff.lua:reject_pending_diffs] [lua/claudecode/diff.lua:M._cleanup_all_active_diffs]: ```lua -- diff.lua, module level (shortened) local function cleanup_at_exit() - M._reject_pending_diffs() -- reject each diff that is pending, oldest first + reject_pending_diffs() -- reject each diff that is pending, oldest first M._cleanup_all_active_diffs("shutdown") -- close each diff and clear its state end @@ -190,7 +190,7 @@ vim.api.nvim_create_autocmd("VimLeavePre", { }) ``` -The `BufUnload` loop that runs before both autocmds already rejects most diffs, because the `BufUnload` autocmd of a proposal buffer rejects its diff [lua/claudecode/diff.lua:register_diff_autocmds]. That loop stops when an autocmd deletes the buffer that it fired for, and SIGTERM and SIGHUP run `VimLeavePre` without the loop. `_reject_pending_diffs` rejects the diffs that the loop did not reach, and it skips a diff whose status is not `pending`. A reject can remove its own diff (a new-file diff with `on_new_file_reject = "keep_empty"`), so a diff can be gone before the next one is rejected. +The `BufUnload` loop that runs before both autocmds already rejects most diffs, because the `BufUnload` autocmd of a proposal buffer rejects its diff [lua/claudecode/diff.lua:register_diff_autocmds]. That loop stops when an autocmd deletes the buffer that it fired for, and SIGTERM and SIGHUP run `VimLeavePre` without the loop. `reject_pending_diffs` rejects the diffs that the loop did not reach, and it skips a diff whose status is not `pending`. A reject can remove its own diff (a new-file diff with `on_new_file_reject = "keep_empty"`), so a diff can be gone before the next one is rejected. Second, the autocmd of `init.lua`, in the `ClaudeCodeShutdown` group, runs `stop` only when the server runs. Without a server, it only clears the @-mention queue [lua/claudecode/init.lua:M.setup] [lua/claudecode/init.lua:clear_mention_queue]. `stop` removes the lock file, stops the server, and clears the queue [lua/claudecode/init.lua:M.stop]. The server's own `stop` sends a response for each request whose handler is still running, and the TCP server then closes each client with the close code 1001 [lua/claudecode/server/init.lua:M.stop] [lua/claudecode/server/tcp.lua:M.stop_server]: diff --git a/README.md b/README.md index 3f0df0e03..9893e40cd 100644 --- a/README.md +++ b/README.md @@ -1147,7 +1147,7 @@ The plugin runs the CLI through a [terminal provider](./lua/claudecode/terminal/ about `:qa!`, SIGTERM and a quit from a non-nested autocmd also hold on 0.9.5, 0.10.4 and 0.11.5. SIGKILL stands in for a crash, which was not probed: - An exit that runs `VimLeavePre` (`:qa!`, SIGTERM or SIGHUP) gives each waiting call one `DIFF_REJECTED` - [lua/claudecode/diff.lua:M._reject_pending_diffs]. A call that already has its answer, such as a `FILE_SAVED`, + [lua/claudecode/diff.lua:reject_pending_diffs]. A call that already has its answer, such as a `FILE_SAVED`, gets no second one. - A call that the exit cuts off during its `openDiff` setup gets one -32603 error from the server stop. It has no response sender yet, so nothing else can answer it. This happens when a `nested = true` `ClaudeCodeDiffOpened` diff --git a/lua/claudecode/README.md b/lua/claudecode/README.md index 22ee5077d..c9ed327af 100644 --- a/lua/claudecode/README.md +++ b/lua/claudecode/README.md @@ -182,7 +182,7 @@ Module map (direct files): - In the `vertical` and `horizontal` layouts the saved content is the lines of the proposal buffer, joined with line breaks, plus a final line break only when the buffer's `'eol'` is on and the buffer has a line. The diff sets `'eol'` from the proposal when it creates the buffer, so a proposal without a final line break is saved without one, and the user's edits keep that ending. A buffer with no lines at all (an empty proposal, or a buffer the user emptied with `ggdG` or `:%delete`) is saved as an empty string, because `:w` writes no bytes for it whatever `'eol'` says. A buffer with one empty line is saved as `"\n"` when `'eol'` is on and as `""` when it is off. The save tells the two apart with `wordcount().bytes`, because `nvim_buf_get_lines` returns one empty string for both [diff.lua:M._setup_blocking_diff] [diff.lua:M._resolve_diff_as_saved] - A pending diff rejects at once when its proposal buffer is shown in no window, in the split and the unified layout: its last window closes (`:q`, `:close`, `c`, `:tabclose`), or that window switches to another buffer (`:edit`, `:buffer`, `:enew`, which only hide it). A copy of the proposal in another window or tab defers the reject until that copy leaves too. The switch path rejects on the next loop turn, and only when the diff is still the same one and still hidden. Both layouts keep the proposal buffer's `'bufhidden'` at `hide`: the buffer is modified, and `wipe` would make `:q` fail with E37 [diff.lua:M._register_reject_when_hidden] [diff_inline.lua:M.setup_inline_diff] - A diff that has its answer, by either one, clears `'modified'` on its proposal buffer. While the flag stayed set, `:wq` left the window open, and `:qa` stopped with E37 or E162 and moved a window onto the hidden buffer until `close_tab` deleted it [diff.lua:mark_proposal_unmodified] -- At exit, Neovim fires `BufUnload` for each loaded buffer before `VimLeavePre`, and a proposal's `BufUnload` autocmd rejects its diff, but that loop stops as soon as an autocmd deletes the buffer it fired for. The reject of a new-file diff with `on_new_file_reject = "keep_empty"`, in a tab that the diff did not open, deletes its proposal buffer, and another plugin's buffer can delete itself in its own `BufUnload`. The diffs after either get no `BufUnload` (probed on Neovim 0.8.0, 0.9.5, 0.10.4, 0.11.5 and 0.12.3). SIGTERM and SIGHUP run `VimLeavePre` without that loop. A quit from inside an autocmd that is not `nested`, SIGKILL and a crash run no `VimLeavePre`. The `VimLeavePre` autocmd of `ClaudeCodeMCPDiff` therefore runs `_reject_pending_diffs` and then `_cleanup_all_active_diffs("shutdown")`. `_reject_pending_diffs` rejects each diff with the status `pending`, oldest first, each under `pcall`, and skips a diff with another status. A reject can remove its own diff (the `keep_empty` case above), so a diff can be gone before the next one is rejected. The reject of a diff whose `openDiff` setup has not finished stays in `setup_result`, because the call has no response sender yet, so the server's `stop` sends that call's response. The README at the repository root, under Global Invariants, lists what Claude receives for each kind of call [diff.lua:M._reject_pending_diffs] [diff.lua:cleanup_at_exit] [diff.lua:M.open_diff_blocking] [server/init.lua:M.stop] +- At exit, Neovim fires `BufUnload` for each loaded buffer before `VimLeavePre`, and a proposal's `BufUnload` autocmd rejects its diff, but that loop stops as soon as an autocmd deletes the buffer it fired for. The reject of a new-file diff with `on_new_file_reject = "keep_empty"`, in a tab that the diff did not open, deletes its proposal buffer, and another plugin's buffer can delete itself in its own `BufUnload`. The diffs after either get no `BufUnload` (probed on Neovim 0.8.0, 0.9.5, 0.10.4, 0.11.5 and 0.12.3). SIGTERM and SIGHUP run `VimLeavePre` without that loop. A quit from inside an autocmd that is not `nested`, SIGKILL and a crash run no `VimLeavePre`. The `VimLeavePre` autocmd of `ClaudeCodeMCPDiff` therefore runs `reject_pending_diffs` and then `_cleanup_all_active_diffs("shutdown")`. `reject_pending_diffs` rejects each diff with the status `pending`, oldest first, each under `pcall`, and skips a diff with another status. A reject can remove its own diff (the `keep_empty` case above), so a diff can be gone before the next one is rejected. The reject of a diff whose `openDiff` setup has not finished stays in `setup_result`, because the call has no response sender yet, so the server's `stop` sends that call's response. The README at the repository root, under Global Invariants, lists what Claude receives for each kind of call [diff.lua:reject_pending_diffs] [diff.lua:cleanup_at_exit] [diff.lua:M.open_diff_blocking] [server/init.lua:M.stop] - The cleanup of a diff closes the proposal window, and the window that the split layout opened for the file's old text, only while each still shows its buffer. A window that the user switched to another buffer stays with that buffer. `:edit` turns diff mode off by itself, except when the new buffer is in diff mode in another window, as the file's own buffer is. So the split layout's cleanup ends diff mode only in a kept window that shows the file's own buffer, and leaves a diff mode in a window that shows another buffer, which is the user's. One limit comes from Neovim: the default `closeoff` in `'diffopt'` turns diff mode off in the last window that has it when another diff window closes. That ends a user's diff mode in a kept original-side window when the proposal window closes, and in a kept proposal window when the cleanup closes the window that the plugin opened for the file. A user's diff mode over two or more windows stays. The unified layout needs no cleanup there: it sets `'signcolumn'` to `yes` with local scope, for the proposal buffer only, so the window shows the sign column it had before the diff once the user switches it, and keeps a value the user sets afterwards. Two cases still close the window: a diff in its own tab (`open_in_new_tab`, or a tab that already has a diff) closes the whole tab, and the fallback window that the plugin opens in a tab that holds only the Claude terminal always closes [diff.lua:M._close_proposal_window] [diff.lua:M._cleanup_diff_state] [diff_inline.lua:M.cleanup_inline_diff] [diff_inline.lua:M.setup_inline_diff] - Rejecting a new-file diff that is not in its own tab cleans it up at once when `diff_opts.on_new_file_reject` is `"keep_empty"` (the default), before `close_tab`. A `unified` diff gets no such cleanup [diff.lua:M._resolve_diff_as_rejected] [diff_inline.lua:M.resolve_inline_as_rejected] - `BufDelete`, `BufUnload`, and `BufWipeout` on the proposal buffer reject a pending diff. `WinClosed` rejects it only when no other window in any tab shows the proposal buffer. The original file's buffer is not watched. The unified layout registers the same autocmds [diff.lua:register_diff_autocmds] [diff_inline.lua:M.setup_inline_diff] @@ -202,7 +202,7 @@ Module map (direct files): - The plugin broadcasts `at_mentioned` (`filePath`, `lineStart`, `lineEnd`) [init.lua:M._broadcast_at_mention] and `selection_changed` [selection.lua:M.send_selection_update] to every connected client - While Claude holds a visual selection, and while you select in a buffer (from entering visual mode until the flush), `selection.lua` attaches an `nvim_buf_attach` listener (`on_lines`, `on_reload`, `on_detach`) to that buffer, once per buffer. It does not attach to the Claude terminal buffer, whose output is a stream of edits. It detaches itself at the next `on_lines` after Claude holds no selection of that buffer and you do not select in it, and a reload with `:edit` or `:edit!` detaches it, so it attaches again when the buffer still holds the selection [selection.lua:watch_selection_buffer] - `init.lua` feeds `` after a successful `:ClaudeCodeSend` that does not take the visual path, outside a tree buffer. It also feeds `` when `:ClaudeCode` or `:ClaudeCodeFocus` runs in Visual mode [init.lua:M._create_commands] -- Augroups: `ClaudeCodeShutdown`, which holds the `VimLeavePre` autocmd that stops on exit [init.lua:M.setup], `ClaudeCodeSelection` [selection.lua:M._create_autocommands], `ClaudeCodeMCPDiff` [diff.lua:get_autocmd_group], `ClaudeCodeDiffCleanup` [diff.lua:M._open_native_diff], `ClaudeCodeNativeAdopted` [terminal/native.lua:adopt_existing_terminal], `ClaudeCodeOpenFileNewFiles` [tools/open_file.lua:new_file_buffers], `snacks_win_`, one per Snacks terminal whose split the plugin re-creates [terminal/snacks.lua:register_window_events], and a module-level `VimLeavePre` autocmd in `diff.lua` whose callback calls `M._reject_pending_diffs()` and then `M._cleanup_all_active_diffs("shutdown")` [diff.lua:cleanup_at_exit] +- Augroups: `ClaudeCodeShutdown`, which holds the `VimLeavePre` autocmd that stops on exit [init.lua:M.setup], `ClaudeCodeSelection` [selection.lua:M._create_autocommands], `ClaudeCodeMCPDiff` [diff.lua:get_autocmd_group], `ClaudeCodeDiffCleanup` [diff.lua:M._open_native_diff], `ClaudeCodeNativeAdopted` [terminal/native.lua:adopt_existing_terminal], `ClaudeCodeOpenFileNewFiles` [tools/open_file.lua:new_file_buffers], `snacks_win_`, one per Snacks terminal whose split the plugin re-creates [terminal/snacks.lua:register_window_events], and a module-level `VimLeavePre` autocmd in `diff.lua` whose callback calls `reject_pending_diffs()` and then `M._cleanup_all_active_diffs("shutdown")` [diff.lua:cleanup_at_exit] - Each diff that opens in a new tab adds a buffer-local `BufEnter` autocmd (`startinsert`) to the Claude terminal buffer in `ClaudeCodeMCPDiff`, unless `terminal.auto_insert` is `false`, `diff_opts.hide_terminal_in_new_tab` is `true`, or no valid Claude terminal buffer exists. A `WinClosed` autocmd deletes it when the terminal window it was made for closes [diff.lua:display_terminal_in_new_tab] - Diffs create scratch buffers, splits, and tabs. They resize the terminal window and fire `User` events. Cleanup closes these and reloads unchanged file buffers 100 ms after a save [diff.lua:M.close_diff_by_tab_name] - When a blocking diff resolves, `open_diff_blocking` sends the reply through the global `_G.claude_deferred_responses[tostring(co)]`, then sets that entry to nil. Without an entry, it logs an error [diff.lua:M.open_diff_blocking] diff --git a/lua/claudecode/diff.lua b/lua/claudecode/diff.lua index 044525742..00dfdd84d 100644 --- a/lua/claudecode/diff.lua +++ b/lua/claudecode/diff.lua @@ -2013,7 +2013,7 @@ end ---(`keep_empty`), so a diff can be gone before the next one is rejected. The reject of a diff whose openDiff setup is ---still running stays in `setup_result` (see `open_diff_blocking`) and sends nothing: the call has no response sender ---yet, and the server's `stop` sends its response. -function M._reject_pending_diffs() +local function reject_pending_diffs() local pending = {} for tab_name, diff_data in pairs(active_diffs) do if diff_data.status == "pending" then @@ -2039,7 +2039,7 @@ end ---pending. SIGKILL, a crash, or a quit from inside an autocmd that is not `nested` runs no VimLeavePre. The reject ---pass runs first. The cleanup pass of every diff runs after it. local function cleanup_at_exit() - M._reject_pending_diffs() + reject_pending_diffs() M._cleanup_all_active_diffs("shutdown") end From f2867bf4ecc4983c809d208c9706d42b94527a17 Mon Sep 17 00:00:00 2001 From: Thomas Kosiewski Date: Thu, 12 Nov 2026 10:29:15 +0000 Subject: [PATCH 09/11] docs: scope the exit probes per fact and name what the exit spec covers The root README said every exit fact was probed on Neovim 0.8.0 and 0.12.3. A probe set on the final code now covers each fact on Neovim 0.8.0, 0.9.5, 0.10.4, 0.11.5 and 0.12.3: :qa!, SIGTERM, SIGHUP, SIGKILL, a quit from a non-nested autocmd, a call cut off in setup, :ClaudeCodeStop from such a handler, a saved diff at exit, and the ClaudeCodeDiffClosed order with keep_empty and close_window in the vertical and horizontal layouts. A crash was not probed, and SIGKILL stands in for it. The README named only the vertical layout for the early removal of a new-file diff with keep_empty. The horizontal layout does the same, so the sentence says both. The header of the exit integration spec now names the four exits that it runs and the ones that it does not. --- README.md | 15 +++++++-------- tests/integration/diff_exit_reject_spec.lua | 5 ++++- 2 files changed, 11 insertions(+), 9 deletions(-) diff --git a/README.md b/README.md index 9893e40cd..25e1e3e43 100644 --- a/README.md +++ b/README.md @@ -1142,10 +1142,9 @@ The plugin runs the CLI through a [terminal provider](./lua/claudecode/terminal/ pending diff [lua/claudecode/diff.lua:M.close_diff_by_tab_name]. A diff that resolves before `openDiff` yields, for example from a `ClaudeCodeDiffOpened` handler, is answered at once [lua/claudecode/diff.lua:M.open_diff_blocking]. - Exit answers. This bullet is the one place that states what Claude gets when Neovim exits. The other docs describe - how the code runs the exit and link here. A call is waiting when its `openDiff` setup has finished and nobody has - answered its diff. Each fact below was probed with a real WebSocket client on Neovim 0.8.0 and 0.12.3. The facts - about `:qa!`, SIGTERM and a quit from a non-nested autocmd also hold on 0.9.5, 0.10.4 and 0.11.5. SIGKILL stands in - for a crash, which was not probed: + how the code runs the exit and point here. A call is waiting when its `openDiff` setup has finished and nobody has + answered its diff. Each fact below was probed with a real WebSocket client on Neovim 0.8.0, 0.9.5, 0.10.4, 0.11.5 + and 0.12.3. SIGKILL stands in for a crash, which was not probed: - An exit that runs `VimLeavePre` (`:qa!`, SIGTERM or SIGHUP) gives each waiting call one `DIFF_REJECTED` [lua/claudecode/diff.lua:reject_pending_diffs]. A call that already has its answer, such as a `FILE_SAVED`, gets no second one. @@ -1161,10 +1160,10 @@ The plugin runs the CLI through a [terminal provider](./lua/claudecode/terminal/ - In an exit that runs `VimLeavePre`, the autocmd of `ClaudeCodeMCPDiff` rejects every diff with the status `pending` first and removes every diff after that [lua/claudecode/diff.lua:cleanup_at_exit]. A reject can remove its own diff at once. With the default `on_new_file_reject = "keep_empty"`, a new-file diff in the `vertical` - layout that is not in its own tab is removed when it is rejected, so its `ClaudeCodeDiffClosed` handler runs while - a later diff is still `pending` [lua/claudecode/diff.lua:M.\_resolve_diff_as_rejected]. With `"close_window"`, - both diffs have their answers before the first `ClaudeCodeDiffClosed` fires (probed with one new-file and one - existing-file diff, for `:qa!` and SIGTERM). + or `horizontal` layout that is not in its own tab is removed when it is rejected, so its `ClaudeCodeDiffClosed` + handler runs while a later diff is still `pending` [lua/claudecode/diff.lua:M.\_resolve_diff_as_rejected]. With + `"close_window"`, both diffs have their answers before the first `ClaudeCodeDiffClosed` fires (probed in both + layouts with one new-file and one existing-file diff, for `:qa!` and SIGTERM). - How the code runs the exit: [lua/claudecode/README.md](./lua/claudecode/README.md) and the Resource Cleanup section of [ARCHITECTURE.md](./ARCHITECTURE.md). diff --git a/tests/integration/diff_exit_reject_spec.lua b/tests/integration/diff_exit_reject_spec.lua index 42fe1515d..7bacabc19 100644 --- a/tests/integration/diff_exit_reject_spec.lua +++ b/tests/integration/diff_exit_reject_spec.lua @@ -1,4 +1,7 @@ --- Real-Neovim check of the exit answers that README.md lists under Global Invariants, in "Exit answers". +-- Real-Neovim check of the answer that each openDiff call gets in four exits: `:qa!`, SIGTERM, a buffer that deletes +-- itself in its own BufUnload, and `:qa!` inside a `nested = true` ClaudeCodeDiffOpened handler. README.md lists the +-- answers under Global Invariants, in "Exit answers". This spec does not run SIGHUP, SIGKILL, a quit from a +-- non-nested autocmd or :ClaudeCodeStop, and it does not check the close codes or the ClaudeCodeDiffClosed order. -- Neovim fires BufUnload for each loaded buffer at exit, and the proposal buffers' autocmds reject their diffs. That -- loop stops once an autocmd deletes the buffer it fired for: the reject of a new-file diff with the default -- `on_new_file_reject = "keep_empty"`, in a tab the diff did not open, deletes its proposal buffer, and another From 9e3e0aa3b2d47e1b4148e2eed43c2b3b07fb82aa Mon Sep 17 00:00:00 2001 From: Thomas Kosiewski Date: Thu, 12 Nov 2026 12:10:31 +0000 Subject: [PATCH 10/11] chore(skills): re-record the mention-queue and selection-tracking locks after the server stop change Both models cover lua/claudecode/server/init.lua, which this PR changes in M._handle_request and M.stop. Only the file hash changed in each lock. No covered block changed, and the change cannot reach a queue, a timer, the handshake, a broadcast or the selection state: M.stop now sends the response of a request whose handler is still running, before it stops the TCP server. The full check.sh of each skill passes (mention queue: main 270120, live 57814/57814/24692 distinct states. selection tracking: main 129986). The twelve mention-queue replays all kept their promise, among them restart-leak and stop-in-wait, which go through M.stop. --- .agents/skills/verify-mention-queue/covered.lock | 2 +- .agents/skills/verify-selection-tracking/covered.lock | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/.agents/skills/verify-mention-queue/covered.lock b/.agents/skills/verify-mention-queue/covered.lock index 5ab4c37c5..c1bcc48ce 100644 --- a/.agents/skills/verify-mention-queue/covered.lock +++ b/.agents/skills/verify-mention-queue/covered.lock @@ -98,7 +98,7 @@ site lua/claudecode/server/client.lua: if not client.handshake_complete then @ f site lua/claudecode/server/client.lua: if on_open then @ function M.process_data(client, data, on_message, on_close, on_error, auth_token, on_open) site lua/claudecode/server/client.lua: on_open(client) @ function M.process_data(client, data, on_message, on_close, on_error, auth_token, on_open) == lua/claudecode/server/init.lua -file lua/claudecode/server/init.lua 8da8851fff781946 +file lua/claudecode/server/init.lua 8a3d1b16b91dd5a7 block on_open 605dc737a2bfdad0 block server_broadcast fc69234ae8ed7cf0 block server_status e2759841ce0f1b90 diff --git a/.agents/skills/verify-selection-tracking/covered.lock b/.agents/skills/verify-selection-tracking/covered.lock index 29c192c7b..da99f88b8 100644 --- a/.agents/skills/verify-selection-tracking/covered.lock +++ b/.agents/skills/verify-selection-tracking/covered.lock @@ -162,7 +162,7 @@ site lua/claudecode/selection.lua: watch_selection_buffer(buf) @ local function site lua/claudecode/selection.lua: watch_selection_buffer(bufnr) @ local function record_selection_read(bufnr) site lua/claudecode/selection.lua: watched_buffers = {}, @ M.state = { == lua/claudecode/server/init.lua -file lua/claudecode/server/init.lua 8da8851fff781946 +file lua/claudecode/server/init.lua 8a3d1b16b91dd5a7 block broadcast fc69234ae8ed7cf0 block on_connect daf282e2fe8f3c9a == lua/claudecode/tools/get_current_selection.lua From 43fc68d619e218995dbba2fc07a128c6ff1fbaee Mon Sep 17 00:00:00 2001 From: Thomas Kosiewski Date: Thu, 12 Nov 2026 12:47:04 +0000 Subject: [PATCH 11/11] refactor(diff): inline the exit reject pass into cleanup_at_exit, and wrap the long lines Codex review, round 2, on 9e3e0aa3: 1. AGENTS.md (Code Shape, one-off logic) keeps a single-use function only when it is recursive, a named algorithm, or when inlining adds flags or two levels of nesting. reject_pending_diffs had one caller, and inlining it adds none of those, so its body now sits in cleanup_at_exit. The behavior is identical. check.sh reads the loop from the cleanup_at_exit block (the reject_pending block is gone), and the docs and model comments name cleanup_at_exit. 2. One comment line in server/init.lua was 131 columns wide. It and three other added Lua lines over 120 columns, in two test files, are wrapped. The three covered.lock files that cover the changed files are re-recorded after the full check.sh of each skill passed. --- .agents/skills/verify-diff-lifecycle/SKILL.md | 6 ++-- .agents/skills/verify-diff-lifecycle/check.sh | 12 ++++---- .../skills/verify-diff-lifecycle/covered.lock | 13 ++++---- .../model/DiffLifecycle.tla | 4 +-- .../skills/verify-mention-queue/covered.lock | 2 +- .../verify-selection-tracking/covered.lock | 2 +- ARCHITECTURE.md | 6 ++-- README.md | 2 +- lua/claudecode/README.md | 4 +-- lua/claudecode/diff.lua | 30 ++++++++----------- lua/claudecode/server/init.lua | 4 +-- tests/integration/diff_exit_reject_spec.lua | 13 ++++++-- tests/unit/server/in_flight_request_spec.lua | 3 +- 13 files changed, 51 insertions(+), 50 deletions(-) diff --git a/.agents/skills/verify-diff-lifecycle/SKILL.md b/.agents/skills/verify-diff-lifecycle/SKILL.md index 54dcb1284..a12b63494 100644 --- a/.agents/skills/verify-diff-lifecycle/SKILL.md +++ b/.agents/skills/verify-diff-lifecycle/SKILL.md @@ -55,7 +55,7 @@ The record has one group of lines per file: a `== ` header, the file's has Covered blocks: -- In `diff.lua`: `_resolve_diff_as_saved`, `_resolve_diff_as_rejected`, `open_diff_blocking`, `_setup_blocking_diff`, `close_diff_by_tab_name`, `close_pending_diffs`, `close_diffs_for_client`, `close_active_diffs`, `_cleanup_all_active_diffs`, `reject_pending_diffs`, `cleanup_at_exit` (the `VimLeavePre` callback), `_register_diff_state`, `_cleanup_diff_state`, `register_diff_autocmds` (the `:w` and buffer-delete paths), `_write_proposal` (what a write of the proposal does), `_register_reject_when_hidden` (the `:q` and buffer-switch paths, shared by both layouts), `fire_diff_event`, `accept_current_diff`, `deny_current_diff`. +- In `diff.lua`: `_resolve_diff_as_saved`, `_resolve_diff_as_rejected`, `open_diff_blocking`, `_setup_blocking_diff`, `close_diff_by_tab_name`, `close_pending_diffs`, `close_diffs_for_client`, `close_active_diffs`, `_cleanup_all_active_diffs`, `cleanup_at_exit` (the `VimLeavePre` callback), `_register_diff_state`, `_cleanup_diff_state`, `register_diff_autocmds` (the `:w` and buffer-delete paths), `_write_proposal` (what a write of the proposal does), `_register_reject_when_hidden` (the `:q` and buffer-switch paths, shared by both layouts), `fire_diff_event`, `accept_current_diff`, `deny_current_diff`. - In `diff_inline.lua`: `setup_inline_diff`, `resolve_inline_as_saved`, `resolve_inline_as_rejected`. - In `server/init.lua`: `M.stop`, `on_disconnect`, `M._handle_request`, `M._setup_deferred_response`. - In `tools/init.lua` and `tools/open_diff.lua`: `M.handle_invoke`, and the openDiff `handler`. @@ -77,7 +77,7 @@ A changed file with no changed block or call site still needs a look. A wrapper | `CloseTabRejectsPending` | `close_diff_by_tab_name` rejects a pending diff before cleanup | | `CloseEachByName` | `close_active_diffs` closes each match through `close_diff_by_tab_name` | | `StopRejectsPending` | `M.stop` calls `close_pending_diffs`, then `stop_server`, then clears the senders | -| `ExitRejectsPending` | the `VimLeavePre` callback `cleanup_at_exit` calls `reject_pending_diffs` before `_cleanup_all_active_diffs`, so the final cleanup pass finds no pending diff | +| `ExitRejectsPending` | the `VimLeavePre` callback `cleanup_at_exit` rejects each pending diff before it calls `_cleanup_all_active_diffs`, so the final cleanup pass finds no pending diff | | `ExitAnswersInFlight` | `M.stop` answers each request in `M.state.in_flight`, and `_handle_request` sends nothing after a stop that answered it | | `StopPendingOnly`, `DisconnectPendingOnly` | the `status == "pending"` filters in `close_pending_diffs` and `close_diffs_for_client` | | `DisconnectRejects` | `on_disconnect` schedules `close_diffs_for_client` | @@ -124,7 +124,7 @@ Then decide which kind of failure it is: - The invariants run with 3 openDiff calls, 2 tab names and 2 clients, about 93,000 states. `DisconnectedCallsAnswered` runs with 2 calls, 2 tabs and 2 clients. It assumes weak fairness only on setup and on the scheduled disconnect cleanup. The user is never assumed to act, with one exception: the disconnect cleanup waits until Neovim allows window changes (`utils.when_windows_allowed`), so a user in the command-line window is assumed to leave it, and textlock is assumed to end. - In every run, a user `ClaudeCodeDiffOpened` handler (README "Diff Lifecycle Events") can accept or deny the diff during setup: `:ClaudeCodeDiffAccept`, `:ClaudeCodeDiffDeny`, or a `nested = true` autocmd that closes the proposal window. A default handler that closes the window resolves nothing, because WinClosed does not fire inside a non-nested autocmd. Other handler actions during setup are not modeled. -- `DiffCleanupAtExit` is the `VimLeavePre` callback of `diff.lua`: `reject_pending_diffs`, then `_cleanup_all_active_diffs("shutdown")`. It models neither a `ClaudeCodeDiffClosed` handler (the reject pass has none unless a `keep_empty` reject runs a cleanup) nor a saved diff lost at exit: Neovim quits right after the step, and its buffers go with it. The model represents `DiffCleanupAtExit` and `ServerStops` as separate atomic actions and explores both orders, because both are `VimLeavePre` autocmds. Neovim's own exit `BufUnload` loop, which rejects before `VimLeavePre`, is the `UserRejects` action. +- `DiffCleanupAtExit` is the `VimLeavePre` callback of `diff.lua`: it rejects each pending diff, then calls `_cleanup_all_active_diffs("shutdown")`. It models neither a `ClaudeCodeDiffClosed` handler (the reject pass has none unless a `keep_empty` reject runs a cleanup) nor a saved diff lost at exit: Neovim quits right after the step, and its buffers go with it. The model represents `DiffCleanupAtExit` and `ServerStops` as separate atomic actions and explores both orders, because both are `VimLeavePre` autocmds. Neovim's own exit `BufUnload` loop, which rejects before `VimLeavePre`, is the `UserRejects` action. - `ExitInSetup` is an exit inside a `nested = true` `ClaudeCodeDiffOpened` handler, while one call is in setup, so it needs `OpenedEventInSetup`. The call has no sender and its setup never returns, so only the server stop can answer it (`ExitAnswersInFlight`). The action runs both `VimLeavePre` autocmds in one order, diff cleanup first, then the server stop. The other order gives the same result, because each leaves the other nothing to do. A handler that runs `:ClaudeCodeStop` instead of an exit is not modeled: the handler returns after the stop, and `tests/unit/server/in_flight_request_spec.lua` checks that `_handle_request` then sends nothing. What Claude receives at an exit is in the root `README.md`, under Global Invariants. - `SenderCleared` alone is caught by no rule, because the pending guards also stop a second answer. `--mutants` shows that `AtMostOneAnswer` breaks when both `SenderCleared` and `GuardRejected` are removed. - `SavedDiffKept` knows which client owns each diff, and, while `:ClaudeCodeCloseAllDiffs` runs, which files or file buffers hold the accepted text. The model has no file or buffer contents: the command's first pass leaves the accepted text in any subset of the saved diffs' files or buffers, because the code skips a diff whose file buffer already has unsaved changes, or whose buffer it cannot change. The plugin never writes the file. That the first pass leaves the file on disk and the user's unsaved changes alone is checked by the unit tests, not here. A saved diff counts as lost when the user, a disconnect cleanup or a server stop tears it down, or when a client that does not own it does so while the owner is disconnected. A close by another client while the owner is connected counts as safe, because the owner got `FILE_SAVED` and can still write the file. The code is stricter than the rule: it keeps another client's saved diff in both cases. Ownership compares client ids, so a new connection must never get an old id. With `ClientIdsUnique` off, `ClientIdReused` lets a new connection take the id of a disconnected client. diff --git a/.agents/skills/verify-diff-lifecycle/check.sh b/.agents/skills/verify-diff-lifecycle/check.sh index bb8d22482..418a4ada7 100755 --- a/.agents/skills/verify-diff-lifecycle/check.sh +++ b/.agents/skills/verify-diff-lifecycle/check.sh @@ -50,7 +50,6 @@ covered_block setup_deferred "$SERVER" '^function M\._setup_deferred_response\(' covered_block user_close_all "$MAIN" 'nvim_create_user_command\("ClaudeCodeCloseAllDiffs"' covered_block close_active "$DIFF" '^local function close_active_diffs\(' covered_block cleanup_all "$DIFF" '^function M\._cleanup_all_active_diffs\(' -covered_block reject_pending "$DIFF" '^local function reject_pending_diffs\(' covered_block exit_cleanup "$DIFF" '^local function cleanup_at_exit\(' covered_block inline_saved "$INLINE" '^function M\.resolve_inline_as_saved\(' covered_block inline_rejected "$INLINE" '^function M\.resolve_inline_as_rejected\(' @@ -139,14 +138,13 @@ fact[ClientIdsUnique]=$(as_bool client_ids_unique) fact[StopRejectsPending]=$(as_bool in_order 'diff\.close_pending_diffs\(' 'tcp_server\.stop_server\(' \ '_G\.claude_deferred_responses = \{\}' <<<"$server_stop") fact[StopPendingOnly]=$(as_bool grep -qE '^ *return diff_data\.status == "pending"$' <<<"$close_pending") -# The VimLeavePre callback runs reject_pending_diffs before _cleanup_all_active_diffs, so a diff that no BufUnload -# rejected is rejected before the final cleanup pass. +# The VimLeavePre callback rejects each pending diff before it calls _cleanup_all_active_diffs, so a diff that no +# BufUnload rejected is rejected before the final cleanup pass. FACTS+=(ExitRejectsPending) exit_rejects_pending() { - in_order '^local function reject_pending_diffs\(\)$' 'diff_data\.status == "pending"' \ - 'pcall\(M\._resolve_diff_as_rejected, entry\.tab_name\)' <<<"$reject_pending" && - in_order '^local function cleanup_at_exit\(\)$' '^ reject_pending_diffs\(\)$' \ - '^ M\._cleanup_all_active_diffs\("shutdown"\)$' <<<"$exit_cleanup" && + in_order '^local function cleanup_at_exit\(\)$' 'diff_data\.status == "pending"' \ + 'pcall\(M\._resolve_diff_as_rejected, entry\.tab_name\)' \ + '^ M\._cleanup_all_active_diffs\("shutdown"\)$' <<<"$exit_cleanup" && in_order '^vim\.api\.nvim_create_autocmd\("VimLeavePre", \{$' '^ callback = cleanup_at_exit,$' <"$DIFF" } fact[ExitRejectsPending]=$(as_bool exit_rejects_pending) diff --git a/.agents/skills/verify-diff-lifecycle/covered.lock b/.agents/skills/verify-diff-lifecycle/covered.lock index 2f786c62a..63e852c6f 100644 --- a/.agents/skills/verify-diff-lifecycle/covered.lock +++ b/.agents/skills/verify-diff-lifecycle/covered.lock @@ -3,7 +3,7 @@ file lua/claudecode/config.lua 3afb1bdeff27bfb6 == lua/claudecode/cwd.lua file lua/claudecode/cwd.lua 35b7af3572d13018 == lua/claudecode/diff.lua -file lua/claudecode/diff.lua 4016a94cbce343f1 +file lua/claudecode/diff.lua a1c57f34e79b17ce block resolve_saved 4d34b0c7e34d561f block resolve_rejected c7a620816abff9a1 block open_blocking 92067df7f9d4d4d9 @@ -20,8 +20,7 @@ block put_in_buffer c5dedfca8261379b block held_for_other 3d1e2911d5310cc2 block close_active 446fbed632748300 block cleanup_all 43e7b595cf106229 -block reject_pending 4dc1ccb4a92cc822 -block exit_cleanup 46793014076b4e79 +block exit_cleanup 8bc9fbded77d818f block accept_current 7577efa33506569e block fire_event ef778bb39419d91a block register_state 8cf78b6711a9ecc6 @@ -64,8 +63,8 @@ site lua/claudecode/diff.lua: for _, diff_data in pairs(active_diffs) do @ funct site lua/claudecode/diff.lua: for _, diff_data in pairs(active_diffs) do @ function M.close_saved_diffs(reason) site lua/claudecode/diff.lua: for _, other in pairs(active_diffs) do @ local function save_window_sizes(tab) site lua/claudecode/diff.lua: for tab_name, _ in pairs(active_diffs) do @ function M._cleanup_all_active_diffs(reason) +site lua/claudecode/diff.lua: for tab_name, diff_data in pairs(active_diffs) do @ local function cleanup_at_exit() site lua/claudecode/diff.lua: for tab_name, diff_data in pairs(active_diffs) do @ local function close_active_diffs(filter_fn, reason, client_id) -site lua/claudecode/diff.lua: for tab_name, diff_data in pairs(active_diffs) do @ local function reject_pending_diffs() site lua/claudecode/diff.lua: function M._cleanup_all_active_diffs(reason) @ function M._cleanup_all_active_diffs(reason) site lua/claudecode/diff.lua: function M._cleanup_diff_state(tab_name, reason) @ function M._cleanup_diff_state(tab_name, reason) site lua/claudecode/diff.lua: function M._get_active_diffs() @ function M._get_active_diffs() @@ -99,7 +98,7 @@ site lua/claudecode/diff.lua: local diff_data = active_diffs[tab_name] @ local f site lua/claudecode/diff.lua: local entry = active_diffs[tab_name] @ function M._register_reject_when_hidden(tab_name, new_buffer, autocmd_ids) site lua/claudecode/diff.lua: local existing_diff = active_diffs[tab_name] @ function M.open_diff_blocking(old_file_path, new_file_path, new_file_contents, tab_name, client_id) site lua/claudecode/diff.lua: local function close_active_diffs(filter_fn, reason, client_id) @ local function close_active_diffs(filter_fn, reason, client_id) -site lua/claudecode/diff.lua: local ok, err = pcall(M._resolve_diff_as_rejected, entry.tab_name) @ local function reject_pending_diffs() +site lua/claudecode/diff.lua: local ok, err = pcall(M._resolve_diff_as_rejected, entry.tab_name) @ local function cleanup_at_exit() site lua/claudecode/diff.lua: local state = active_diffs[tab_name] @ function M._setup_blocking_diff(params, resolution_callback) site lua/claudecode/diff.lua: resolution_callback = resolution_callback, @ function M._setup_blocking_diff(params, resolution_callback) site lua/claudecode/diff.lua: return active_diffs @ function M._get_active_diffs() @@ -150,10 +149,10 @@ file lua/claudecode/server/frame.lua 3c50766875878d56 == lua/claudecode/server/handshake.lua file lua/claudecode/server/handshake.lua 3eca35d572094a55 == lua/claudecode/server/init.lua -file lua/claudecode/server/init.lua 8a3d1b16b91dd5a7 +file lua/claudecode/server/init.lua dbc9b57cf9a7142b block server_stop 506e2e53358dc947 block on_disconnect a0ce86cede4af786 -block handle_request 9cdbba664a85319f +block handle_request 3de5ff2f22d24871 block setup_deferred f9381caaae0fe517 site lua/claudecode/server/init.lua: _G.claude_deferred_responses = {} @ function M._setup_deferred_response(deferred_info) site lua/claudecode/server/init.lua: _G.claude_deferred_responses = {} @ function M.stop() diff --git a/.agents/skills/verify-diff-lifecycle/model/DiffLifecycle.tla b/.agents/skills/verify-diff-lifecycle/model/DiffLifecycle.tla index d379def4f..e6b36012f 100644 --- a/.agents/skills/verify-diff-lifecycle/model/DiffLifecycle.tla +++ b/.agents/skills/verify-diff-lifecycle/model/DiffLifecycle.tla @@ -343,7 +343,7 @@ ServerStops == SetW([v EXCEPT !.sender = [q \in Reqs |-> FALSE]]) /\ UNCHANGED <> -(* diff.lua reject_pending_diffs: _resolve_diff_as_rejected for each diff that is still pending. *) +(* diff.lua cleanup_at_exit, reject pass: _resolve_diff_as_rejected for each diff that is still pending. *) (* The order does not matter: each rejection touches only its own diff and its own call. *) RECURSIVE RejectEach(_, _) RejectEach(w, S) == @@ -352,7 +352,7 @@ RejectEach(w, S) == IN RejectEach(IF w.active[t] # NoDiff /\ w.active[t].status = "pending" THEN Resolve(w, t, "rejected") ELSE w, S \ {t}) -(* diff.lua VimLeavePre autocmd: reject_pending_diffs, then _cleanup_all_active_diffs("shutdown"), which *) +(* diff.lua VimLeavePre autocmd: the reject pass, then _cleanup_all_active_diffs("shutdown"), which *) (* removes every entry in any status. Neovim fires BufUnload for each loaded buffer before VimLeavePre, and *) (* the proposal buffers' autocmds reject their diffs there (the UserRejects action). That loop stops when an *) (* autocmd deletes the buffer it fired for, so a pending diff can still reach this step. Neovim quits right *) diff --git a/.agents/skills/verify-mention-queue/covered.lock b/.agents/skills/verify-mention-queue/covered.lock index c1bcc48ce..d1bf457ff 100644 --- a/.agents/skills/verify-mention-queue/covered.lock +++ b/.agents/skills/verify-mention-queue/covered.lock @@ -98,7 +98,7 @@ site lua/claudecode/server/client.lua: if not client.handshake_complete then @ f site lua/claudecode/server/client.lua: if on_open then @ function M.process_data(client, data, on_message, on_close, on_error, auth_token, on_open) site lua/claudecode/server/client.lua: on_open(client) @ function M.process_data(client, data, on_message, on_close, on_error, auth_token, on_open) == lua/claudecode/server/init.lua -file lua/claudecode/server/init.lua 8a3d1b16b91dd5a7 +file lua/claudecode/server/init.lua dbc9b57cf9a7142b block on_open 605dc737a2bfdad0 block server_broadcast fc69234ae8ed7cf0 block server_status e2759841ce0f1b90 diff --git a/.agents/skills/verify-selection-tracking/covered.lock b/.agents/skills/verify-selection-tracking/covered.lock index da99f88b8..b95e042bc 100644 --- a/.agents/skills/verify-selection-tracking/covered.lock +++ b/.agents/skills/verify-selection-tracking/covered.lock @@ -162,7 +162,7 @@ site lua/claudecode/selection.lua: watch_selection_buffer(buf) @ local function site lua/claudecode/selection.lua: watch_selection_buffer(bufnr) @ local function record_selection_read(bufnr) site lua/claudecode/selection.lua: watched_buffers = {}, @ M.state = { == lua/claudecode/server/init.lua -file lua/claudecode/server/init.lua 8a3d1b16b91dd5a7 +file lua/claudecode/server/init.lua dbc9b57cf9a7142b block broadcast fc69234ae8ed7cf0 block on_connect daf282e2fe8f3c9a == lua/claudecode/tools/get_current_selection.lua diff --git a/ARCHITECTURE.md b/ARCHITECTURE.md index e3533e01f..856d97103 100644 --- a/ARCHITECTURE.md +++ b/ARCHITECTURE.md @@ -175,12 +175,12 @@ return true, result At exit, Neovim fires `BufUnload` for each loaded buffer, and then runs the `VimLeavePre` autocmds. The plugin registers two of them, and they run in this order. Neovim runs the autocmds of one event in the order that they were registered. `diff.lua` registers its autocmd when it first loads, and `M.setup` loads `diff.lua` before it registers its own [lua/claudecode/init.lua:M.setup]. -First, the autocmd of `diff.lua`, in the `ClaudeCodeMCPDiff` group, rejects each diff whose status is `pending`, oldest first, while the server still runs. Then it closes every diff and clears its state. It runs whether or not the server runs [lua/claudecode/diff.lua:cleanup_at_exit] [lua/claudecode/diff.lua:reject_pending_diffs] [lua/claudecode/diff.lua:M._cleanup_all_active_diffs]: +First, the autocmd of `diff.lua`, in the `ClaudeCodeMCPDiff` group, rejects each diff whose status is `pending`, oldest first, while the server still runs. Then it closes every diff and clears its state. It runs whether or not the server runs [lua/claudecode/diff.lua:cleanup_at_exit] [lua/claudecode/diff.lua:M._cleanup_all_active_diffs]: ```lua -- diff.lua, module level (shortened) local function cleanup_at_exit() - reject_pending_diffs() -- reject each diff that is pending, oldest first + -- reject each diff that is pending, oldest first, each under pcall M._cleanup_all_active_diffs("shutdown") -- close each diff and clear its state end @@ -190,7 +190,7 @@ vim.api.nvim_create_autocmd("VimLeavePre", { }) ``` -The `BufUnload` loop that runs before both autocmds already rejects most diffs, because the `BufUnload` autocmd of a proposal buffer rejects its diff [lua/claudecode/diff.lua:register_diff_autocmds]. That loop stops when an autocmd deletes the buffer that it fired for, and SIGTERM and SIGHUP run `VimLeavePre` without the loop. `reject_pending_diffs` rejects the diffs that the loop did not reach, and it skips a diff whose status is not `pending`. A reject can remove its own diff (a new-file diff with `on_new_file_reject = "keep_empty"`), so a diff can be gone before the next one is rejected. +The `BufUnload` loop that runs before both autocmds already rejects most diffs, because the `BufUnload` autocmd of a proposal buffer rejects its diff [lua/claudecode/diff.lua:register_diff_autocmds]. That loop stops when an autocmd deletes the buffer that it fired for, and SIGTERM and SIGHUP run `VimLeavePre` without the loop. The reject pass of `cleanup_at_exit` rejects the diffs that the loop did not reach, and it skips a diff whose status is not `pending`. A reject can remove its own diff (a new-file diff with `on_new_file_reject = "keep_empty"`), so a diff can be gone before the next one is rejected. Second, the autocmd of `init.lua`, in the `ClaudeCodeShutdown` group, runs `stop` only when the server runs. Without a server, it only clears the @-mention queue [lua/claudecode/init.lua:M.setup] [lua/claudecode/init.lua:clear_mention_queue]. `stop` removes the lock file, stops the server, and clears the queue [lua/claudecode/init.lua:M.stop]. The server's own `stop` sends a response for each request whose handler is still running, and the TCP server then closes each client with the close code 1001 [lua/claudecode/server/init.lua:M.stop] [lua/claudecode/server/tcp.lua:M.stop_server]: diff --git a/README.md b/README.md index 25e1e3e43..bea1d8508 100644 --- a/README.md +++ b/README.md @@ -1146,7 +1146,7 @@ The plugin runs the CLI through a [terminal provider](./lua/claudecode/terminal/ answered its diff. Each fact below was probed with a real WebSocket client on Neovim 0.8.0, 0.9.5, 0.10.4, 0.11.5 and 0.12.3. SIGKILL stands in for a crash, which was not probed: - An exit that runs `VimLeavePre` (`:qa!`, SIGTERM or SIGHUP) gives each waiting call one `DIFF_REJECTED` - [lua/claudecode/diff.lua:reject_pending_diffs]. A call that already has its answer, such as a `FILE_SAVED`, + [lua/claudecode/diff.lua:cleanup_at_exit]. A call that already has its answer, such as a `FILE_SAVED`, gets no second one. - A call that the exit cuts off during its `openDiff` setup gets one -32603 error from the server stop. It has no response sender yet, so nothing else can answer it. This happens when a `nested = true` `ClaudeCodeDiffOpened` diff --git a/lua/claudecode/README.md b/lua/claudecode/README.md index d073eac3c..ddc5ede98 100644 --- a/lua/claudecode/README.md +++ b/lua/claudecode/README.md @@ -182,7 +182,7 @@ Module map (direct files): - In the `vertical` and `horizontal` layouts the saved content is the lines of the proposal buffer, joined with line breaks, plus a final line break only when the buffer's `'eol'` is on and the buffer has a line. The diff sets `'eol'` from the proposal when it creates the buffer, so a proposal without a final line break is saved without one, and the user's edits keep that ending. A buffer with no lines at all (an empty proposal, or a buffer the user emptied with `ggdG` or `:%delete`) is saved as an empty string, because `:w` writes no bytes for it whatever `'eol'` says. A buffer with one empty line is saved as `"\n"` when `'eol'` is on and as `""` when it is off. The save tells the two apart with `wordcount().bytes`, because `nvim_buf_get_lines` returns one empty string for both [diff.lua:M._setup_blocking_diff] [diff.lua:M._resolve_diff_as_saved] - A pending diff rejects at once when its proposal buffer is shown in no window, in the split and the unified layout: its last window closes (`:q`, `:close`, `c`, `:tabclose`), or that window switches to another buffer (`:edit`, `:buffer`, `:enew`, which only hide it). A copy of the proposal in another window or tab defers the reject until that copy leaves too. The switch path rejects on the next loop turn, and only when the diff is still the same one and still hidden. Both layouts keep the proposal buffer's `'bufhidden'` at `hide`: the buffer is modified, and `wipe` would make `:q` fail with E37 [diff.lua:M._register_reject_when_hidden] [diff_inline.lua:M.setup_inline_diff] - A diff that has its answer, by either one, clears `'modified'` on its proposal buffer. While the flag stayed set, `:wq` left the window open, and `:qa` stopped with E37 or E162 and moved a window onto the hidden buffer until `close_tab` deleted it [diff.lua:mark_proposal_unmodified] -- At exit, Neovim fires `BufUnload` for each loaded buffer before `VimLeavePre`, and a proposal's `BufUnload` autocmd rejects its diff, but that loop stops as soon as an autocmd deletes the buffer it fired for. The reject of a new-file diff with `on_new_file_reject = "keep_empty"`, in a tab that the diff did not open, deletes its proposal buffer, and another plugin's buffer can delete itself in its own `BufUnload`. The diffs after either get no `BufUnload` (probed on Neovim 0.8.0, 0.9.5, 0.10.4, 0.11.5 and 0.12.3). SIGTERM and SIGHUP run `VimLeavePre` without that loop. A quit from inside an autocmd that is not `nested`, SIGKILL and a crash run no `VimLeavePre`. The `VimLeavePre` autocmd of `ClaudeCodeMCPDiff` therefore runs `reject_pending_diffs` and then `_cleanup_all_active_diffs("shutdown")`. `reject_pending_diffs` rejects each diff with the status `pending`, oldest first, each under `pcall`, and skips a diff with another status. A reject can remove its own diff (the `keep_empty` case above), so a diff can be gone before the next one is rejected. The reject of a diff whose `openDiff` setup has not finished stays in `setup_result`, because the call has no response sender yet, so the server's `stop` sends that call's response. The README at the repository root, under Global Invariants, lists what Claude receives for each kind of call [diff.lua:reject_pending_diffs] [diff.lua:cleanup_at_exit] [diff.lua:M.open_diff_blocking] [server/init.lua:M.stop] +- At exit, Neovim fires `BufUnload` for each loaded buffer before `VimLeavePre`, and a proposal's `BufUnload` autocmd rejects its diff, but that loop stops as soon as an autocmd deletes the buffer it fired for. The reject of a new-file diff with `on_new_file_reject = "keep_empty"`, in a tab that the diff did not open, deletes its proposal buffer, and another plugin's buffer can delete itself in its own `BufUnload`. The diffs after either get no `BufUnload` (probed on Neovim 0.8.0, 0.9.5, 0.10.4, 0.11.5 and 0.12.3). SIGTERM and SIGHUP run `VimLeavePre` without that loop. A quit from inside an autocmd that is not `nested`, SIGKILL and a crash run no `VimLeavePre`. The `VimLeavePre` autocmd of `ClaudeCodeMCPDiff` therefore runs `cleanup_at_exit`, which first rejects each diff with the status `pending`, oldest first, each under `pcall`, and skips a diff with another status. A reject can remove its own diff (the `keep_empty` case above), so a diff can be gone before the next one is rejected. The reject of a diff whose `openDiff` setup has not finished stays in `setup_result`, because the call has no response sender yet, so the server's `stop` sends that call's response. The README at the repository root, under Global Invariants, lists what Claude receives for each kind of call [diff.lua:cleanup_at_exit] [diff.lua:M.open_diff_blocking] [server/init.lua:M.stop] - The cleanup of a diff closes the proposal window, and the window that the split layout opened for the file's old text, only while each still shows its buffer. A window that the user switched to another buffer stays with that buffer. Neovim saves a window's options, `'diff'` included, for each buffer that leaves it, and sets them again when that buffer enters the window. So `:edit #`, or `:edit `, back to a file that left the window in diff mode turns diff mode on again, after the diff has ended. A window that never showed the buffer can take diff mode on in two ways. It can copy the current options of a window that shows the buffer: `:split ` copies the window that it splits, and a window in a new tab page, from `:tabnew ` or from `:tabnew` and `:buffer `, can get diff mode while a window of another tab page shows the file in diff mode. Or it can take a set that another window saved when the buffer left it: while no window shows the buffer, a set with diff mode on reaches a window of the same tab page, and does not reach a window of another tab page or a window that closed. The split layout's cleanup does one thing about that. After it closes its windows and deletes its buffers, and before it fires `ClaudeCodeDiffClosed`, it runs `:diffoff` in a kept window that shows the file's own buffer, and in each other window of the original side's tab page that shows the file and is the only window of that tab page with `'diff'` on, so nothing that a handler of that event starts is looked at. A diff needs a partner window, so the plugin takes a window that is the only one with `'diff'` on in its tab page for the leftover of the ended diff, and a diff of the user's own over two or more windows stays. The `'diff'` check leaves an option that the user set after turning diff mode off. `:diffoff` also puts back the options that diff mode set (`'scrollbind'`, `'cursorbind'`, `'foldmethod'`, `'foldcolumn'`, `'wrap'`). When Neovim refuses `:diffoff` in a window, as it does in the command-line window, the window keeps its diff mode [diff.lua:end_diff_mode_in_windows_that_show_the_file]. The cleanup looks only at the tab page of the original side and only once, so a file that comes back later gets the options that Neovim saved or copies, diff mode included, as on main. That covers `:edit #` or `:edit ` in a window that left the file, a new window for it such as `:split `, a window of another tab page that took diff mode from the diff's windows (`:tabnew `, `:tab sbuffer`, or `:tabnew` then `:buffer `), and a `ClaudeCodeDiffClosed` handler that brings the file back into a window. One limit comes from Neovim: the default `closeoff` in `'diffopt'` turns diff mode off in the last window that has it when another diff window closes. That ends a user's diff mode in a kept original-side window when the proposal window closes, and in a kept proposal window when the cleanup closes the window that the plugin opened for the file. It also ends it in a window that the diff never used, when the user joined that window to the diff with `:diffthis` and it is the last diff window left after the cleanup closes the plugin's window for the file. A user's diff mode over two or more windows stays. The unified layout needs no cleanup there: it sets `'signcolumn'` to `yes` with local scope, for the proposal buffer only, so once the user switches the window, `'signcolumn'` is what Neovim gives the new buffer in that window: the value of a window that shows or showed that buffer, a `:setlocal` value of the editor window included, or, for a buffer that no window showed, the window default (what `:set` gives, which a new split copies from the window it splits). The window keeps a value that the user sets afterwards. Two cases still close the window: a diff in its own tab (`open_in_new_tab`, or a tab that already has a diff) closes the whole tab, and the fallback window that the plugin opens in a tab that holds only the Claude terminal always closes [diff.lua:M._close_proposal_window] [diff.lua:M._cleanup_diff_state] [diff_inline.lua:M.cleanup_inline_diff] [diff_inline.lua:M.setup_inline_diff] - Rejecting a new-file diff that is not in its own tab cleans it up at once when `diff_opts.on_new_file_reject` is `"keep_empty"` (the default), before `close_tab`. A `unified` diff gets no such cleanup [diff.lua:M._resolve_diff_as_rejected] [diff_inline.lua:M.resolve_inline_as_rejected] - `BufDelete`, `BufUnload`, and `BufWipeout` on the proposal buffer reject a pending diff. `WinClosed` rejects it only when no other window in any tab shows the proposal buffer. The original file's buffer is not watched. The unified layout registers the same autocmds [diff.lua:register_diff_autocmds] [diff_inline.lua:M.setup_inline_diff] @@ -202,7 +202,7 @@ Module map (direct files): - The plugin broadcasts `at_mentioned` (`filePath`, `lineStart`, `lineEnd`) [init.lua:M._broadcast_at_mention] and `selection_changed` [selection.lua:M.send_selection_update] to every connected client - While Claude holds a visual selection, and while you select in a buffer (from entering visual mode until the flush), `selection.lua` attaches an `nvim_buf_attach` listener (`on_lines`, `on_reload`, `on_detach`) to that buffer, once per buffer. It does not attach to the Claude terminal buffer, whose output is a stream of edits. It detaches itself at the next `on_lines` after Claude holds no selection of that buffer and you do not select in it, and a reload with `:edit` or `:edit!` detaches it, so it attaches again when the buffer still holds the selection [selection.lua:watch_selection_buffer] - `init.lua` feeds `` after a successful `:ClaudeCodeSend` that does not take the visual path, outside a tree buffer. It also feeds `` when `:ClaudeCode` or `:ClaudeCodeFocus` runs in Visual mode [init.lua:M._create_commands] -- Augroups: `ClaudeCodeShutdown`, which holds the `VimLeavePre` autocmd that stops on exit [init.lua:M.setup], `ClaudeCodeSelection` [selection.lua:M._create_autocommands], `ClaudeCodeMCPDiff` [diff.lua:get_autocmd_group], `ClaudeCodeDiffCleanup` [diff.lua:M._open_native_diff], `ClaudeCodeNativeAdopted` [terminal/native.lua:adopt_existing_terminal], `ClaudeCodeOpenFileNewFiles` [tools/open_file.lua:new_file_buffers], `snacks_win_`, one per Snacks terminal whose split the plugin re-creates [terminal/snacks.lua:register_window_events], and a module-level `VimLeavePre` autocmd in `diff.lua` whose callback calls `reject_pending_diffs()` and then `M._cleanup_all_active_diffs("shutdown")` [diff.lua:cleanup_at_exit] +- Augroups: `ClaudeCodeShutdown`, which holds the `VimLeavePre` autocmd that stops on exit [init.lua:M.setup], `ClaudeCodeSelection` [selection.lua:M._create_autocommands], `ClaudeCodeMCPDiff` [diff.lua:get_autocmd_group], `ClaudeCodeDiffCleanup` [diff.lua:M._open_native_diff], `ClaudeCodeNativeAdopted` [terminal/native.lua:adopt_existing_terminal], `ClaudeCodeOpenFileNewFiles` [tools/open_file.lua:new_file_buffers], `snacks_win_`, one per Snacks terminal whose split the plugin re-creates [terminal/snacks.lua:register_window_events], and a module-level `VimLeavePre` autocmd in `diff.lua` whose callback rejects each pending diff and then calls `M._cleanup_all_active_diffs("shutdown")` [diff.lua:cleanup_at_exit] - Each diff that opens in a new tab adds a buffer-local `BufEnter` autocmd (`startinsert`) to the Claude terminal buffer in `ClaudeCodeMCPDiff`, unless `terminal.auto_insert` is `false`, `diff_opts.hide_terminal_in_new_tab` is `true`, or no valid Claude terminal buffer exists. A `WinClosed` autocmd deletes it when the terminal window it was made for closes [diff.lua:display_terminal_in_new_tab] - Diffs create scratch buffers, splits, and tabs. They resize the terminal window and fire `User` events. Cleanup closes these and reloads unchanged file buffers 100 ms after a save [diff.lua:M.close_diff_by_tab_name] - When a blocking diff resolves, `open_diff_blocking` sends the reply through the global `_G.claude_deferred_responses[tostring(co)]`, then sets that entry to nil. Without an entry, it logs an error [diff.lua:M.open_diff_blocking] diff --git a/lua/claudecode/diff.lua b/lua/claudecode/diff.lua index 1f82f39ce..e6c524a49 100644 --- a/lua/claudecode/diff.lua +++ b/lua/claudecode/diff.lua @@ -2047,12 +2047,19 @@ function M.open_diff_blocking(old_file_path, new_file_path, new_file_contents, t return user_action_result end ----Resolve each diff whose status is `pending` as rejected, oldest first, each under `pcall`, so one diff that fails to ----reject cannot leave the others waiting. A diff with another status is skipped. A reject can remove its own diff ----(`keep_empty`), so a diff can be gone before the next one is rejected. The reject of a diff whose openDiff setup is ----still running stays in `setup_result` (see `open_diff_blocking`) and sends nothing: the call has no response sender ----yet, and the server's `stop` sends its response. -local function reject_pending_diffs() +---Runs at VimLeavePre. 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. +---The reject of a new-file diff with `on_new_file_reject = "keep_empty"`, in a tab that the diff did not open, deletes +---its proposal buffer, and another plugin's buffer can delete itself in its own BufUnload. The diffs behind that +---buffer then reach this point pending. SIGTERM and SIGHUP run VimLeavePre without the loop, so every diff reaches it +---pending. SIGKILL, a crash, or a quit from inside an autocmd that is not `nested` runs no VimLeavePre. +---The reject pass runs first. It resolves each diff whose status is `pending` as rejected, oldest first, each under +---`pcall`, so one diff that fails to reject cannot leave the others waiting. A diff with another status is skipped. A +---reject can remove its own diff (`keep_empty`), so a diff can be gone before the next one is rejected. The reject of a +---diff whose openDiff setup is still running stays in `setup_result` (see `open_diff_blocking`) and sends nothing: the +---call has no response sender yet, and the server's `stop` sends its response. The cleanup pass of every diff runs +---after the reject pass. +local function cleanup_at_exit() local pending = {} for tab_name, diff_data in pairs(active_diffs) do if diff_data.status == "pending" then @@ -2068,17 +2075,6 @@ local function reject_pending_diffs() logger.warn("diff", "Could not reject the pending diff", entry.tab_name, "at exit:", tostring(err)) end end -end - ----Runs at VimLeavePre. 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. ----The reject of a new-file diff with `on_new_file_reject = "keep_empty"`, in a tab that the diff did not open, deletes ----its proposal buffer, and another plugin's buffer can delete itself in its own BufUnload. The diffs behind that ----buffer then reach this point pending. SIGTERM and SIGHUP run VimLeavePre without the loop, so every diff reaches it ----pending. SIGKILL, a crash, or a quit from inside an autocmd that is not `nested` runs no VimLeavePre. The reject ----pass runs first. The cleanup pass of every diff runs after it. -local function cleanup_at_exit() - reject_pending_diffs() M._cleanup_all_active_diffs("shutdown") end diff --git a/lua/claudecode/server/init.lua b/lua/claudecode/server/init.lua index 53731f1f0..96055484e 100644 --- a/lua/claudecode/server/init.lua +++ b/lua/claudecode/server/init.lua @@ -216,8 +216,8 @@ function M._handle_request(client, request) end -- A request has no answer until its handler returns, and a blocking tool has no sender before that. When Neovim - -- exits inside the handler (a `nested = true` ClaudeCodeDiffOpened handler that runs :qa!), the handler never returns, so `stop` - -- answers the request, and this function sends nothing after it. + -- exits inside the handler (a `nested = true` ClaudeCodeDiffOpened handler that runs :qa!), the handler never + -- returns, so `stop` answers the request, and this function sends nothing after it. local in_flight = { client = client, id = id } M.state.in_flight[in_flight] = true local success, result, error_data = pcall(handler, client, params) diff --git a/tests/integration/diff_exit_reject_spec.lua b/tests/integration/diff_exit_reject_spec.lua index 7bacabc19..9f0a4ed2e 100644 --- a/tests/integration/diff_exit_reject_spec.lua +++ b/tests/integration/diff_exit_reject_spec.lua @@ -55,9 +55,11 @@ describe("openDiff at exit", function() return diff.open_diff_blocking(path, path, "new line\n", "tab_" .. name, nil) end) local ok, err = coroutine.resume(co) - log("resume " .. name .. " ok=" .. tostring(ok) .. " status=" .. coroutine.status(co) .. (ok and "" or " " .. tostring(err))) + local resumed = "resume " .. name .. " ok=" .. tostring(ok) .. " status=" .. coroutine.status(co) + log(resumed .. (ok and "" or " " .. tostring(err))) _G.claude_deferred_responses[tostring(co)] = function(response) - log("answer " .. name .. " " .. tostring(response.content and response.content[1].text or vim.inspect(response))) + local text = response.content and response.content[1].text or vim.inspect(response) + log("answer " .. name .. " " .. tostring(text)) end end vim.defer_fn(function() @@ -112,7 +114,12 @@ describe("openDiff at exit", function() method = "tools/call", params = { name = "openDiff", - arguments = { old_file_path = path, new_file_path = path, new_file_contents = "new line\n", tab_name = "tab_" .. name }, + arguments = { + old_file_path = path, + new_file_path = path, + new_file_contents = "new line\n", + tab_name = "tab_" .. name, + }, }, })) end diff --git a/tests/unit/server/in_flight_request_spec.lua b/tests/unit/server/in_flight_request_spec.lua index ab303cfb1..cfd8cda65 100644 --- a/tests/unit/server/in_flight_request_spec.lua +++ b/tests/unit/server/in_flight_request_spec.lua @@ -107,7 +107,8 @@ end) -- The answer must leave before the TCP server stops. After `tcp_server.stop_server` the clients are closing, and after -- `M.state.server = nil` `send_response` sends nothing, so an answer moved there never reaches Claude. The describe --- above replaces `send_response` and `stop_server`, so it cannot see the order. This one records what reaches the socket. +-- above replaces `send_response` and `stop_server`, so it cannot see the order. This one records what reaches the +-- socket. describe("server.stop with a request in flight, as the socket sees it", function() local server, tcp, saved, order