fix(diff): close stranded tab when open_in_new_tab diff setup errors

fixtures/issue-262/init.lua

@@ -0,0 +1,170 @@
+-- Fixture for issue #262:
+--   "diff: open_in_new_tab can strand a tab if setup errors before the diff
+--    state is registered"
+--   https://github.com/coder/claudecode.nvim/issues/262
+--
+-- This fixture configures diff_opts.open_in_new_tab = true and provides a single
+-- trigger (:ReproStrandTab / <leader>x) that exercises the realistic failure: a
+-- user BufReadPre autocmd throws while the original file is :edit-ed during diff
+-- setup. Because the error happens AFTER display_terminal_in_new_tab() ran
+-- `:tabnew` but BEFORE the diff state is registered, the post-pcall error handler
+-- cannot close the new tab -> it is stranded, and focus is left on it.
+--
+-- Usage (from repo root):
+--   source fixtures/nvim-aliases.sh && vv issue-262
+-- then press <leader>x (or run :ReproStrandTab). Watch the tabline jump from one
+-- tab to two; the extra empty tab is the stranded one (#262).
+
+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 = "\\"
+
+-- Always show the tabline so the stranded tab is visible, with an explicit,
+-- unambiguous label per tab (number + active marker + buffer name).
+vim.o.showtabline = 2
+vim.o.laststatus = 2
+function _G.Repro262Tabline()
+  local s = {}
+  for i = 1, vim.fn.tabpagenr("$") do
+    local active = (i == vim.fn.tabpagenr())
+    local winnr = vim.fn.tabpagewinnr(i)
+    local buflist = vim.fn.tabpagebuflist(i)
+    local bufname = vim.fn.bufname(buflist[winnr])
+    local label = (bufname == "" and "[No Name]" or vim.fn.fnamemodify(bufname, ":t"))
+    s[#s + 1] = (active and "%#TabLineSel#" or "%#TabLine#")
+    s[#s + 1] = (" TAB %d%s: %s "):format(i, active and " (active)" or "", label)
+  end
+  s[#s + 1] = "%#TabLineFill#"
+  return table.concat(s)
+end
+vim.o.tabline = "%!v:lua.Repro262Tabline()"
+
+local ok, claudecode = pcall(require, "claudecode")
+assert(ok, "Failed to load claudecode.nvim from repo root: " .. tostring(claudecode))
+
+claudecode.setup({
+  auto_start = false,
+  log_level = "info",
+  terminal = {
+    provider = "native",
+    auto_close = false,
+  },
+  diff_opts = {
+    layout = "vertical",
+    open_in_new_tab = true, -- the path under test (#262)
+    keep_terminal_focus = false,
+    on_new_file_reject = "keep_empty",
+  },
+})
+
+-- A normal editor buffer in the first tab so the layout looks like real usage.
+local banner = {
+  "claudecode.nvim -- issue #262 reproduction fixture",
+  "",
+  "diff_opts.open_in_new_tab = true",
+  "",
+  "Press <leader>x (space then x) or run :ReproStrandTab",
+  "",
+  "Expected on UNFIXED code: the tabline jumps from 1 tab to 2,",
+  "and focus is left on the new, EMPTY tab -- that extra tab is",
+  "stranded because diff setup errored before the diff state was",
+  "registered, so neither cleanup path can close it.",
+}
+vim.api.nvim_buf_set_lines(0, 0, -1, false, banner)
+vim.bo.modifiable = false
+vim.bo.modified = false
+
+---Trigger the realistic #262 failure: a throwing BufReadPre autocmd fires while
+---diff setup runs `:edit <old_file>`, after the new tab was already created.
+local function repro_strand_tab()
+  -- Assert the diff module config (defensive; claudecode.setup already did this).
+  local diff = require("claudecode.diff")
+  diff.setup({
+    diff_opts = { layout = "vertical", open_in_new_tab = true, on_new_file_reject = "keep_empty" },
+    terminal = {},
+  })
+
+  local before = vim.fn.tabpagenr("$")
+
+  -- A fresh on-disk file that is NOT already loaded, so `:edit` reads it and
+  -- fires BufReadPre (where our autocmd throws).
+  local old_file = vim.fn.tempname() .. "_issue262.md"
+  local fh = io.open(old_file, "w")
+  fh:write("# original\n\nline one\nline two\n")
+  fh:close()
+
+  local grp = vim.api.nvim_create_augroup("repro262", { clear = true })
+  vim.api.nvim_create_autocmd("BufReadPre", {
+    group = grp,
+    pattern = old_file,
+    callback = function()
+      error("simulated BufReadPre failure (#262)")
+    end,
+  })
+
+  pcall(function()
+    diff._setup_blocking_diff({
+      old_file_path = old_file,
+      new_file_path = old_file,
+      new_file_contents = "# proposed by Claude\n\nNEW line one\nline two\n",
+      tab_name = "✻ [Claude Code] issue262.md ⧉",
+    }, function() end)
+  end)
+
+  pcall(vim.api.nvim_del_augroup_by_id, grp)
+  os.remove(old_file)
+
+  local after = vim.fn.tabpagenr("$")
+  -- Keep the message to ONE short line so it doesn't trip the hit-enter prompt.
+  vim.api.nvim_echo({
+    {
+      ("repro262: tabs %d -> %d%s"):format(before, after, after > before and "  <<< STRANDED TAB" or "  (no leak)"),
+      after > before and "ErrorMsg" or "MoreMsg",
+    },
+  }, false, {})
+end
+
+vim.api.nvim_create_user_command("ReproStrandTab", repro_strand_tab, { desc = "Repro #262 stranded tab" })
+vim.keymap.set("n", "<leader>x", repro_strand_tab, { desc = "Repro #262 stranded tab" })
+
+-- Success-path probe: open a real diff in a new tab with NO injected error. The
+-- fix must NOT close this tab (the error-branch tabclose should only fire on
+-- failure). Used during /verify to confirm the change didn't over-close.
+vim.api.nvim_create_user_command("ReproOpenDiffOk", function()
+  local diff = require("claudecode.diff")
+  diff.setup({
+    diff_opts = { layout = "vertical", open_in_new_tab = true, on_new_file_reject = "keep_empty" },
+    terminal = {},
+  })
+  local before = vim.fn.tabpagenr("$")
+  local old_file = vim.fn.tempname() .. "_ok262.md"
+  local fh = io.open(old_file, "w")
+  fh:write("# original\n\nline one\nline two\n")
+  fh:close()
+  local ok_setup = pcall(function()
+    diff._setup_blocking_diff({
+      old_file_path = old_file,
+      new_file_path = old_file,
+      new_file_contents = "# proposed by Claude\n\nNEW line one\nline two\n",
+      tab_name = "✻ [Claude Code] ok262.md ⧉",
+    }, function() end)
+  end)
+  os.remove(old_file)
+  local after = vim.fn.tabpagenr("$")
+  vim.api.nvim_echo({
+    {
+      ("ReproOpenDiffOk: ok=%s  tabs %d -> %d"):format(tostring(ok_setup), before, after),
+      ok_setup and "MoreMsg" or "ErrorMsg",
+    },
+  }, false, {})
+end, { desc = "Open a successful diff in a new tab (#262 success-path probe)" })
+
+vim.api.nvim_create_user_command("ReproReset", function()
+  require("claudecode.diff")._cleanup_all_active_diffs("repro reset")
+  vim.cmd("silent! tabonly!")
+  vim.cmd("silent! only!")
+  vim.api.nvim_echo({ { ("ReproReset: tabs=%d"):format(vim.fn.tabpagenr("$")), "MoreMsg" } }, false, {})
+end, { desc = "Reset repro layout to a single tab" })

lua/claudecode/diff.lua

@@ -1143,6 +1143,11 @@ function M._setup_blocking_diff(params, resolution_callback)
   -- (the state-based cleanup is gated on a registered diff). Issue #231.
   local fallback_window = nil
   local new_buffer = nil
+  -- Same rationale for the open_in_new_tab path: display_terminal_in_new_tab() runs `:tabnew`
+  -- early, so a failure before registration would strand that tab. Hoist its handle (and the tab
+  -- we came from, to refocus) so the error handler can close it and switch back. Issue #262.
+  local new_tab_handle = nil
+  local original_tab_handle = nil
 
   -- Wrap the setup in error handling to ensure cleanup on failure
   local setup_success, setup_error = pcall(function()
@@ -1165,11 +1170,15 @@ function M._setup_blocking_diff(params, resolution_callback)
     local terminal_win_in_new_tab = nil
     local existing_buffer = nil
     local target_window = nil
-    -- Track new tab handle and original terminal visibility for robust cleanup
-    local new_tab_handle = nil
+    -- new_tab_handle is hoisted above the pcall (issue #262) so the error handler can reach it;
+    -- only the original-terminal visibility flag is local here.
     local had_terminal_in_original = false
 
     if config and config.diff_opts and config.diff_opts.open_in_new_tab then
+      -- Capture the tab we're leaving BEFORE display_terminal_in_new_tab() runs `:tabnew`, so the
+      -- error handler can still refocus it if that helper throws partway (Lua then leaves
+      -- new_tab_handle unassigned -- see the error branch below). Issue #262.
+      original_tab_handle = vim.api.nvim_get_current_tabpage()
       original_tab_number, terminal_win_in_new_tab, had_terminal_in_original, new_tab_handle =
         display_terminal_in_new_tab()
       created_new_tab = true
@@ -1320,6 +1329,29 @@ function M._setup_blocking_diff(params, resolution_callback)
       if new_buffer and vim.api.nvim_buf_is_valid(new_buffer) then
         pcall(vim.api.nvim_buf_delete, new_buffer, { force = true })
       end
+      -- Close the tab opened by display_terminal_in_new_tab() before registration (issue #262).
+      -- That helper runs `:tabnew` (switching to the new tab) and the tab holds no user content.
+      -- Prefer the returned handle; if the helper itself threw after `:tabnew`, Lua never assigned
+      -- new_tab_handle, so fall back to the current tab (which is still that new tab) when we can
+      -- tell it apart from the original. Refocus the original tab afterwards.
+      local stranded_tab = new_tab_handle
+      if not (stranded_tab and vim.api.nvim_tabpage_is_valid(stranded_tab)) then
+        local current_tab = vim.api.nvim_get_current_tabpage()
+        if
+          original_tab_handle
+          and vim.api.nvim_tabpage_is_valid(original_tab_handle)
+          and current_tab ~= original_tab_handle
+        then
+          stranded_tab = current_tab
+        end
+      end
+      if stranded_tab and vim.api.nvim_tabpage_is_valid(stranded_tab) and stranded_tab ~= original_tab_handle then
+        pcall(vim.api.nvim_set_current_tabpage, stranded_tab)
+        pcall(vim.cmd, "tabclose")
+        if original_tab_handle and vim.api.nvim_tabpage_is_valid(original_tab_handle) then
+          pcall(vim.api.nvim_set_current_tabpage, original_tab_handle)
+        end
+      end
     end
 
     -- Re-throw the error for MCP compliance