Skip to content

Commit 4162f60

Browse files
committed
fix(diff): user-initiated accept/reject should clean up windows and buffers
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 #205, partially addresses #238.
1 parent 432121f commit 4162f60

2 files changed

Lines changed: 140 additions & 3 deletions

File tree

lua/claudecode/diff.lua

Lines changed: 22 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1442,8 +1442,22 @@ function M.reload_file_buffers_manual(file_path, original_cursor_pos)
14421442
return reload_file_buffers(file_path, original_cursor_pos)
14431443
end
14441444

1445+
-- Restore focus to the Claude terminal after a user-initiated resolve, when configured.
1446+
local function restore_terminal_focus_after_user_resolve()
1447+
if not (config and config.diff_opts and config.diff_opts.keep_terminal_focus) then
1448+
return
1449+
end
1450+
local terminal_win = find_claudecode_terminal_window()
1451+
if terminal_win and vim.api.nvim_win_is_valid(terminal_win) then
1452+
pcall(vim.api.nvim_set_current_win, terminal_win)
1453+
pcall(vim.cmd, "startinsert")
1454+
end
1455+
end
1456+
14451457
---Accept the current diff (user command version)
1446-
---This function reads the diff context from buffer variables
1458+
---This function reads the diff context from buffer variables.
1459+
---Unlike Claude-initiated resolve (which waits for the close_tab MCP tool call),
1460+
---user commands clean up the diff UI immediately since close_tab is internal-only.
14471461
function M.accept_current_diff()
14481462
local current_buffer = vim.api.nvim_get_current_buf()
14491463
local tab_name = vim.b[current_buffer].claudecode_diff_tab_name
@@ -1454,10 +1468,14 @@ function M.accept_current_diff()
14541468
end
14551469

14561470
M._resolve_diff_as_saved(tab_name, current_buffer)
1471+
M._cleanup_diff_state(tab_name, "accepted via :ClaudeCodeDiffAccept")
1472+
restore_terminal_focus_after_user_resolve()
14571473
end
14581474

14591475
---Deny/reject the current diff (user command version)
1460-
---This function reads the diff context from buffer variables
1476+
---This function reads the diff context from buffer variables.
1477+
---Unlike Claude-initiated resolve (which waits for the close_tab MCP tool call),
1478+
---user commands clean up the diff UI immediately since close_tab is internal-only.
14611479
function M.deny_current_diff()
14621480
local current_buffer = vim.api.nvim_get_current_buf()
14631481
local tab_name = vim.b[current_buffer].claudecode_diff_tab_name
@@ -1467,8 +1485,9 @@ function M.deny_current_diff()
14671485
return
14681486
end
14691487

1470-
-- Do not close windows/tabs here; just mark as rejected.
14711488
M._resolve_diff_as_rejected(tab_name)
1489+
M._cleanup_diff_state(tab_name, "rejected via :ClaudeCodeDiffDeny")
1490+
restore_terminal_focus_after_user_resolve()
14721491
end
14731492

14741493
return M
Lines changed: 118 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,118 @@
1+
require("tests.busted_setup")
2+
3+
local function reset_vim_state()
4+
assert(vim and vim._mock and vim._mock.reset, "Expected vim mock with _mock.reset()")
5+
6+
vim._mock.reset()
7+
8+
vim._tabs = { [1] = true }
9+
vim._current_tabpage = 1
10+
vim._current_window = 1000
11+
vim._next_winid = 1001
12+
13+
vim._mock.add_buffer(1, "/home/user/project/test.lua", "local test = {}\nreturn test", { modified = false })
14+
vim._mock.add_window(1000, 1, { 1, 0 })
15+
vim._win_tab[1000] = 1
16+
vim._tab_windows[1] = { 1000 }
17+
end
18+
19+
describe("Diff user command cleanup (accept/deny)", function()
20+
local diff
21+
local test_old_file = "/tmp/test_user_command_cleanup_old.txt"
22+
local tab_name = "test_user_command_cleanup_tab"
23+
24+
before_each(function()
25+
reset_vim_state()
26+
27+
local f = assert(io.open(test_old_file, "w"))
28+
f:write("line1\nline2\n")
29+
f:close()
30+
31+
package.loaded["claudecode.logger"] = {
32+
debug = function() end,
33+
error = function() end,
34+
info = function() end,
35+
warn = function() end,
36+
}
37+
38+
package.loaded["claudecode.diff"] = nil
39+
diff = require("claudecode.diff")
40+
41+
diff.setup({
42+
diff_opts = {
43+
layout = "vertical",
44+
open_in_new_tab = false,
45+
keep_terminal_focus = false,
46+
},
47+
terminal = {},
48+
})
49+
end)
50+
51+
after_each(function()
52+
os.remove(test_old_file)
53+
if diff and diff._cleanup_all_active_diffs then
54+
diff._cleanup_all_active_diffs("test teardown")
55+
end
56+
package.loaded["claudecode.diff"] = nil
57+
end)
58+
59+
it("accept_current_diff cleans up windows and removes diff state", function()
60+
local params = {
61+
old_file_path = test_old_file,
62+
new_file_path = test_old_file,
63+
new_file_contents = "new1\nnew2\n",
64+
tab_name = tab_name,
65+
}
66+
67+
diff._setup_blocking_diff(params, function() end)
68+
69+
local state = diff._get_active_diffs()[tab_name]
70+
assert.is_table(state)
71+
72+
local new_win = state.new_window
73+
local new_buffer = state.new_buffer
74+
75+
-- Make the proposed buffer current so accept_current_diff can read the
76+
-- buffer-local tab_name marker.
77+
vim.api.nvim_set_current_buf(new_buffer)
78+
79+
diff.accept_current_diff()
80+
81+
-- After accept, windows must be closed and state must be cleared.
82+
assert.is_false(vim.api.nvim_win_is_valid(new_win))
83+
assert.is_nil(diff._get_active_diffs()[tab_name])
84+
end)
85+
86+
it("deny_current_diff cleans up windows and removes diff state", function()
87+
local params = {
88+
old_file_path = test_old_file,
89+
new_file_path = test_old_file,
90+
new_file_contents = "new1\nnew2\n",
91+
tab_name = tab_name,
92+
}
93+
94+
diff._setup_blocking_diff(params, function() end)
95+
96+
local state = diff._get_active_diffs()[tab_name]
97+
assert.is_table(state)
98+
99+
local new_win = state.new_window
100+
local new_buffer = state.new_buffer
101+
102+
vim.api.nvim_set_current_buf(new_buffer)
103+
104+
diff.deny_current_diff()
105+
106+
assert.is_false(vim.api.nvim_win_is_valid(new_win))
107+
assert.is_nil(diff._get_active_diffs()[tab_name])
108+
end)
109+
110+
it("accept_current_diff is a no-op when buffer has no diff marker", function()
111+
-- No diff registered; current buffer is a plain non-diff buffer.
112+
diff.accept_current_diff()
113+
-- Should not raise; no diffs should be present.
114+
local diffs = diff._get_active_diffs()
115+
assert.is_table(diffs)
116+
assert.is_nil(next(diffs))
117+
end)
118+
end)

0 commit comments

Comments
 (0)