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. diff --git a/.agents/skills/verify-diff-lifecycle/SKILL.md b/.agents/skills/verify-diff-lifecycle/SKILL.md index 54a65b428..a12b63494 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 @@ -54,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`, `_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`. @@ -76,6 +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` 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` | | `UserCloseAllPendingOnly` | `:ClaudeCodeCloseAllDiffs` calls `close_pending_diffs`, never `close_all_diffs` | @@ -119,8 +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`: 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. - 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. @@ -134,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 4e980b15f..418a4ada7 100755 --- a/.agents/skills/verify-diff-lifecycle/check.sh +++ b/.agents/skills/verify-diff-lifecycle/check.sh @@ -50,6 +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 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 +138,26 @@ 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 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 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) +# 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 \ @@ -199,7 +220,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 @@ -208,7 +229,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 [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 00573853f..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 ac01bc69a313e411 +file lua/claudecode/diff.lua a1c57f34e79b17ce block resolve_saved 4d34b0c7e34d561f block resolve_rejected c7a620816abff9a1 block open_blocking 92067df7f9d4d4d9 @@ -20,6 +20,7 @@ block put_in_buffer c5dedfca8261379b block held_for_other 3d1e2911d5310cc2 block close_active 446fbed632748300 block cleanup_all 43e7b595cf106229 +block exit_cleanup 8bc9fbded77d818f block accept_current 7577efa33506569e block fire_event ef778bb39419d91a block register_state 8cf78b6711a9ecc6 @@ -28,7 +29,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 +63,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 @ 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: 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 +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 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() @@ -146,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 8da8851fff781946 -block server_stop 4dae0bf675cf2cc1 +file lua/claudecode/server/init.lua dbc9b57cf9a7142b +block server_stop 506e2e53358dc947 block on_disconnect a0ce86cede4af786 -block handle_request b74ff7dc2e55a2b8 +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 68632c42d..e6b36012f 100644 --- a/.agents/skills/verify-diff-lifecycle/model/DiffLifecycle.tla +++ b/.agents/skills/verify-diff-lifecycle/model/DiffLifecycle.tla @@ -33,6 +33,9 @@ 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 + 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 @@ -56,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 @@ -191,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]] @@ -339,6 +343,43 @@ ServerStops == SetW([v EXCEPT !.sender = [q \in Reqs |-> FALSE]]) /\ UNCHANGED <> +(* 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) == + 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: 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 *) +(* 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 <> + +(* 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) @@ -352,6 +393,8 @@ Next == \/ \E c \in Clients : ClientIdReused(c) \/ \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 == @@ -374,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 0bab1aaf1..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}`; @@ -43,5 +45,8 @@ 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)", + ExitInSetup: + "Neovim exits inside a ClaudeCodeDiffOpened handler, during setup", }; export const step = (action: string) => labels[action] ?? action; diff --git a/.agents/skills/verify-mention-queue/covered.lock b/.agents/skills/verify-mention-queue/covered.lock index 5ab4c37c5..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 8da8851fff781946 +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 29c192c7b..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 8da8851fff781946 +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 0031d3ed5..856d97103 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, 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 each diff that is pending, oldest first, each under pcall + 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. 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]: ```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. 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 ``` @@ -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 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 acfb4ca5b..bea1d8508 100644 --- a/README.md +++ b/README.md @@ -1140,16 +1140,32 @@ 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], 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. + `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 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: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` + 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` + 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). ### Global Side Effects diff --git a/lua/claudecode/README.md b/lua/claudecode/README.md index c78570306..ddc5ede98 100644 --- a/lua/claudecode/README.md +++ b/lua/claudecode/README.md @@ -182,6 +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 `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] @@ -201,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` that calls `M._cleanup_all_active_diffs("shutdown")` [diff.lua:M._cleanup_all_active_diffs]. That cleanup does not resolve pending diffs. On exit Neovim fires `BufUnload` for each loaded buffer before `VimLeavePre`, and the proposal buffer's `BufUnload` autocmd rejects its pending diff [diff.lua:register_diff_autocmds], so with one pending diff none is left for this cleanup. With two or more pending native diffs, a new-file diff in the current tab with `diff_opts.on_new_file_reject = "keep_empty"` (the default) breaks this: its reject deletes its proposal buffer [diff.lua:M._cleanup_diff_state], Neovim stops firing `BufUnload` for the later buffers, and this cleanup removes the diffs that are still pending with no answer to Claude (Neovim 0.8.0 to 0.12.3). The `unified` layout is not affected +- 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 dc2efb9c1..e6c524a49 100644 --- a/lua/claudecode/diff.lua +++ b/lua/claudecode/diff.lua @@ -2047,12 +2047,41 @@ function M.open_diff_blocking(old_file_path, new_file_path, new_file_contents, t return user_action_result 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. 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 + 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 + 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/lua/claudecode/server/README.md b/lua/claudecode/server/README.md index c1c0e331e..007139e75 100644 --- a/lua/claudecode/server/README.md +++ b/lua/claudecode/server/README.md @@ -18,9 +18,9 @@ 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, 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` 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] @@ -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)`. With an auth token it first logs `Authenticated WebSocket client connected` at debug level, because the handshake has checked the token by then. `on_connect` runs at the TCP accept, before the handshake and the token check, and only logs `WebSocket client connected, awaiting authentication` (`WebSocket client connected (no auth)` without a token) [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 7e7a0d6e6..96055484e 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 @@ -141,6 +143,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 @@ -203,7 +215,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 `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) + 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 new file mode 100644 index 000000000..9f0a4ed2e --- /dev/null +++ b/tests/integration/diff_exit_reject_spec.lua @@ -0,0 +1,295 @@ +-- 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 +-- 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 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() + 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") + 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("^(.-):(.*)$") + 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) + 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) + local text = response.content and response.content[1].text or vim.inspect(response) + log("answer " .. name .. " " .. tostring(text)) + 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) + ]] + + -- 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(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", + "-u", + "NONE", + "-i", + "NONE", + "--cmd", + "set rtp^=" .. vim.fn.getcwd(), + "-c", + "luafile " .. vim.fn.fnameescape(script), + }, { 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 + vim.fn.jobstop(job) + end + local lines = vim.fn.filereadable(log_path) == 1 and vim.fn.readfile(log_path) or {} + 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 + 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 + 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 + 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_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 new file mode 100644 index 000000000..dc356959d --- /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). 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() + local diff, dir, answers + + ---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 + 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 = {} + _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 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() + + local names = {} + for _, answer in ipairs(answers) do + names[#names + 1] = answer.name + end + assert.same({ "c.txt", "a.txt", "b.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) 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..cfd8cda65 --- /dev/null +++ b/tests/unit/server/in_flight_request_spec.lua @@ -0,0 +1,143 @@ +-- 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, 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() + 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) + +-- 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)