Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
### Bug Fixes

- 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))
- 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))
- 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))

## [0.3.0] - 2025-09-15
Expand Down
48 changes: 40 additions & 8 deletions lua/claudecode/diff.lua
Original file line number Diff line number Diff line change
Expand Up @@ -1063,6 +1063,12 @@ function M._cleanup_diff_state(tab_name, reason)
pcall(vim.api.nvim_win_close, diff_data.new_window, true)
end

-- Close the fallback window we created when no editor window existed (issue #231); it's reused
-- as the original pane, so target_window_created_by_plugin below doesn't cover it.
if diff_data.fallback_window and vim.api.nvim_win_is_valid(diff_data.fallback_window) then
pcall(vim.api.nvim_win_close, diff_data.fallback_window, false)
end

-- If we created an extra window/split for the diff, close it. Otherwise just disable diff mode.
if diff_data.target_window and vim.api.nvim_win_is_valid(diff_data.target_window) then
if diff_data.target_window_created_by_plugin then
Expand Down Expand Up @@ -1132,6 +1138,12 @@ function M._setup_blocking_diff(params, resolution_callback)
local tab_name = params.tab_name
logger.debug("diff", "Setting up diff for:", params.old_file_path)

-- Hoisted so the error handler can clean them up if setup fails before the diff state is
-- registered: otherwise the terminal-only fallback split and the proposed buffer are stranded
-- (the state-based cleanup is gated on a registered diff). Issue #231.
local fallback_window = nil
local new_buffer = nil

-- Wrap the setup in error handling to ensure cleanup on failure
local setup_success, setup_error = pcall(function()
local old_file_exists = vim.fn.filereadable(params.old_file_path) == 1
Expand Down Expand Up @@ -1197,17 +1209,26 @@ function M._setup_blocking_diff(params, resolution_callback)
target_window = find_main_editor_window()
end
end
-- If created_new_tab is true, target_window stays nil and will be created in the new tab
-- If we still can't find a suitable window AND we're not in a new tab, error out
-- If created_new_tab is true, target_window stays nil and will be created in the new tab.
-- Otherwise, if no editor window is suitable (e.g. the Claude terminal is the only window --
-- issue #231), create one by splitting the current window instead of erroring out, mirroring
-- the fallback in lua/claudecode/tools/open_file.lua.
if not target_window and not created_new_tab then
error({
code = -32000,
message = "No suitable editor window found",
data = "Could not find a main editor window to display the diff",
})
create_split()
local scratch_buf = vim.api.nvim_create_buf(false, true) -- unlisted, scratch
Comment thread
ThomasK33 marked this conversation as resolved.
if scratch_buf ~= 0 then
-- wipe it once it leaves the window so it isn't leaked when the diff reuses it (new file)
-- or :edit replaces it with the real file (existing file)
vim.api.nvim_buf_set_option(scratch_buf, "bufhidden", "wipe")
vim.api.nvim_win_set_buf(vim.api.nvim_get_current_win(), scratch_buf)
end
target_window = vim.api.nvim_get_current_win()
-- Track it so _cleanup_diff_state closes it; the reused scratch buffer means it won't be
-- flagged target_window_created_by_plugin.
fallback_window = target_window
Comment thread
claude[bot] marked this conversation as resolved.
end

local new_buffer = vim.api.nvim_create_buf(false, true) -- unlisted, scratch
new_buffer = vim.api.nvim_create_buf(false, true) -- unlisted, scratch (hoisted above the pcall)
if new_buffer == 0 then
error({
code = -32000,
Expand Down Expand Up @@ -1253,6 +1274,7 @@ function M._setup_blocking_diff(params, resolution_callback)
new_window = diff_info.new_window,
target_window = diff_info.target_window,
target_window_created_by_plugin = diff_info.target_window_created_by_plugin,
fallback_window = fallback_window,
original_buffer = diff_info.original_buffer,
original_buffer_created_by_plugin = diff_info.original_buffer_created_by_plugin,
original_cursor_pos = original_cursor_pos,
Expand Down Expand Up @@ -1288,6 +1310,16 @@ function M._setup_blocking_diff(params, resolution_callback)
-- Clean up any partial state that might have been created
if active_diffs[tab_name] then
M._cleanup_diff_state(tab_name, "setup failed")
else
-- Errored before the diff state was registered, so the state-based cleanup can't run. Close
-- the fallback split we may have created (its bufhidden=wipe scratch self-cleans) and delete
-- the proposed buffer; neither is owned by a registered diff.
if fallback_window and vim.api.nvim_win_is_valid(fallback_window) then
pcall(vim.api.nvim_win_close, fallback_window, true)
end
if new_buffer and vim.api.nvim_buf_is_valid(new_buffer) then
pcall(vim.api.nvim_buf_delete, new_buffer, { force = true })
end
end
Comment thread
ThomasK33 marked this conversation as resolved.
Comment on lines +1313 to 1323

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Two more resources slip past the new pre-registration error handler at lua/claudecode/diff.lua:1313-1323, completing the symmetric pattern from 17626a3 / bf30c36 / #262: (1) fallback_window is assigned too late within the fallback blockcreate_split() runs at line 1217 but fallback_window = target_window is only assigned at line 1228, so any throw from nvim_buf_set_option (line 1222) or nvim_win_set_buf (line 1223) leaves the split with fallback_window still nil and the cleanup gate skipping it. (2) new_window from setup_new_buffer is unreachable from the error handlerdiff_info is bound to a local inside the pcall closure (line 1252), so if anything in setup_new_buffer throws after create_split() at line 584 (most realistically vim.cmd('diffthis') at line 592 propagating a DiffUpdated/OptionSet error, or the FileType autocmd from set_option_value at line 590), force-deleting new_buffer swaps in a replacement buffer and the proposed pane is left stranded next to the terminal. Same one-line-hoist fix as the three prior cases; could be folded into #262.

Extended reasoning...

The two missed resources

The pre-registration error handler added in 17626a3 and extended in bf30c36 currently handles fallback_window and new_buffer. Two siblings in the same code path are still missed.

(1) fallback_window is assigned too late within the fallback block

if not target_window and not created_new_tab then
  create_split()                                                          -- (A) split created
  local scratch_buf = vim.api.nvim_create_buf(false, true)                -- (B)
  if scratch_buf ~= 0 then
    vim.api.nvim_buf_set_option(scratch_buf, "bufhidden", "wipe")        -- (C)
    vim.api.nvim_win_set_buf(vim.api.nvim_get_current_win(), scratch_buf) -- (D)
  end
  target_window = vim.api.nvim_get_current_win()                          -- (E)
  fallback_window = target_window                                         -- (F)
end

If (C) or (D) throws (via a vimscript :throw from BufLeave on the terminal duplicate, BufWinEnter on the scratch, or OptionSet 'bufhidden'), the split from (A) exists but fallback_window is still nil. The error handler's if fallback_window gate at line 1317 short-circuits and the split is stranded next to the terminal. bufhidden=wipe on the scratch doesn't save us — wipe fires on the transition "in some window → in no window", and in the throw path the scratch may never have been associated with a window at all.

(2) new_window from setup_new_buffer is unreachable from the error handler

diff_info (containing new_window from setup_new_buffer) is bound to a local inside the pcall closure at line 1252. The else branch at line 1313 lives outside that closure and can't see it — even though new_window exists from line 585 onward and the pcall can throw any time between that and _register_diff_state at line 1269.

When the handler then runs nvim_buf_delete(new_buffer, {force=true}) at line 1321, Neovim does not close the window holding the buffer — it swaps in a replacement buffer (alternate or fresh unnamed). new_window persists, leaving a stranded empty pane next to the terminal (in diff mode if the throw fired after diffthis, with diff mode off otherwise).

Addressing the refutations

The refutations argue that direct API calls (nvim_buf_set_option, nvim_win_set_buf) don't propagate Lua callback errors from autocmds. The technical nuance is real: pure Lua autocmd callback errors are wrapped by nlua_pcall and surfaced via nvim_err_writeln, not as Lua errors at the API boundary. However:

  • Vimscript autocmds using :throw (still common, especially in older configs and plugins) do propagate through try_start/try_end and become Lua errors at the API boundary. This is the same mechanism that lets vim.cmd('edit ...') throw via BufRead, which the maintainer already accepted as a trigger for the prior two hoists.
  • For (2) specifically, the throw points include vim.cmd('diffthis') at line 592 and the FileType autocmd from set_option_value. vim.cmd goes through do_cmdline — exactly the path the maintainer already accepted in 17626a3 / bf30c36 — so the refutation arguing direct-API autocmd insulation is largely a non-objection for (2).
  • Even for (1), the symmetric-hoist fix is one assignment moved. The repository pattern this PR has codified three times in succession (hoist resource handles outside the throw window so error-cleanup can find them) is exactly what's missing here.

Step-by-step proof for (2) (existing-file fallback path)

  1. Terminal-only layout: window W_t holds the Claude terminal. User asks Claude to edit foo.md.
  2. Fallback block at lines 1216-1229 runs successfully — fallback_window = W_f, scratch S displayed.
  3. new_buffer is created and named (lines 1229-1239).
  4. Line 1250: _create_diff_view_from_window runs; eventually setup_new_buffer is called.
  5. Line 584-585: create_split() creates new_win = W_n.
  6. Line 586: nvim_win_set_buf(W_n, new_buf) puts the proposed buffer into W_n.
  7. Line 592: vim.cmd('diffthis') — a user DiffUpdated/OptionSet 'diff' vimscript autocmd :throws.
  8. pcall catches. active_diffs[tab_name] is nil → else branch runs. fallback_window valid → W_f closed. new_buffer valid → force-deleted (Neovim swaps a replacement buffer into W_n). W_n survives. diff_info is unreachable from this scope.
  9. User sees terminal + stranded empty pane, MCP returns 'Diff setup failed', each retry stacks another stranded window.

Why the regression test misses both

tests/unit/diff_terminal_only_window_spec.lua:108-110 stubs _create_diff_view_from_window itself to throw. That throw fires (a) after fallback_window is assigned, hiding (1); and (b) before setup_new_buffer runs, so new_win is never created, hiding (2). The buffer-count assertion catches new_buffer leaks but cannot catch either window leak.

Fix (symmetric to 17626a3 / bf30c36)

For (1), hoist the assignment to immediately after create_split():

if not target_window and not created_new_tab then
  create_split()
  fallback_window = vim.api.nvim_get_current_win()  -- track immediately
  target_window = fallback_window
  local scratch_buf = vim.api.nvim_create_buf(false, true)
  if scratch_buf ~= 0 then
    vim.api.nvim_buf_set_option(scratch_buf, "bufhidden", "wipe")
    vim.api.nvim_win_set_buf(fallback_window, scratch_buf)
  end
end

For (2), hoist a new_window local above the pcall and assign it from diff_info.new_window immediately after _create_diff_view_from_window returns. In the else branch:

if new_window and vim.api.nvim_win_is_valid(new_window) then
  pcall(vim.api.nvim_win_close, new_window, true)
end

The regression spec should gain a case that lets _create_diff_view_from_window succeed and injects the throw afterwards (e.g. stubbing register_diff_autocmds or nvim_win_get_cursor), asserting window count returns to baseline.

Severity

nit — same trigger class and same one-line-hoist fix as the three prior cases the maintainer has accepted on this PR. (1) is a regression in this PR's new fallback block; (2) is structurally pre-existing but the new fallback path materially expands its trigger surface (terminal-only diffs that used to error out before any window was created now run all the way through setup_new_buffer). Folding into #262 closes out the symmetric pattern.


-- Re-throw the error for MCP compliance
Comment thread
ThomasK33 marked this conversation as resolved.
Expand Down
105 changes: 105 additions & 0 deletions scripts/repro_issue_231.lua
Original file line number Diff line number Diff line change
@@ -0,0 +1,105 @@
-- Reproduction / verification for issue #231:
-- "When the Claude Code terminal is the only window (no other splits), an
-- error is generated when Claude tries to suggest changes."
-- https://github.com/coder/claudecode.nvim/issues/231
--
-- The bug: with a single `buftype=terminal` window, diff.lua's
-- find_main_editor_window() returns nil (it correctly excludes terminals). The
-- fix makes M._setup_blocking_diff create a split to host the diff instead of
-- erroring with "No suitable editor window found".
--
-- This script drives the REAL diff.lua against a terminal-only layout, with no
-- WebSocket/Claude CLI needed. It exercises the exact code path the openDiff MCP
-- tool uses (M._setup_blocking_diff), so it both reproduces the original bug (on
-- unfixed code) and verifies the fix.
--
-- Run from the repo root:
-- nvim --headless -u NONE -l scripts/repro_issue_231.lua
--
-- Exit code: 0 if the diff opens (fixed), 1 if the #231 error is reproduced.
-- The detailed verdict is printed to stdout either way.

local script_path = debug.getinfo(1, "S").source:sub(2)
local repo_root = vim.fn.fnamemodify(script_path, ":h:h")
vim.opt.rtp:prepend(repo_root)

local function out(msg)
io.stdout:write(msg .. "\n")
end

local diff = require("claudecode.diff")

---Make a `buftype=terminal` window the ONLY window (the issue #231 layout).
local function make_terminal_only_window()
vim.cmd("silent! only")
vim.cmd("enew!")
-- jobstart({term=true}) (Neovim 0.11+) / fallback to termopen on older versions.
if vim.fn.has("nvim-0.11") == 1 then
vim.fn.jobstart({ "cat" }, { term = true })
else
vim.fn.termopen({ "cat" })
end
vim.cmd("silent! only")
return #vim.api.nvim_list_wins(), vim.api.nvim_buf_get_option(0, "buftype")
end

---Run M._setup_blocking_diff for a brand-new file and capture the outcome.
---@return boolean ok, string detail
local function try_open_diff()
local new_file = repo_root .. "/__issue_231_repro__.md"
os.remove(new_file) -- ensure is_new_file = true (matches: Claude proposing a new file)
local ok, err = pcall(function()
diff._setup_blocking_diff({
old_file_path = new_file,
new_file_path = new_file,
new_file_contents = "# Proposed by Claude\n\nhello\n",
tab_name = "✻ [Claude Code] __issue_231_repro__.md (445ca6) ⧉",
}, function() end)
end)
-- Best-effort cleanup of any windows/diff state the setup created.
pcall(function()
diff._cleanup_all_active_diffs("repro cleanup")
end)
if ok then
return true, "setup SUCCEEDED (a window was found or created)"
end
local msg = type(err) == "table" and (tostring(err.message) .. " - " .. tostring(err.data)) or tostring(err)
return false, msg
end

out("== issue #231 reproduction ==")
out(("Neovim: %s"):format(vim.version and tostring(vim.version()) or vim.fn.execute("version"):match("NVIM[^\n]*")))

-- Scenario A: default diff_opts (open_in_new_tab = false) -- the path that regressed in #231.
-- This exercises the actual fix (find_main_editor_window -> nil -> create a split fallback).
diff.setup({ diff_opts = { layout = "vertical", open_in_new_tab = false } })
local wins, bt = make_terminal_only_window()
out(("\n[A] default config | precondition: windows=%d, only buftype=%q"):format(wins, bt))
local a_ok, a_detail = try_open_diff()
out(("[A] result: %s -> %s"):format(a_ok and "OK" or "ERROR", a_detail))

-- Scenario B: open_in_new_tab = true -- a pre-existing WORKAROUND. NOTE: this does NOT exercise
-- the #231 fix path; the new-tab path creates its own window and never calls
-- find_main_editor_window, so it succeeds even on unfixed code. Included only to confirm the
-- documented workaround still works; scenario A is the real regression signal.
diff.setup({ diff_opts = { layout = "vertical", open_in_new_tab = true } })
wins, bt = make_terminal_only_window()
out(("\n[B] open_in_new_tab=true | precondition: windows=%d, only buftype=%q"):format(wins, bt))
local b_ok, b_detail = try_open_diff()
out(("[B] result: %s -> %s"):format(b_ok and "OK" or "ERROR", b_detail))

out("\n== verdict ==")
local bug_reproduced = (not a_ok) and a_detail:match("No suitable editor window found") ~= nil
if bug_reproduced then
out("BUG REPRODUCED: default config errors with 'No suitable editor window found' (issue #231).")
else
out("FIXED: default config opens the diff in a terminal-only layout (scenario A).")
end
if b_ok then
out("WORKAROUND OK: diff_opts.open_in_new_tab=true opens the diff in a new tab (does not exercise the fix).")
else
out("NOTE: open_in_new_tab=true did NOT open the diff in this environment: " .. b_detail)
end

io.stdout:flush()
vim.cmd("cquit " .. (bug_reproduced and 1 or 0))
129 changes: 129 additions & 0 deletions tests/unit/diff_terminal_only_window_spec.lua
Original file line number Diff line number Diff line change
@@ -0,0 +1,129 @@
-- Regression test for issue #231:
-- "When the Claude Code terminal is the only window (no other splits), an
-- error is generated when Claude tries to suggest changes."
-- https://github.com/coder/claudecode.nvim/issues/231
--
-- Before the fix, M._setup_blocking_diff errored with "No suitable editor window
-- found" whenever find_main_editor_window() returned nil (e.g. the only window
-- is a terminal). The fix creates a split + fresh buffer to host the diff, tracks
-- that window as `fallback_window`, and closes it on cleanup so it is not leaked.
require("tests.busted_setup")

-- Build a consistent mock window model where the ONLY window (1000) is a terminal,
-- so find_main_editor_window() returns nil -- the issue #231 layout. (_next_winid is
-- advanced past 1000 so create_split() allocates fresh window ids without colliding.)
local function reset_to_terminal_only()
assert(vim and vim._mock and vim._mock.reset, "expected vim mock with _mock.reset()")

vim._mock.reset()
vim._tabs = { [1] = true }
vim._current_tabpage = 1
vim._current_window = 1000
vim._next_winid = 1001

vim._mock.add_buffer(1, "term://fake/claude", "", { buftype = "terminal", modified = false })
vim._mock.add_window(1000, 1, { 1, 0 })
vim._win_tab[1000] = 1
vim._tab_windows[1] = { 1000 }
end

describe("Diff with the Claude terminal as the only window (issue #231)", function()
local diff

before_each(function()
reset_to_terminal_only()

package.loaded["claudecode.logger"] = {
debug = function() end,
error = function() end,
info = function() end,
warn = function() end,
}

package.loaded["claudecode.diff"] = nil
diff = require("claudecode.diff")

diff.setup({
diff_opts = {
layout = "vertical",
open_in_new_tab = false, -- default: the path that used to error
keep_terminal_focus = false,
on_new_file_reject = "keep_empty",
},
terminal = {},
})
end)

after_each(function()
if diff and diff._cleanup_all_active_diffs then
diff._cleanup_all_active_diffs("test teardown")
end
package.loaded["claudecode.diff"] = nil
package.loaded["claudecode.logger"] = nil
end)

it("has no suitable editor window in this layout (root-cause precondition)", function()
expect(diff._find_main_editor_window()).to_be(nil)
end)

it("creates a window for the diff instead of erroring, then cleans it up (new file)", function()
local tab_name = "✻ [Claude Code] INSTALL.md (445ca6) ⧉"
local params = {
old_file_path = "/nonexistent/INSTALL.md", -- new file (matches the issue)
new_file_path = "/nonexistent/INSTALL.md",
new_file_contents = "# Install\n\nproposed by Claude\n",
tab_name = tab_name,
}

-- The regression: this used to raise "No suitable editor window found".
local setup_ok, setup_err = pcall(function()
diff._setup_blocking_diff(params, function() end)
end)
assert.is_true(setup_ok, "diff setup should not error in a terminal-only layout: " .. tostring(setup_err))

local state = diff._get_active_diffs()[tab_name]
assert.is_table(state)
assert.is_true(vim.api.nvim_buf_is_valid(state.new_buffer))

-- The plugin had to create a window to host the diff (none existed). It must be recorded as
-- `fallback_window` (distinct from the terminal) so cleanup can close it.
assert.is_number(state.fallback_window)
assert.are_not.equal(1000, state.fallback_window)
assert.is_true(vim.api.nvim_win_is_valid(state.fallback_window))
local fallback_buf = vim.api.nvim_win_get_buf(state.fallback_window)

-- Cleanup must close the plugin-created fallback window (leaving the terminal, 1000) and wipe
-- its throwaway scratch buffer. Regression guard for the window + buffer leak (the host window
-- was left open and the scratch buffer left behind on every terminal-only diff).
diff._cleanup_diff_state(tab_name, "test cleanup")
assert.is_false(vim.api.nvim_win_is_valid(state.fallback_window))
assert.is_false(vim.api.nvim_buf_is_valid(fallback_buf))
assert.is_true(vim.api.nvim_win_is_valid(1000))
end)

-- If setup errors after the fallback window + proposed buffer are created but before the diff
-- state is registered, the error handler must still clean them up (not covered by state cleanup).
it("cleans up the fallback window and proposed buffer when setup errors before registration", function()
-- Force a failure after the fallback split is created (winid 1001) but before registration.
diff._create_diff_view_from_window = function()
error({ code = -32000, message = "boom" })
end

local bufs_before = #vim.api.nvim_list_bufs()
local tab_name = "✻ [Claude Code] err.md ⧉"
local ok = pcall(function()
diff._setup_blocking_diff({
old_file_path = "/nonexistent/err.md",
new_file_path = "/nonexistent/err.md",
new_file_contents = "x\n",
tab_name = tab_name,
}, function() end)
end)

assert.is_false(ok) -- setup is expected to fail
assert.is_nil(diff._get_active_diffs()[tab_name]) -- no diff state registered
assert.is_false(vim.api.nvim_win_is_valid(1001)) -- the stranded fallback split was closed
assert.is_true(vim.api.nvim_win_is_valid(1000)) -- the terminal window survives
assert.equals(bufs_before, #vim.api.nvim_list_bufs()) -- proposed buffer + scratch not leaked
end)
end)
Loading