fix(diff): user-initiated accept/reject should clean up windows and buffers#241
fix(diff): user-initiated accept/reject should clean up windows and buffers#241mightreya wants to merge 1 commit into
Conversation
…uffers The user commands :ClaudeCodeDiffAccept and :ClaudeCodeDiffDeny call _resolve_diff_as_saved/_rejected, which only mark the diff status. Real cleanup (close windows, force-delete buffers, remove from active_diffs) is performed by _cleanup_diff_state, currently triggered exclusively by the close_tab MCP tool. But close_tab has schema = nil (it is intentionally internal-only, as documented in the source), so the Claude CLI never invokes it. The result is that user-initiated accept/reject leaves the diff windows and buffers behind, requiring manual cleanup. The fix: have the user commands call _cleanup_diff_state themselves, since they are not waiting for an MCP response from the CLI. Also restore terminal focus when diff_opts.keep_terminal_focus is set (previously this only applied at diff-open time). Adds unit tests covering both commands and the no-marker no-op case. Closes coder#205, partially addresses coder#238.
|
Note This triage report is AI-generated using Claude Code This PR was looked at while triaging #205. The cleanup it adds is a genuine improvement, so this is not a request to close the PR — but the evidence gathered during triage indicates that #205 would not actually be resolved by this change, so decoupling the The change itself looks reasonableHaving The premise behind "Closes #205" does not match the observed behaviourThe PR describes the root cause as A reproduction was set up with Neovim running the plugin ( So the connected client does send In other words, in the connected case the diff teardown already happens over the protocol. The immediate cleanup this PR adds is valuable mainly as a safety net for paths where that round-trip does not occur — for example a diff resolved via the user commands when no client will follow up with Why #205 specifically would remainAcross the scenarios that could be exercised in a headless harness — including the reporter's plugin commit with their Neovim version — diffs were torn down correctly: both the internal window state and the on-screen content were correct, whether or not Neovim held focus. The #205 symptom is a previous diff remaining painted on an unfocused split pane until focus returns, which points at a terminal-side repaint behaviour (consistent with the Ghostty split-pane repaint reports linked on #205). Since this PR changes when internal cleanup runs but adds no redraw, it would not change what an unfocused pane displays. SuggestionThe fix is worth keeping. Re-scoping it to the user-command / #238 cleanup and dropping the fixture used for the protocol observation (provider=none + env-gated MCP-call log)The diff lifecycle was observed by pointing a connected Claude session at this fixture and logging the -- Fixture for issue #205:
-- "[BUG] Diff panels not cleared when new suggestion is shown — stale diffs
-- accumulate until window focus change"
-- https://github.com/coder/claudecode.nvim/issues/205
--
-- Reporter setup: Ghostty tab split into two panes — Claude Code CLI in one,
-- Neovim (LazyVim) in the other. Claude connects to the plugin's WebSocket
-- server (so terminal.provider = "none": Claude lives OUTSIDE Neovim). When
-- Claude proposes a second change, the previous diff panes stay on screen and
-- only disappear once the user focuses the Neovim pane.
--
-- This fixture mirrors that: provider="none", auto_start, plus a window-state
-- probe so a script can compare Neovim's INTERNAL window state against what is
-- actually rendered on screen (the focus-dependent symptom):
-- nvim --server <sock> --remote-expr 'v:lua.Repro205State()'
-- nvim --server <sock> --remote-expr 'v:lua.Repro205DiffWinCount()'
--
-- Usage (from repo root):
-- source fixtures/nvim-aliases.sh && vv issue-205
-- or scripted:
-- scripts/repro_issue_205.sh
local config_dir = vim.fn.stdpath("config")
local repo_root = vim.fn.fnamemodify(config_dir, ":h:h")
vim.opt.rtp:prepend(repo_root)
vim.g.mapleader = " "
vim.g.maplocalleader = "\\"
-- ---------------------------------------------------------------------------
-- claudecode.nvim (dev version from this repo)
-- ---------------------------------------------------------------------------
local ok, claudecode = pcall(require, "claudecode")
assert(ok, "Failed to load claudecode.nvim from repo root: " .. tostring(claudecode))
claudecode.setup({
auto_start = true, -- server + lock file immediately, so scripts can connect
-- "warn", not "debug": multi-line debug echoes trip nvim's hit-enter prompt,
-- which blocks --remote-expr probes in the scripted repro.
log_level = "warn",
terminal = {
-- Reporter runs Claude in a SEPARATE Ghostty pane, not inside Neovim.
provider = "none",
},
})
vim.o.showtabline = 2
vim.o.laststatus = 2
-- ---------------------------------------------------------------------------
-- Optional MCP trace: when $ISSUE205_TRACE is a writable path, log every
-- tools/call the connected client makes (name + tab_name) to that file. Used to
-- verify empirically whether the real Claude CLI sends `close_tab` after a diff
-- resolves (PR #241 claims it does not). Wrapping the module field works because
-- the server looks up handle_invoke on the shared tools table each dispatch.
-- ---------------------------------------------------------------------------
do
local trace_path = vim.env.ISSUE205_TRACE
if trace_path and trace_path ~= "" then
-- Use the EXACT module key the server uses ("claudecode.tools.init"); Lua
-- caches "claudecode.tools" and "claudecode.tools.init" as separate tables,
-- so wrapping the wrong one is a silent no-op.
local ok_tools, tools = pcall(require, "claudecode.tools.init")
if ok_tools and type(tools.handle_invoke) == "function" then
local orig = tools.handle_invoke
tools.handle_invoke = function(client, params)
pcall(function()
local name = params and params.name or "?"
local tab = params and params.arguments and params.arguments.tab_name or ""
local fh = io.open(trace_path, "a")
if fh then
fh:write(string.format("%d\t%s\t%s\n", vim.fn.localtime(), name, tab))
fh:close()
end
end)
return orig(client, params)
end
end
end
end
-- ---------------------------------------------------------------------------
-- Window-state probe (for --remote-expr / on-screen verification)
-- ---------------------------------------------------------------------------
---Compact state of every window across all tabpages.
---@return string JSON: [{win,tab,name,buftype,diff}...]
function _G.Repro205State()
local out = {}
for _, win in ipairs(vim.api.nvim_list_wins()) do
local buf = vim.api.nvim_win_get_buf(win)
local name = vim.api.nvim_buf_get_name(buf)
out[#out + 1] = {
win = win,
tab = vim.api.nvim_tabpage_get_number(vim.api.nvim_win_get_tabpage(win)),
name = vim.fn.fnamemodify(name, ":t") ~= "" and vim.fn.fnamemodify(name, ":~:.") or "[No Name]",
buftype = vim.bo[buf].buftype,
diff = vim.wo[win].diff,
}
end
return vim.json.encode(out)
end
---WebSocket endpoint of the running claudecode server ("port token", or "" if
---not started yet). Lets scripts connect without scanning ~/.claude/ide.
---@return string
function _G.Repro205Server()
local cc = require("claudecode")
if cc.state.port and cc.state.auth_token then
return cc.state.port .. " " .. cc.state.auth_token
end
return ""
end
---Count of windows currently in diff mode (quick assertion helper).
---@return integer
function _G.Repro205DiffWinCount()
local n = 0
for _, win in ipairs(vim.api.nvim_list_wins()) do
if vim.wo[win].diff then
n = n + 1
end
end
return n
end
---Number of distinct diffs the plugin is actively tracking.
---@return integer
function _G.Repro205ActiveDiffs()
local ok_diff, diff = pcall(require, "claudecode.diff")
if not ok_diff or not diff._get_active_diffs then
return -1
end
local n = 0
for _ in pairs(diff._get_active_diffs()) do
n = n + 1
end
return n
end
---Accept every still-pending diff (mirrors the user pressing :w / accepting in
---the IDE). Returns FILE_SAVED to the connected Claude client, which then sends
---close_tab. Used by the scripted end-to-end repro so diffs don't block forever.
---@return integer accepted Number of diffs accepted
function _G.Repro205AcceptAll()
local ok_diff, diff = pcall(require, "claudecode.diff")
if not ok_diff or not diff._get_active_diffs then
return 0
end
local accepted = 0
for tab_name, diff_data in pairs(diff._get_active_diffs()) do
if diff_data.status == "pending" and diff_data.new_buffer and vim.api.nvim_buf_is_valid(diff_data.new_buffer) then
pcall(diff._resolve_diff_as_saved, tab_name, diff_data.new_buffer)
accepted = accepted + 1
end
end
return accepted
end
vim.api.nvim_create_user_command("ReproState", function()
vim.notify(_G.Repro205State())
end, { desc = "Show issue-205 window state" }) |
Problem
:ClaudeCodeDiffAcceptand:ClaudeCodeDiffDeny(theaccept_current_diff/deny_current_diffuser commands) leave the diff buffers, split windows, andactive_diffsstate behind after resolving. Users have to close the leftover windows and buffers by hand.This shows up as the symptom in #205 and as the partial behavior described in #238 (rejecting via
:qcloses one buffer but does nothing else).Root cause
The two user commands call
_resolve_diff_as_saved/_resolve_diff_as_rejected, which only set the diff status and resume the MCP coroutine. The actual UI cleanup lives in_cleanup_diff_stateand is triggered exclusively by theclose_tabMCP tool.But
close_tabhasschema = niland is documented as "Internal tool — must remain as requested by user" (seelua/claudecode/tools/close_tab.lua). The Claude CLI never invokes it. So when a user resolves a diff via the bundled commands, cleanup never fires.This was less visible before #175 because the cleanup function did less work; with #175 expanding cleanup to close plugin-created splits and force-delete buffers, the gap between "resolve" and "cleanup" became user-facing as accumulating diff windows.
Fix
Have the user commands call
_cleanup_diff_statethemselves. They're not waiting for an MCP response from the CLI — they're driven by the user — so the "wait forclose_tab" architecture doesn't apply to them.This PR does not touch
close_tab.lua. Itsschema = nilis left intact per the existing comment.It also extends
diff_opts.keep_terminal_focusto apply at user-resolve time, not only at diff-open time, so focus returns to the Claude terminal after accept/reject.Tests
Adds
tests/unit/diff_user_command_cleanup_spec.luacovering:accept_current_diffcloses the diff window and removes the entry fromactive_diffsdeny_current_diffcloses the diff window and removes the entry fromactive_diffsaccept_current_diffis a safe no-op when called outside a diff bufferI was unable to run
make testlocally (the Nix-based dev shell wasn't available on my machine). I confirmed Lua syntax vialoadfile. Please run the suite in CI / locally before merging.Closes / refs
:qdoes not work #238