fix(terminal): harden paste shim per code review (#161)

Address findings from a max-effort review of the streamed-paste shim:

- Fixture repro was self-defeating: the plugin's own fix_streamed_paste="auto"
  coalesced the paste on exactly the affected Neovim versions, so the
  agent-repro.sh "default" run could never observe the bug. The fixture now sets
  fix_streamed_paste=false by default (raw repro), with PASTE_REPRO_PLUGIN_FIX
  to opt into exercising the shipped fix.
- Harden the hot path: is_managed_terminal() now pcall-guards
  get_active_terminal_bufnr() (provider resolution could throw and break pasting
  editor-wide) and caches the lazily-required terminal module instead of
  pcall(require) on every phase.
- Latch the managed-terminal decision once per stream at phase 1 and reuse it
  for phases 2/3. This resolves the provider only once per paste (not per phase)
  and keeps buffered content intact if focus changes mid-stream (previously a
  mid-stream focus change stranded phases 1/2 and mis-delegated phase 3).
- Make the config dynamic: an `enabled` flag is checked in the override, so a
  later setup() with fix_streamed_paste=false disables it without needing to
  uninstall/restore the global vim.paste.
- is_affected_version: guard v.minor like v.patch ((v.minor or 0)) to avoid a
  nil comparison on a malformed version table.
- Simplify _accumulate to a single append loop.
- Tests: add phase-2 (1->2->3) coverage, a mid-stream focus-change case, and a
  dynamic-disable case (26 unit tests).

Skipped (noted, not applied): switching the version gate to vim.fn.has() (the
explicit, tested vim.version() check avoids patch-level has() support risk);
a buffer-local/TermOpen-based marker instead of the global override (correct
altitude but touches provider internals outside this change — the latch + pcall
capture most of the benefit); cosmetic divergences from the default handler
(progress dots, empty-chunk skip) which were verified to produce byte-identical
output.

Change-Id: I0e3fc55dc517c98cde042a8c1babea2e7e45bed3
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Signed-off-by: Thomas Kosiewski <tk@coder.com>

Author: ThomasK33

fixtures/paste-repro/README.md

@@ -148,12 +148,15 @@ This is **shipped** in the plugin as of the change that added this fixture:
   segments to 1, byte-identical to 0.12.2. Set `fix_streamed_paste = false` to opt out, or `true`
   to force it on. Unit tests: `tests/unit/terminal/paste_fix_spec.lua`.
 
-The `APPLY_PASTE_FIX=1` toggle in this fixture applies the _standalone_ community override and is
-kept for isolating the workaround independently of the plugin.
+This fixture sets `terminal.fix_streamed_paste = false` by default so it reproduces the **raw** bug
+(`APPLY_PASTE_FIX` toggles the _standalone_ community override, kept for isolating the workaround
+independently of the plugin). To instead exercise the **shipped plugin fix** end-to-end, launch
+with `PASTE_REPRO_PLUGIN_FIX=auto` (or `=true`).
 
 ## Files
 
-- `init.lua` — fixture config (native provider; `terminal_cmd` → observer; `APPLY_PASTE_FIX`,
-  `PASTE_REPRO_AUTOOPEN`, `PASTE_OBSERVER_LOG` env toggles).
+- `init.lua` — fixture config (native provider; `terminal_cmd` → observer). Env toggles:
+  `APPLY_PASTE_FIX` (standalone workaround), `PASTE_REPRO_PLUGIN_FIX` (the plugin's own
+  `fix_streamed_paste`, default off here), `PASTE_REPRO_AUTOOPEN`, `PASTE_OBSERVER_LOG`.
 - `observer.py` — bracketed-paste segment counter (the measurement instrument).
 - `agent-repro.sh` — self-contained automated reproduction via agent-tty.

fixtures/paste-repro/init.lua

@@ -73,6 +73,19 @@ if fix_active then
   end
 end
 
+-- Whether to enable the *plugin's own* paste shim. Defaults to false so the
+-- fixture reproduces the raw bug (with APPLY_PASTE_FIX as the controlled
+-- variable). Set PASTE_REPRO_PLUGIN_FIX=auto|true to instead exercise the
+-- shipped plugin fix end-to-end.
+local plugin_fix = os.getenv("PASTE_REPRO_PLUGIN_FIX")
+if plugin_fix == "auto" then
+  plugin_fix = "auto"
+elseif plugin_fix == "true" then
+  plugin_fix = true
+else
+  plugin_fix = false
+end
+
 local ok, claudecode = pcall(require, "claudecode")
 assert(ok, "Failed to load claudecode.nvim from repo root: " .. tostring(claudecode))
 
@@ -86,6 +99,12 @@ claudecode.setup({
   terminal = {
     provider = "native",
     auto_close = false,
+    -- Disabled by default so this fixture reproduces the RAW bug (APPLY_PASTE_FIX
+    -- is the controlled variable). If the plugin's shim ("auto") were left on, it
+    -- would coalesce the paste on exactly the affected Neovim versions, so the
+    -- "default" run could never observe the >1-segment fragmentation it is meant
+    -- to show. Set PASTE_REPRO_PLUGIN_FIX=auto to exercise the shipped fix.
+    fix_streamed_paste = plugin_fix,
   },
 })
 

lua/claudecode/terminal.lua

@@ -19,11 +19,7 @@ local defaults = {
   auto_close = true,
   env = {},
   snacks_win_opts = {},
-  -- Workaround for a Neovim core bug (< 0.12.2) that fragments large bracketed
-  -- pastes into the terminal; see lua/claudecode/terminal/paste_fix.lua and
-  -- https://github.com/coder/claudecode.nvim/issues/161.
-  -- true = force on, false = off, "auto" = on only for affected Neovim versions.
-  fix_streamed_paste = "auto",
+  fix_streamed_paste = "auto", -- work around Neovim <0.12.2 paste fragmentation (#161): true|false|"auto"
   -- Working directory control
   cwd = nil, -- static cwd override
   git_repo_cwd = false, -- resolve to git root when spawning
@@ -508,8 +504,7 @@ function M.setup(user_term_config, p_terminal_cmd, p_env)
   -- Setup providers with config
   get_provider().setup(defaults)
 
-  -- Install the streamed-paste compatibility shim for issue #161. No-op on
-  -- Neovim >= 0.12.2 (and when disabled), scoped to the managed terminal buffer.
+  -- Streamed-paste compatibility shim for #161 (no-op on Neovim >= 0.12.2).
   require("claudecode.terminal.paste_fix").apply(defaults.fix_streamed_paste)
 end
 

lua/claudecode/terminal/paste_fix.lua

@@ -1,30 +1,18 @@
---- Compatibility shim for a Neovim core bug that fragments large bracketed
---- pastes into a `:terminal` buffer.
----
---- See https://github.com/coder/claudecode.nvim/issues/161 and the upstream fix
---- https://github.com/neovim/neovim/pull/39152 (landed in Neovim 0.12.2).
----
---- On affected Neovim (< 0.12.2), the default `vim.paste` runs `nvim_put()` once
---- per streamed phase (1 -> 2 -> 3). In a terminal buffer each write is
---- independently wrapped in bracketed-paste markers (ESC[200~ .. ESC[201~) when
---- the inner program enabled them, so a single large paste reaches the program
---- running in the terminal (e.g. the Claude CLI) as N separate paste events.
---- Claude renders N `[Pasted text #k]` placeholders with phase-boundary bytes
---- leaking between them, which the user perceives as truncation.
----
---- This module installs a cooperative `vim.paste` override that coalesces the
---- streamed phases of a paste into ONE non-streamed (`phase == -1`) replay, but
---- only for the plugin's own managed terminal buffer. Everything else delegates
---- to the original handler unchanged. The override is a no-op on Neovim >= 0.12.2
---- (which already coalesces) unless explicitly forced.
+--- Compatibility shim for a Neovim core bug (< 0.12.2) that fragments a large
+--- bracketed paste into a `:terminal` across `vim.paste` phases, so the program
+--- in the terminal sees it as N separate pastes. Coalesces the streamed phases
+--- into one `phase == -1` replay, scoped to the plugin's managed terminal; a
+--- no-op on Neovim >= 0.12.2 unless forced. See coder/claudecode.nvim#161 and the
+--- upstream fix neovim/neovim#39152.
 --- @module 'claudecode.terminal.paste_fix'
 
 local M = {}
 
--- Accumulator for the in-flight streamed paste. Pastes are processed
--- sequentially, so a single shared buffer is safe.
-local chunks = {}
-local installed = false
+local chunks = {} -- accumulator for the in-flight streamed paste (pastes are sequential)
+local installed = false -- the global vim.paste wrap happens at most once
+local enabled = false -- config-controlled; lets a later setup() disable in place
+local streaming = false -- does the current stream target the managed terminal? (decided at phase 1)
+local terminal_mod = nil -- lazily required to avoid a terminal -> paste_fix cycle
 
 --- True if the running Neovim has the per-phase terminal-paste fragmentation
 --- bug, i.e. is older than the 0.12.2 fix.
@@ -37,7 +25,9 @@ function M.is_affected_version()
   if v.major ~= 0 then
     return false -- 1.0+ is well past the fix
   end
-  return v.minor < 12 or (v.minor == 12 and (v.patch or 0) < 2)
+  local minor = v.minor or 0
+  local patch = v.patch or 0
+  return minor < 12 or (minor == 12 and patch < 2)
 end
 
 --- Resolve whether the shim should be active for a given config value.
@@ -55,86 +45,88 @@ function M.should_enable(opt)
   return M.is_affected_version()
 end
 
---- Append one streamed phase's `lines` to `acc`, re-gluing the mid-line seam.
----
---- `lines` is a `readfile()`-style split of the chunk on "\n" with the delimiters
---- dropped, so when a paste is fragmented across chunks the boundary usually
---- falls mid-line: the last element of the previous chunk and the first element
---- of this chunk are two halves of the same source line. Appending naively would
---- insert a spurious newline at every seam, so the first incoming line is joined
---- onto the last buffered line instead.
+--- Append one streamed phase's `lines` to `acc`. `lines` is a `readfile()`-style
+--- split, so consecutive chunks meet mid-line; re-glue the seam rather than
+--- inserting a spurious newline at every chunk boundary.
 --- @param acc string[] accumulator (mutated in place)
---- @param lines string[] the current phase's lines
+--- @param lines string[]
 function M._accumulate(acc, lines)
   if #lines == 0 then
     return
   end
-  if #acc == 0 then
-    for _, line in ipairs(lines) do
-      acc[#acc + 1] = line
-    end
-  else
-    acc[#acc] = acc[#acc] .. lines[1]
-    for i = 2, #lines do
-      acc[#acc + 1] = lines[i]
-    end
+  local start = 1
+  if #acc > 0 then
+    acc[#acc] = acc[#acc] .. lines[1] -- re-glue mid-line seam
+    start = 2
+  end
+  for i = start, #lines do
+    acc[#acc + 1] = lines[i]
   end
 end
 
---- Whether `bufnr` is the plugin's managed Claude terminal buffer.
+--- Whether `bufnr` is the plugin's managed terminal. Must never throw: it runs on
+--- the paste hot path, so failures degrade to "not managed".
 --- @param bufnr integer
 --- @return boolean
 local function is_managed_terminal(bufnr)
   if vim.bo[bufnr].buftype ~= "terminal" then
     return false
   end
-  -- Lazy require to avoid a load-time dependency cycle (terminal -> paste_fix).
-  local ok, terminal = pcall(require, "claudecode.terminal")
-  if not ok then
-    return false
+  if not terminal_mod then
+    local ok, mod = pcall(require, "claudecode.terminal")
+    if not ok then
+      return false
+    end
+    terminal_mod = mod
   end
-  local active = terminal.get_active_terminal_bufnr()
-  return active ~= nil and active == bufnr
+  local ok, active = pcall(terminal_mod.get_active_terminal_bufnr)
+  return ok and active ~= nil and active == bufnr
 end
 
---- Install the cooperative `vim.paste` override (idempotent). Captures the
---- current `vim.paste` and delegates to it for everything except streamed pastes
---- into the plugin's managed terminal buffer.
+--- Install the cooperative `vim.paste` override (idempotent). Delegates to the
+--- captured original except for a streamed paste into the managed terminal, and
+--- honours `enabled` at call time so it can be disabled without uninstalling.
 function M.install()
   if installed then
     return
   end
   installed = true
-  chunks = {}
 
   local orig_paste = vim.paste
   vim.paste = function(lines, phase)
-    -- Only intervene for streamed pastes (phase 1/2/3) into the plugin's own
-    -- terminal buffer. `phase == -1` is already a whole, non-streamed paste.
-    if phase == -1 or not is_managed_terminal(vim.api.nvim_get_current_buf()) then
+    if not enabled then
       return orig_paste(lines, phase)
     end
 
+    -- Decide once per stream (at phase 1) whether it targets the managed
+    -- terminal, so phases 2/3 stay coalesced even if focus moves mid-stream.
     if phase == 1 then
+      streaming = is_managed_terminal(vim.api.nvim_get_current_buf())
       chunks = {}
     end
+
+    if not streaming or phase == -1 then
+      return orig_paste(lines, phase)
+    end
+
     M._accumulate(chunks, lines)
 
     if phase == 3 then
       local buffered = chunks
       chunks = {}
-      -- Replay as a single non-streamed paste so the terminal wraps it in one
-      -- bracketed-paste segment.
-      return orig_paste(buffered, -1)
+      streaming = false
+      return orig_paste(buffered, -1) -- one non-streamed replay = one bracketed segment
     end
     return true
   end
 end
 
---- Apply the shim according to the resolved config value.
+--- Apply the shim per the resolved config value: set the active state (so a later
+--- call with `false` disables an installed override) and install on first enable.
 --- @param opt boolean|string|nil
 function M.apply(opt)
-  if M.should_enable(opt) then
+  enabled = M.should_enable(opt)
+  if enabled then
     M.install()
   end
 end
@@ -145,4 +137,10 @@ function M._is_installed()
   return installed
 end
 
+--- Test helper: report whether the override is currently active.
+--- @return boolean
+function M._is_enabled()
+  return enabled
+end
+
 return M

tests/unit/terminal/paste_fix_spec.lua

@@ -155,7 +155,8 @@ describe("claudecode.terminal.paste_fix", function()
           return managed_bufnr
         end,
       }
-      paste_fix.install()
+      -- apply(true) sets enabled and installs the override.
+      paste_fix.apply(true)
     end
 
     it("coalesces a streamed paste into one phase==-1 replay for the managed terminal", function()
@@ -169,6 +170,18 @@ describe("claudecode.terminal.paste_fix", function()
       assert.are.same({ "hello world", "second line" }, orig_calls[1].lines)
     end)
 
+    it("coalesces a three-phase (1->2->3) stream including the middle phase", function()
+      setup_env()
+      -- Source "foo\nbar\nbaz" streamed across three chunks, each split mid-line.
+      assert.is_true(vim.paste({ "foo", "ba" }, 1))
+      assert.is_true(vim.paste({ "r", "ba" }, 2))
+      assert.is_true(vim.paste({ "z" }, 3))
+
+      assert.are.equal(1, #orig_calls)
+      assert.are.equal(-1, orig_calls[1].phase)
+      assert.are.same({ "foo", "bar", "baz" }, orig_calls[1].lines)
+    end)
+
     it("delegates a non-streamed (phase==-1) paste unchanged", function()
       setup_env()
       vim.paste({ "whole" }, -1)
@@ -177,6 +190,32 @@ describe("claudecode.terminal.paste_fix", function()
       assert.are.same({ "whole" }, orig_calls[1].lines)
     end)
 
+    it("keeps buffered content when focus leaves the managed terminal mid-stream", function()
+      setup_env()
+      -- Phase 1 targets the managed terminal; the decision is latched.
+      assert.is_true(vim.paste({ "kept ", "dat" }, 1))
+      -- Focus moves away before phase 3 (active bufnr no longer current).
+      current_bufnr = 99
+      buftype_by_buf[99] = ""
+      vim.paste({ "a" }, 3)
+      -- Still coalesced into one replay; buffered content is not stranded.
+      assert.are.equal(1, #orig_calls)
+      assert.are.equal(-1, orig_calls[1].phase)
+      assert.are.same({ "kept ", "data" }, orig_calls[1].lines)
+    end)
+
+    it("respects a later apply(false): delegates instead of coalescing", function()
+      setup_env()
+      paste_fix.apply(false) -- disable without uninstalling
+      assert.is_false(paste_fix._is_enabled())
+      vim.paste({ "a", "b" }, 1)
+      vim.paste({ "c" }, 3)
+      -- No coalescing: each phase passes straight through.
+      assert.are.equal(2, #orig_calls)
+      assert.are.equal(1, orig_calls[1].phase)
+      assert.are.equal(3, orig_calls[2].phase)
+    end)
+
     it("delegates pastes into a non-managed terminal unchanged", function()
       setup_env()
       current_bufnr = 99 -- not the managed terminal