Skip to content

Commit b9b99f9

Browse files
ThomasK33claude
andauthored
fix(diff): reject diffs on window close so :q works (#266)
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: Thomas Kosiewski <tk@coder.com>
1 parent 9b69b85 commit b9b99f9

9 files changed

Lines changed: 981 additions & 0 deletions

File tree

.github/workflows/test.yml

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -149,3 +149,9 @@ jobs:
149149
150150
- name: Run integration tests
151151
run: ./scripts/run_integration_tests_individually.sh
152+
153+
# Real-Neovim regression gate for issue #238 (reject-with-:q). The unit spec
154+
# only exercises the WinClosed handler logic against a mock; this drives a
155+
# genuine `:q` and self-reports via cquit (exit 0 = fixed, 1 = regressed).
156+
- name: Reject-on-quit regression gate (#238)
157+
run: nvim --headless -u NONE -l scripts/repro_issue_238.lua

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
### Bug Fixes
1111

1212
- `focus_after_send = true` no longer fails silently with `terminal.provider = "none"`/`"external"`: those providers run Claude outside Neovim, so focus cannot move there. A one-time warning is now emitted at setup pointing to the new `User ClaudeCodeSendComplete` autocmd, which you can hook to focus your own terminal. (`focus_after_send` still only auto-focuses the in-editor providers.) ([#228](https://github.com/coder/claudecode.nvim/issues/228))
13+
- Rejecting a Claude diff with `:q` (or `:close` / `<C-w>c` / closing the tab) now resolves it as rejected, matching the documented behavior. The proposed buffer is a scratch buffer that `:q` only hides, so the existing `BufDelete`/`BufUnload`/`BufWipeout` autocmds never fired; a `WinClosed` autocmd now handles window-close rejection. ([#238](https://github.com/coder/claudecode.nvim/issues/238))
1314
- Diffs opened via `openDiff` no longer linger forever when they are resolved outside this Neovim or their Claude session goes away. Pending diffs are now automatically closed when the client that opened them disconnects or the integration is stopped, and `closeAllDiffTabs` now also resolves/cleans the diff module's tracked state instead of only closing windows. ([#248](https://github.com/coder/claudecode.nvim/issues/248))
1415
- Show diffs when the Claude Code terminal is the only window (no other splits). Previously `openDiff` failed with "No suitable editor window found"; now a split is created to host the diff, matching the behavior of the `openFile` tool. ([#231](https://github.com/coder/claudecode.nvim/issues/231))
1516
- Work around a Neovim core bug (< 0.12.2) that fragmented large bracketed pastes into the terminal across `vim.paste` phases, making Cmd+V appear to truncate content. Added a scoped, version-gated `vim.paste` shim controlled by `terminal.fix_streamed_paste` (`"auto"` by default; no-op on Neovim >= 0.12.2). ([#161](https://github.com/coder/claudecode.nvim/issues/161))

fixtures/issue-238/README.md

Lines changed: 91 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,91 @@
1+
# `issue-238` fixture — repro for issue #238
2+
3+
Reproduces [#238 "[BUG] Rejecting with `:q` does not work"](https://github.com/coder/claudecode.nvim/issues/238):
4+
the README documents two ways to reject a Claude diff — `:q` **or** `<leader>ad`
5+
(`:ClaudeCodeDiffDeny`). The keymap works; **`:q` does not reject**. The proposed
6+
window closes but Claude is never told `DIFF_REJECTED`, and (with
7+
`open_in_new_tab = true`) the diff tab lingers.
8+
9+
This fixture uses the reporter's exact config:
10+
11+
- `terminal.provider = "none"` (Claude runs in an external terminal, e.g.
12+
sidekick.nvim, so claudecode manages no terminal of its own), and
13+
- `diff_opts = { layout = "vertical", open_in_new_tab = true }`.
14+
15+
It mirrors [`remote-diff`](../remote-diff) but adds a JSON `:DiffStateFile`
16+
inspector that records window/tab counts, each diff's status, **and the proposed
17+
buffer's `bufhidden`** — the smoking gun for this bug.
18+
19+
> Set `REPRO238_NEW_TAB=0` to launch in the **default** same-tab layout and
20+
> confirm the bug is not tab-specific (it reproduces there too — the split just
21+
> silently collapses to the original file).
22+
23+
## Files
24+
25+
- `init.lua` — claudecode.nvim config matching the issue + `:DiffState`/`:DiffStateFile`.
26+
- `example/target.txt` — a sample file to diff against.
27+
28+
## Root cause (verified)
29+
30+
The proposed buffer is created with `vim.api.nvim_create_buf(false, true)`
31+
(a scratch buffer ⇒ `bufhidden = "hide"`) and rejection is wired **only** through
32+
buffer-destruction autocmds (`BufDelete` / `BufUnload` / `BufWipeout`
33+
`_resolve_diff_as_rejected`). Because `bufhidden = "hide"`, `:q` merely **hides**
34+
the still-loaded buffer instead of destroying it, so none of those autocmds fire
35+
and the diff is never resolved. (`:ClaudeCodeDiffDeny` works because it calls
36+
`_resolve_diff_as_rejected` directly, bypassing the autocmds.)
37+
38+
## Quick start — headless one-liner (no WebSocket needed)
39+
40+
The fastest way to see the bug. Drives the real `diff.lua` and performs a genuine
41+
`:q`, for both `open_in_new_tab` layouts:
42+
43+
```sh
44+
nvim --headless -u NONE -l scripts/repro_issue_238.lua; echo "exit: $?"
45+
```
46+
47+
Exit code **1** = bug reproduced (current code), **0** = fixed. On current code it prints:
48+
49+
```
50+
[default config (open_in_new_tab=false)]
51+
proposed buffer bufhidden = "hide"
52+
after :q -> rejected=false status=pending proposed_buf_still_loaded=true tabpages 1->1
53+
=> BUG: `:q` did NOT reject the diff (Claude never receives DIFF_REJECTED)
54+
[reporter config (open_in_new_tab=true)]
55+
proposed buffer bufhidden = "hide"
56+
after :q -> rejected=false status=pending proposed_buf_still_loaded=true tabpages 2->2
57+
=> BUG: ...
58+
```
59+
60+
## Quick start — live, playing the role of Claude over MCP
61+
62+
```sh
63+
# Terminal 1 — the editor under test:
64+
source fixtures/nvim-aliases.sh
65+
vv issue-238 example/target.txt
66+
# (equivalently: NVIM_APPNAME=issue-238 XDG_CONFIG_HOME=fixtures nvim fixtures/issue-238/example/target.txt)
67+
# The server auto-starts; check the lock file: ls ~/.claude/ide/*.lock
68+
69+
# Terminal 2 — open a diff and HOLD the socket open while you reject in Neovim:
70+
scripts/repro_issue_238.sh --file "$PWD/fixtures/issue-238/example/target.txt" --hold 30
71+
```
72+
73+
A diff opens in a new tab. In Neovim, try to reject it with **`:q`**. Then:
74+
75+
- Run `:DiffState` in Neovim → it still shows the diff as `[pending]`
76+
(`bufhidden=hide`), and the tab is still open.
77+
- The Terminal-2 script reports `no DIFF_REJECTED / FILE_SAVED in window` — Claude
78+
never learned the diff was rejected.
79+
80+
Contrast: reject with `:ClaudeCodeDiffDeny` (or `<leader>ad`) instead → the script
81+
prints `DIFF_REJECTED was received` and `:DiffState` shows the diff `[rejected]`.
82+
83+
> If `websocat` is a `mise` shim that refuses to run in this directory, pass the
84+
> real binary: `WEBSOCAT="$(mise which websocat)" scripts/repro_issue_238.sh ...`.
85+
86+
## Inspector commands (added by this fixture)
87+
88+
- `:DiffState` — notify window/tab count + each active diff's status, `created_new_tab`, and proposed `bufhidden`.
89+
- `:DiffStateFile [path]` — write the same info as JSON (for automation; defaults to `stdpath('cache')/diff_state.json`).
90+
- `<leader>as` — run `:DiffState`.
91+
- `<leader>aa` / `<leader>ad` — accept / deny the focused diff.
Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
line one
2+
line two
3+
line three
4+
line four
5+
line five

fixtures/issue-238/init.lua

Lines changed: 143 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,143 @@
1+
-- Repro fixture for issue #238: "[BUG] Rejecting with `:q` does not work".
2+
--
3+
-- Scenario this fixture is built to demonstrate:
4+
-- 1. claudecode.nvim is configured exactly like the reporter:
5+
-- - terminal.provider = "none" (Claude runs in an *external* terminal,
6+
-- e.g. sidekick.nvim — Neovim manages no terminal of its own)
7+
-- - diff_opts.open_in_new_tab = true
8+
-- - diff_opts.layout = "vertical"
9+
-- 2. Claude opens a diff via the `openDiff` MCP tool. It lands in a NEW tab
10+
-- with the original file on the left and the proposed buffer on the right.
11+
-- 3. The user tries to REJECT the change with `:q` (as the README documents:
12+
-- "Reject: `:q` or <leader>ad").
13+
-- 4. EXPECTED: the diff is rejected (Claude is told DIFF_REJECTED) and the
14+
-- tab closes.
15+
-- ACTUAL (the bug): `:q` only closes the proposed window; the buffer is
16+
-- merely *hidden* (it is a scratch buffer => bufhidden=hide), so none of
17+
-- the BufDelete/BufUnload/BufWipeout autocmds that drive rejection fire.
18+
-- The diff stays "pending" forever and the tab lingers.
19+
--
20+
-- This fixture mirrors `remote-diff` but uses the reporter's exact config and
21+
-- exposes a `:DiffStateFile` command that writes a machine-readable JSON
22+
-- snapshot (window/tab counts, per-diff status, and the proposed buffer's
23+
-- bufhidden) so automation can assert on the bug without scraping the screen.
24+
--
25+
-- Usage (from repo root):
26+
-- source fixtures/nvim-aliases.sh
27+
-- vv issue-238 example/target.txt
28+
-- # or: NVIM_APPNAME=issue-238 XDG_CONFIG_HOME=fixtures nvim fixtures/issue-238/example/target.txt
29+
--
30+
-- Then drive the MCP side (play the role of Claude) with:
31+
-- scripts/repro_issue_238.sh
32+
33+
local config_dir = vim.fn.stdpath("config")
34+
local repo_root = vim.fn.fnamemodify(config_dir, ":h:h")
35+
vim.opt.rtp:prepend(repo_root)
36+
37+
vim.g.mapleader = " "
38+
vim.g.maplocalleader = "\\"
39+
40+
local ok, claudecode = pcall(require, "claudecode")
41+
assert(ok, "Failed to load claudecode.nvim from repo root: " .. tostring(claudecode))
42+
43+
-- The reporter's exact config uses open_in_new_tab = true, but the underlying
44+
-- bug is not tab-specific. Set REPRO238_NEW_TAB=0 to probe the default
45+
-- (same-tab) layout and confirm `:q` rejection is broken there too.
46+
local open_in_new_tab = os.getenv("REPRO238_NEW_TAB") ~= "0"
47+
48+
claudecode.setup({
49+
auto_start = false,
50+
-- Quiet logging keeps the diff UI clean for screenshots / automation and
51+
-- avoids the hit-enter prompt that long :messages can trigger.
52+
log_level = "warn",
53+
terminal = {
54+
-- The reporter uses sidekick.nvim to run Claude in an external terminal,
55+
-- so claudecode itself manages no terminal: provider = "none".
56+
provider = "none",
57+
},
58+
diff_opts = {
59+
layout = "vertical",
60+
open_in_new_tab = open_in_new_tab,
61+
keep_terminal_focus = false,
62+
},
63+
})
64+
65+
local function ensure_started()
66+
local ok_start, started_or_err, port_or_err = pcall(function()
67+
return claudecode.start(false)
68+
end)
69+
if not ok_start then
70+
vim.notify("ClaudeCode start crashed: " .. tostring(started_or_err), vim.log.levels.ERROR)
71+
return false
72+
end
73+
if started_or_err or port_or_err == "Already running" then
74+
return true
75+
end
76+
vim.notify("ClaudeCode failed to start: " .. tostring(port_or_err), vim.log.levels.ERROR)
77+
return false
78+
end
79+
80+
ensure_started()
81+
82+
-- Build a snapshot of everything that matters for this bug.
83+
local function diff_state()
84+
local diff = require("claudecode.diff")
85+
local active = diff._get_active_diffs()
86+
87+
local diffs = {}
88+
for tab_name, data in pairs(active) do
89+
local proposed_bufhidden = nil
90+
if data.new_buffer and vim.api.nvim_buf_is_valid(data.new_buffer) then
91+
proposed_bufhidden = vim.api.nvim_buf_get_option(data.new_buffer, "bufhidden")
92+
end
93+
diffs[#diffs + 1] = {
94+
tab_name = tab_name,
95+
status = data.status or "?",
96+
created_new_tab = data.created_new_tab or false,
97+
new_buffer = data.new_buffer,
98+
new_buffer_valid = data.new_buffer and vim.api.nvim_buf_is_valid(data.new_buffer) or false,
99+
new_buffer_loaded = data.new_buffer and vim.api.nvim_buf_is_loaded(data.new_buffer) or false,
100+
proposed_bufhidden = proposed_bufhidden,
101+
}
102+
end
103+
table.sort(diffs, function(a, b)
104+
return tostring(a.tab_name) < tostring(b.tab_name)
105+
end)
106+
107+
return {
108+
windows = #vim.api.nvim_list_wins(),
109+
tabpages = #vim.api.nvim_list_tabpages(),
110+
active_diffs = #diffs,
111+
diffs = diffs,
112+
}
113+
end
114+
115+
-- Human-readable variant.
116+
vim.api.nvim_create_user_command("DiffState", function()
117+
local s = diff_state()
118+
local lines = {
119+
("windows=%d tabpages=%d active_diffs=%d"):format(s.windows, s.tabpages, s.active_diffs),
120+
}
121+
for _, d in ipairs(s.diffs) do
122+
lines[#lines + 1] = (" [%s] new_tab=%s bufhidden=%s loaded=%s %s"):format(
123+
d.status,
124+
tostring(d.created_new_tab),
125+
tostring(d.proposed_bufhidden),
126+
tostring(d.new_buffer_loaded),
127+
d.tab_name
128+
)
129+
end
130+
vim.notify(table.concat(lines, "\n"), vim.log.levels.INFO)
131+
end, { desc = "Show window/tab count + active claudecode diffs" })
132+
133+
-- Scriptable variant: writes the state as JSON to a file so external automation
134+
-- can assert on it without scraping the message area.
135+
vim.api.nvim_create_user_command("DiffStateFile", function(opts)
136+
local path = opts.args ~= "" and opts.args or (vim.fn.stdpath("cache") .. "/diff_state.json")
137+
local s = diff_state()
138+
vim.fn.writefile({ vim.json.encode(s) }, path)
139+
end, { nargs = "?", desc = "Write window/diff state as JSON to a file" })
140+
141+
vim.keymap.set("n", "<leader>aa", "<cmd>ClaudeCodeDiffAccept<cr>", { desc = "Accept diff" })
142+
vim.keymap.set("n", "<leader>ad", "<cmd>ClaudeCodeDiffDeny<cr>", { desc = "Deny diff" })
143+
vim.keymap.set("n", "<leader>as", "<cmd>DiffState<cr>", { desc = "Show diff state" })

lua/claudecode/diff.lua

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -867,6 +867,39 @@ local function register_diff_autocmds(tab_name, new_buffer)
867867
end,
868868
})
869869

870+
-- WinClosed: reject when the proposed window is closed (`:q`, `:close`, `<C-w>c`, `:tabclose`).
871+
-- The proposed buffer is scratch (bufhidden="hide"), so closing its window merely HIDES the
872+
-- still-loaded buffer and none of BufDelete/BufUnload/BufWipeout below fire -> `:q` would never
873+
-- resolve the diff and Claude would never receive DIFF_REJECTED (issue #238).
874+
--
875+
-- We do NOT bind to a single window-id pattern: the proposed buffer may be split into several
876+
-- windows (<C-w>v), and rejecting just because the *tracked* window closed would prematurely
877+
-- reject while the user still views a clone (and closing a clone would never match the pattern).
878+
-- Instead we reject only once the proposed buffer is displayed in NO window (across all tabs;
879+
-- a copy the user split into another tab defers rejection until that copy is closed too).
880+
-- WinClosed fires BEFORE the closing window leaves the layout, so we exclude it (args.match).
881+
-- _resolve_diff_as_rejected no-ops once status != "pending", so this is harmless after :w
882+
-- (accept) or during _cleanup_diff_state (which deletes these autocmds before closing windows).
883+
autocmd_ids[#autocmd_ids + 1] = vim.api.nvim_create_autocmd("WinClosed", {
884+
group = get_autocmd_group(),
885+
callback = function(args)
886+
if not vim.api.nvim_buf_is_valid(new_buffer) then
887+
return
888+
end
889+
local closing_win = tonumber(args.match)
890+
local still_visible = false
891+
for _, win in ipairs(vim.fn.win_findbuf(new_buffer)) do
892+
if win ~= closing_win then
893+
still_visible = true
894+
break
895+
end
896+
end
897+
if not still_visible then
898+
M._resolve_diff_as_rejected(tab_name)
899+
end
900+
end,
901+
})
902+
870903
-- Buffer deletion monitoring for rejection (multiple events to catch all deletion methods)
871904

872905
-- BufDelete: When buffer is deleted with :bdelete, :bwipeout, etc.
@@ -902,6 +935,9 @@ local function register_diff_autocmds(tab_name, new_buffer)
902935
return autocmd_ids
903936
end
904937

938+
-- Exposed for testing the reject-on-window-close (WinClosed) behavior.
939+
M._register_diff_autocmds = register_diff_autocmds
940+
905941
---Create diff view from a specific window
906942
---@param target_window NvimWin|nil The window to use as base for the diff
907943
---@param old_file_path string Path to the original file

0 commit comments

Comments
 (0)