diff --git a/CHANGELOG.md b/CHANGELOG.md index f156b1e6..76786cc9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,8 +2,13 @@ ## [Unreleased] +### Features + +- `:ClaudeCodeCloseAllDiffs` command to close pending Claude diffs at once (e.g. proposals orphaned by resolving them via Claude remote control). Diffs you have already accepted but whose file has not been written yet are left intact so saved edits are never discarded. ([#248](https://github.com/coder/claudecode.nvim/issues/248)) + ### 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)) - 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 diff --git a/README.md b/README.md index bdc05a1f..0253aec0 100644 --- a/README.md +++ b/README.md @@ -198,6 +198,7 @@ Configure the plugin with the detected path: - `:ClaudeCodeAdd [start-line] [end-line]` - Add specific file to Claude context with optional line range - `:ClaudeCodeDiffAccept` - Accept diff changes - `:ClaudeCodeDiffDeny` - Reject diff changes +- `:ClaudeCodeCloseAllDiffs` - Close pending Claude diffs (leaves accepted/saved diffs intact) ## Working with Diffs @@ -208,6 +209,8 @@ When Claude proposes changes, the plugin opens a native Neovim diff view: You can edit Claude's suggestions before accepting them. +If a diff is resolved outside this Neovim (for example via Claude remote control on another device) the diff windows would otherwise stay open. They are now closed automatically when the Claude session that opened them disconnects. If you resolve diffs remotely while the session is still connected, run `:ClaudeCodeCloseAllDiffs` to clear the leftover pending proposals — it leaves any diff you have already accepted (`:w`) but whose file has not been written yet untouched, so your saved edits are never discarded. + ## How It Works This plugin creates a WebSocket server that Claude Code CLI connects to, implementing the same protocol as the official VS Code extension. When you launch Claude, it automatically detects Neovim and gains full access to your editor. diff --git a/fixtures/remote-diff/README.md b/fixtures/remote-diff/README.md new file mode 100644 index 00000000..9f48a26f --- /dev/null +++ b/fixtures/remote-diff/README.md @@ -0,0 +1,82 @@ +# `remote-diff` fixture — repro for issue #248 + +Reproduces the behaviour behind +[#248 "Close diff handled by remote control"](https://github.com/coder/claudecode.nvim/issues/248): +diffs Claude opens in Neovim (via the `openDiff` MCP tool) **stay open forever** +when they are accepted/rejected somewhere other than this Neovim instance +(e.g. Claude "remote control" on a phone), or when the Claude session that +opened them goes away without closing them. + +This fixture is like the generic [`repro`](../repro) fixture but: + +- keeps logging at `warn` (so the diff UI is clean for screenshots / automation — + the `repro` fixture's `debug` level spams the message area and triggers + hit-enter prompts), and +- adds a `:DiffState` / `:DiffStateFile` inspector that prints how many windows + are open and how many diffs the diff module still considers **active/pending**. + +## Files + +- `init.lua` — minimal claudecode.nvim config + `:DiffState` inspector. +- `example/{a.txt,b.txt,c.lua}` — sample files to diff against. + +## Quick start + +```sh +# Terminal 1 — the editor under test: +source fixtures/nvim-aliases.sh +vv remote-diff +# (equivalently: NVIM_APPNAME=remote-diff XDG_CONFIG_HOME=fixtures nvim a.txt) +# The server auto-starts; check the lock file exists: +# ls ~/.claude/ide/*.lock + +# Terminal 2 — play the role of Claude over the MCP socket: +scripts/repro_issue_248.sh # open 3 diffs, then DISCONNECT (no close_tab) +``` + +Now back in Neovim run `:DiffState`. With the #248 fix you will see: + +``` +windows=1 active_diffs=0 +``` + +The client went away, and `on_disconnect` automatically closed the diffs it had +opened. **Before the fix** the diff windows lingered (`windows=6 active_diffs=3`, +all `[pending]`) because teardown depended entirely on a `close_tab` the departed +client never sent — that was the bug. + +`scripts/repro_issue_248.sh --cleanup` instead sends `closeAllDiffTabs`, which now +drains the diff registry (resolving pending diffs), so `:DiffState` likewise shows +`active_diffs=0` — before the fix it closed the windows but left `active_diffs > 0`. + +## Verifying with the _real_ Claude CLI + +The synthetic script is convenient, but you can confirm the fix with the real CLI too: + +```sh +# Point a real Claude at this Neovim's MCP server (use the port from the lock file): +PORT=$(basename "$(ls ~/.claude/ide/*.lock | head -1)" .lock) +cd "$(jq -r .workspaceFolders[0] ~/.claude/ide/$PORT.lock)" +ENABLE_IDE_INTEGRATION=true CLAUDE_CODE_SSE_PORT=$PORT claude --ide +``` + +In Claude, switch **off** auto-accept (Shift+Tab until the mode line is blank — +in auto/accept-edits mode Claude edits files directly and never uses the IDE +diff), then ask it to edit a file. The diff opens in Neovim (`:DiffState` shows +`active_diffs=1`). + +- Accept it (in Neovim **or** in Claude's prompt) → Claude sends `close_tab` → + the diff closes. This is the normal local flow. +- Instead, **kill the Claude process** before it sends `close_tab` (mimicking a + phone/remote-control resolution). With the #248 fix the pending diff is now + auto-closed by `on_disconnect` — `:DiffState` shows `windows=1 active_diffs=0` + (before the fix the window leaked and stayed open). A diff you had already + accepted with `:w` is instead left open, so its not-yet-written edits survive. + +## Inspector commands (added by this fixture) + +- `:DiffState` — notify window count + active diff tab names/status. +- `:DiffStateFile [path]` — write the same info to a file (for automation; + defaults to `stdpath('cache')/diff_state.txt`). +- `as` — run `:DiffState`. +- `aa` / `ad` — accept / deny the focused diff. diff --git a/fixtures/remote-diff/example/a.txt b/fixtures/remote-diff/example/a.txt new file mode 100644 index 00000000..a72d44ea --- /dev/null +++ b/fixtures/remote-diff/example/a.txt @@ -0,0 +1,4 @@ +This is file A. + +Keep this window focused. +This simulates "you are looking at nvim". diff --git a/fixtures/remote-diff/example/b.txt b/fixtures/remote-diff/example/b.txt new file mode 100644 index 00000000..c0d0fb45 --- /dev/null +++ b/fixtures/remote-diff/example/b.txt @@ -0,0 +1,2 @@ +line1 +line2 diff --git a/fixtures/remote-diff/example/c.lua b/fixtures/remote-diff/example/c.lua new file mode 100644 index 00000000..b0f0846b --- /dev/null +++ b/fixtures/remote-diff/example/c.lua @@ -0,0 +1,5 @@ +local M = {} +function M.hello() + return "hello" +end +return M diff --git a/fixtures/remote-diff/init.lua b/fixtures/remote-diff/init.lua new file mode 100644 index 00000000..caa597bf --- /dev/null +++ b/fixtures/remote-diff/init.lua @@ -0,0 +1,98 @@ +-- Repro fixture for issue #248: "Close diff handled by remote control". +-- +-- Scenario this fixture is built to demonstrate: +-- 1. Claude opens one or more diffs in Neovim via the `openDiff` MCP tool. +-- 2. The user resolves those diffs from *somewhere other than this Neovim* +-- (e.g. Claude "remote control" on a phone), so the diff is never +-- accepted/rejected inside Neovim and no `close_tab` arrives. +-- 3. The diff windows stay open in Neovim forever. +-- +-- Unlike the generic `repro` fixture this one keeps logging quiet (so the diff +-- UI is clean for screenshots / automation) and exposes a `:DiffState` command +-- that prints how many windows and how many *active* claudecode diffs exist. +-- +-- Usage (from repo root): +-- source fixtures/nvim-aliases.sh +-- vv remote-diff # or: NVIM_APPNAME=remote-diff XDG_CONFIG_HOME=fixtures nvim a.txt +-- +-- Then drive the MCP side with scripts/repro_issue_248.sh. + +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 = "\\" + +local ok, claudecode = pcall(require, "claudecode") +assert(ok, "Failed to load claudecode.nvim from repo root: " .. tostring(claudecode)) + +claudecode.setup({ + auto_start = false, + -- Keep logging quiet so the diff UI is clean for screenshots / automation. + -- (The generic `repro` fixture uses "debug", which spams the message area and + -- triggers hit-enter prompts that interfere with TUI automation.) + log_level = "warn", + terminal = { + provider = "native", + auto_close = false, + }, + diff_opts = { + layout = "vertical", + open_in_new_tab = false, + keep_terminal_focus = false, + }, +}) + +local function ensure_started() + local ok_start, started_or_err, port_or_err = pcall(function() + return claudecode.start(false) + end) + if not ok_start then + vim.notify("ClaudeCode start crashed: " .. tostring(started_or_err), vim.log.levels.ERROR) + return false + end + if started_or_err or port_or_err == "Already running" then + return true + end + vim.notify("ClaudeCode failed to start: " .. tostring(port_or_err), vim.log.levels.ERROR) + return false +end + +ensure_started() + +-- Inspection command: how many windows, and how many *active* diffs does the +-- diff module still think are open? This is the heart of the repro: after a +-- remote resolution the windows linger and active_diffs never drains. +local function diff_state() + local wins = #vim.api.nvim_list_wins() + local active = require("claudecode.diff")._get_active_diffs() + local names = {} + for tab_name, data in pairs(active) do + names[#names + 1] = (" [%s] %s"):format(data.status or "?", tab_name) + end + table.sort(names) + local lines = { + ("windows=%d active_diffs=%d"):format(wins, #names), + } + vim.list_extend(lines, names) + return lines, wins, #names +end + +vim.api.nvim_create_user_command("DiffState", function() + local lines = diff_state() + vim.notify(table.concat(lines, "\n"), vim.log.levels.INFO) +end, { desc = "Show window count + active claudecode diffs" }) + +-- Scriptable variant: writes the state to a file so external automation can +-- assert on it without scraping the message area. +vim.api.nvim_create_user_command("DiffStateFile", function(opts) + -- stdpath("cache") (not "run") so this works on the plugin's Neovim 0.8 floor. + local path = opts.args ~= "" and opts.args or (vim.fn.stdpath("cache") .. "/diff_state.txt") + local lines = diff_state() + vim.fn.writefile(lines, path) +end, { nargs = "?", desc = "Write window/diff state to a file" }) + +vim.keymap.set("n", "aa", "ClaudeCodeDiffAccept", { desc = "Accept diff" }) +vim.keymap.set("n", "ad", "ClaudeCodeDiffDeny", { desc = "Deny diff" }) +vim.keymap.set("n", "as", "DiffState", { desc = "Show diff state" }) diff --git a/lua/claudecode/diff.lua b/lua/claudecode/diff.lua index 79f8bb9d..6ff9057a 100644 --- a/lua/claudecode/diff.lua +++ b/lua/claudecode/diff.lua @@ -1257,6 +1257,7 @@ function M._setup_blocking_diff(params, resolution_callback) resolution_callback = resolution_callback, result_content = nil, is_new_file = is_new_file, + client_id = params.client_id, }) end) -- End of pcall @@ -1293,8 +1294,9 @@ end ---@param new_file_path string Path to the new file (used for naming) ---@param new_file_contents string Contents of the new file ---@param tab_name string Name for the diff tab/view +---@param client_id string|nil Id of the MCP client opening the diff (so it can be cleaned up if that client disconnects) ---@return table response MCP-compliant response with content array -function M.open_diff_blocking(old_file_path, new_file_path, new_file_contents, tab_name) +function M.open_diff_blocking(old_file_path, new_file_path, new_file_contents, tab_name, client_id) -- Check for existing diff with same tab_name if active_diffs[tab_name] then local existing_diff = active_diffs[tab_name] @@ -1324,6 +1326,7 @@ function M.open_diff_blocking(old_file_path, new_file_path, new_file_contents, t new_file_path = new_file_path, new_file_contents = new_file_contents, tab_name = tab_name, + client_id = client_id, }, function(result) -- Resume the coroutine with the result local resume_success, resume_result = coroutine.resume(co, result) @@ -1428,6 +1431,77 @@ function M.close_diff_by_tab_name(tab_name) return false end +---Close every active diff matching an optional filter. +---Reuses close_diff_by_tab_name, which resolves still-pending diffs as rejected +---(resuming their coroutine) before tearing down the UI. +---@param filter_fn (fun(diff_data: table): boolean)|nil Only close diffs for which this returns true (nil = all) +---@param reason string Human-readable reason (for logging) +---@return number count Number of diffs closed +local function close_active_diffs(filter_fn, reason) + local count = 0 + -- Snapshot the tab names first: close_diff_by_tab_name nils out entries as it + -- goes, and mutating a table while iterating it with pairs() is undefined. + local tab_names = {} + for tab_name, diff_data in pairs(active_diffs) do + if not filter_fn or filter_fn(diff_data) then + tab_names[#tab_names + 1] = tab_name + end + end + for _, tab_name in ipairs(tab_names) do + if M.close_diff_by_tab_name(tab_name) then + count = count + 1 + end + end + if count > 0 then + logger.debug("diff", "Closed", count, "active diff(s):", reason) + end + return count +end + +---Close all active diffs, resolving any still pending as rejected. +---Closes diffs in ANY state (including saved), so its only caller is the +---closeAllDiffTabs tool, where Claude is the connected client and has written +---accepted files. Automatic cleanup and the :ClaudeCodeCloseAllDiffs command +---deliberately use close_pending_diffs / close_diffs_for_client instead, to +---leave already-saved diffs alone -- see close_pending_diffs for why. +---@param reason string Human-readable reason (for logging) +---@return number count Number of diffs closed +function M.close_all_diffs(reason) + return close_active_diffs(nil, reason or "close all diffs") +end + +-- Automatic teardown (client disconnect, server stop) must only touch *pending* +-- diffs. A diff with status == "saved" has been :w'd by the user -- its edits +-- live only in the proposed buffer until Claude writes them to disk -- so closing +-- it would run close_diff_by_tab_name's saved-branch, wiping the proposed buffer +-- and reloading the file from unchanged disk, silently destroying the edits if +-- Claude died before writing. Pending diffs carry no such accepted content. + +---Close every still-pending diff (e.g. on server stop, which bypasses +---on_disconnect). Leaves saved/rejected diffs for client-driven finalization. +---@param reason string Human-readable reason (for logging) +---@return number count Number of diffs closed +function M.close_pending_diffs(reason) + return close_active_diffs(function(diff_data) + return diff_data.status == "pending" + end, reason or "close pending diffs") +end + +---Close the still-pending diffs opened by a specific MCP client, used when that +---client disconnects so its orphaned diff windows don't linger (e.g. the Claude +---session that opened them exited or moved to remote control). +---@param client_id string The id of the client whose diffs should be closed +---@param reason string Human-readable reason (for logging) +---@return number count Number of diffs closed +function M.close_diffs_for_client(client_id, reason) + if not client_id then + return 0 + end + return close_active_diffs(function(diff_data) + return diff_data.client_id == client_id and diff_data.status == "pending" + end, reason or ("client " .. tostring(client_id))) +end + ---Test helper function (only for testing) ---@return table active_diffs The active diffs table function M._get_active_diffs() diff --git a/lua/claudecode/init.lua b/lua/claudecode/init.lua index c4b7744e..550a4022 100644 --- a/lua/claudecode/init.lua +++ b/lua/claudecode/init.lua @@ -1042,6 +1042,22 @@ function M._create_commands() desc = "Deny/reject the current diff changes", }) + vim.api.nvim_create_user_command("ClaudeCodeCloseAllDiffs", function() + -- Pending only: a status="saved" diff holds the user's :w'd edits in its + -- proposed buffer until Claude writes the file, and closing it would discard + -- them (same data-loss branch the auto-cleanup avoids). So this clears + -- orphaned proposals but leaves accepted diffs for the user to handle. + local diff = require("claudecode.diff") + local count = diff.close_pending_diffs("user command") + if count > 0 then + vim.notify(("Closed %d pending Claude diff(s)"):format(count), vim.log.levels.INFO) + else + vim.notify("No pending Claude diffs to close", vim.log.levels.WARN) + end + end, { + desc = "Close pending Claude Code diffs (leaves accepted/saved diffs intact)", + }) + vim.api.nvim_create_user_command("ClaudeCodeSelectModel", function(opts) local cmd_args = opts.args and opts.args ~= "" and opts.args or nil M.open_with_model(cmd_args) diff --git a/lua/claudecode/server/init.lua b/lua/claudecode/server/init.lua index 9da47080..07a5248f 100644 --- a/lua/claudecode/server/init.lua +++ b/lua/claudecode/server/init.lua @@ -76,6 +76,16 @@ function M.start(config, auth_token) ", reason:", (reason or "N/A") .. ")" ) + + -- Close diffs this client opened but never resolved (issue #248) -- only if + -- the diff module is in use. Scheduled: diff cleanup touches window APIs. + local diff = package.loaded["claudecode.diff"] + if diff then + local client_id = client.id + vim.schedule(function() + diff.close_diffs_for_client(client_id, "client disconnected") + end) + end end, on_error = function(error_msg) logger.error("server", "WebSocket server error:", error_msg) @@ -109,6 +119,15 @@ function M.stop() M.state.ping_timer = nil end + -- Reject any still-pending diffs before teardown -- stop_server bypasses + -- on_disconnect (#248). Pending only, so saved-but-unflushed edits survive; + -- only if the diff module is in use, and while clients can still receive + -- DIFF_REJECTED. + local diff = package.loaded["claudecode.diff"] + if diff then + diff.close_pending_diffs("server stopping") + end + tcp_server.stop_server(M.state.server) -- CRITICAL: Clear global deferred responses to prevent memory leaks and hanging diff --git a/lua/claudecode/tools/close_all_diff_tabs.lua b/lua/claudecode/tools/close_all_diff_tabs.lua index 30c706a7..00afd536 100644 --- a/lua/claudecode/tools/close_all_diff_tabs.lua +++ b/lua/claudecode/tools/close_all_diff_tabs.lua @@ -15,7 +15,12 @@ local schema = { local function handler(params) local closed_count = 0 - -- Get all windows + -- Tear down tracked diffs first (resolving their pending coroutines); the + -- window/buffer scan below would otherwise leak that diff state (issue #248). + local diff = require("claudecode.diff") + closed_count = closed_count + diff.close_all_diffs("closeAllDiffTabs tool") + + -- Get all windows (catches any untracked diff windows, e.g. fugitive) local windows = vim.api.nvim_list_wins() local windows_to_close = {} -- Use set to avoid duplicates diff --git a/lua/claudecode/tools/init.lua b/lua/claudecode/tools/init.lua index a2219de1..06ec567f 100644 --- a/lua/claudecode/tools/init.lua +++ b/lua/claudecode/tools/init.lua @@ -105,7 +105,7 @@ function M.handle_invoke(client, params) -- client needed for blocking tools -- Wrap in coroutine for blocking behavior require("claudecode.logger").debug("tools", "Wrapping " .. tool_name .. " in coroutine for blocking behavior") local co = coroutine.create(function() - return tool_data.handler(input) + return tool_data.handler(input, client) end) require("claudecode.logger").debug("tools", "About to resume coroutine for " .. tool_name) diff --git a/lua/claudecode/tools/open_diff.lua b/lua/claudecode/tools/open_diff.lua index fdb483de..c3fc9115 100644 --- a/lua/claudecode/tools/open_diff.lua +++ b/lua/claudecode/tools/open_diff.lua @@ -32,8 +32,9 @@ local schema = { ---Opens a diff view and blocks until user interaction (save/close). ---Returns MCP-compliant response with content array format. ---@param params table The input parameters for the tool +---@param client table|nil The MCP client that invoked the tool (used to clean up the diff if the client disconnects) ---@return table response MCP-compliant response with content array -local function handler(params) +local function handler(params, client) -- Validate required parameters local required_params = { "old_file_path", "new_file_path", "new_file_contents", "tab_name" } for _, param_name in ipairs(required_params) do @@ -67,7 +68,8 @@ local function handler(params) params.old_file_path, params.new_file_path, params.new_file_contents, - params.tab_name + params.tab_name, + client and client.id or nil ) if not success then diff --git a/scripts/repro_issue_248.sh b/scripts/repro_issue_248.sh new file mode 100755 index 00000000..61267df9 --- /dev/null +++ b/scripts/repro_issue_248.sh @@ -0,0 +1,152 @@ +#!/usr/bin/env bash +# +# repro_issue_248.sh — Reproduce GitHub issue #248 +# "[FEATURE] Close diff handled by remote control" +# +# Symptom: diffs that Claude opens in Neovim via the `openDiff` MCP tool stay +# open forever when they are resolved *somewhere other than this Neovim* +# (e.g. Claude "remote control" on a phone) — because Neovim only ever closes a +# diff when the connected client sends a `close_tab` / `closeAllDiffTabs` call, +# and that signal is never delivered for diffs resolved out-of-band. +# +# This script acts as the MCP client (i.e. it plays the role of Claude). It: +# 1. discovers the running claudecode.nvim server from its lock file, +# 2. performs the MCP `initialize` handshake, +# 3. opens N diffs via `openDiff` (each blocks server-side — deferred), +# 4. disconnects WITHOUT sending `close_tab`. +# +# After it exits, look at Neovim. With the #248 fix, on_disconnect auto-closes the +# orphaned diffs (`:DiffState` -> windows=1, active_diffs=0). Before the fix the +# client going away left the diff windows open forever — that was the bug. +# +# Usage: +# # Terminal 1 — start the test editor (quiet repro fixture): +# source fixtures/nvim-aliases.sh +# vv remote-diff # or: NVIM_APPNAME=remote-diff XDG_CONFIG_HOME=fixtures nvim a.txt +# +# # Terminal 2 — drive the MCP side: +# scripts/repro_issue_248.sh # open 3 diffs, disconnect -> fix auto-closes them +# scripts/repro_issue_248.sh --cleanup # open 3 diffs, then closeAllDiffTabs +# scripts/repro_issue_248.sh -n 5 # open 5 diffs +# +# In Neovim, run :DiffState (provided by the remote-diff fixture) to print the +# window count and the number of still-"pending" diffs. +# +# Requirements: websocat, jq. + +set -euo pipefail + +NUM_DIFFS=3 +CLEANUP=0 +LOCK_DIR="${CLAUDE_LOCKFILE_DIR:-$HOME/.claude/ide}" + +while [[ $# -gt 0 ]]; do + case "$1" in + -n | --num) + NUM_DIFFS="$2" + shift 2 + ;; + --cleanup) + CLEANUP=1 + shift + ;; + -h | --help) + sed -n '2,40p' "$0" + exit 0 + ;; + *) + echo "Unknown arg: $1" >&2 + exit 2 + ;; + esac +done + +command -v websocat >/dev/null || { + echo "ERROR: websocat not found (try: mise install / brew install websocat)" >&2 + exit 1 +} +command -v jq >/dev/null || { + echo "ERROR: jq not found" >&2 + exit 1 +} + +# --- discover the running server ------------------------------------------- +LOCK_FILE=$(find "$LOCK_DIR" -maxdepth 1 -name '*.lock' -type f 2>/dev/null | head -1 || true) +if [[ -z "$LOCK_FILE" ]]; then + echo "ERROR: no lock file in $LOCK_DIR — is Neovim running with claudecode.nvim started?" >&2 + echo " (in the remote-diff fixture the server auto-starts; otherwise run :ClaudeCodeStart)" >&2 + exit 1 +fi +PORT=$(basename "$LOCK_FILE" .lock) +TOKEN=$(jq -r '.authToken // empty' "$LOCK_FILE") +WORKSPACE=$(jq -r '.workspaceFolders[0] // empty' "$LOCK_FILE") +if [[ -z "$TOKEN" || -z "$WORKSPACE" ]]; then + echo "ERROR: lock file missing authToken/workspaceFolders: $LOCK_FILE" >&2 + exit 1 +fi +echo "server : ws://127.0.0.1:$PORT" +echo "workspace : $WORKSPACE" +echo "action : open $NUM_DIFFS diff(s)$([[ $CLEANUP == 1 ]] && echo ', then closeAllDiffTabs' || echo ', then DISCONNECT (no close_tab)')" +echo + +# --- build the MCP message stream ------------------------------------------ +# We feed websocat from a temp file via `tail -f` so the connection stays open +# while the (blocking/deferred) openDiff calls are in flight. +REQ=$(mktemp -t repro248.XXXXXX) +trap 'rm -f "$REQ"' EXIT + +emit() { printf '%s\n' "$1" >>"$REQ"; } + +# 1) initialize handshake +emit '{"jsonrpc":"2.0","id":1,"method":"initialize","params":{"protocolVersion":"2025-03-26","capabilities":{"roots":{"listChanged":true}},"clientInfo":{"name":"repro-issue-248","version":"1.0.0"}}}' +emit '{"jsonrpc":"2.0","method":"notifications/initialized","params":{}}' + +# pick a couple of real files from the workspace so the diff has content +mapfile -t FILES < <(find "$WORKSPACE" -maxdepth 1 -type f \( -name '*.txt' -o -name '*.lua' -o -name '*.md' \) | sort | head -"$NUM_DIFFS") +if [[ ${#FILES[@]} -eq 0 ]]; then + echo "ERROR: no .txt/.lua/.md files in workspace $WORKSPACE to diff against" >&2 + exit 1 +fi + +# 2) one openDiff per file (these block server-side — responses are deferred) +i=0 +for f in "${FILES[@]}"; do + i=$((i + 1)) + base=$(basename "$f") + tab="✻ [Claude Code] $base (repro$i) ⧉" + contents="$(cat "$f")"$'\n\n-- appended by repro_issue_248.sh (pretend Claude edited this)\n' + msg=$(jq -nc \ + --arg old "$f" --arg new "$f" --arg contents "$contents" --arg tab "$tab" --argjson id "$((100 + i))" \ + '{jsonrpc:"2.0",id:$id,method:"tools/call",params:{name:"openDiff",arguments:{old_file_path:$old,new_file_path:$new,new_file_contents:$contents,tab_name:$tab}}}') + emit "$msg" + echo " -> openDiff: $tab" +done + +if [[ $CLEANUP == 1 ]]; then + emit '{"jsonrpc":"2.0","id":900,"method":"tools/call","params":{"name":"closeAllDiffTabs","arguments":{}}}' + echo " -> closeAllDiffTabs" +fi + +# --- run the connection briefly, then disconnect ---------------------------- +# URL must come BEFORE --header: websocat's --header is variadic and will +# otherwise swallow the URL ("No URL specified"). +( + tail -n +1 -f "$REQ" & + TAIL_PID=$! + # keep the socket open ~4s so diffs render / cleanup runs, then stop feeding + sleep 4 + kill "$TAIL_PID" 2>/dev/null || true +) | websocat -t "ws://127.0.0.1:$PORT" --header "x-claude-code-ide-authorization: $TOKEN" 2>/dev/null || true + +echo +if [[ $CLEANUP == 1 ]]; then + echo "Sent closeAllDiffTabs." + echo ">>> Run :DiffState in Neovim — expect windows=1, active_diffs=0. <<<" + echo " With the #248 fix, closeAllDiffTabs drains the diff registry (resolving" + echo " pending diffs), not just the windows. Pre-fix, active_diffs stayed > 0." +else + echo "Client has DISCONNECTED without sending close_tab." + echo ">>> Run :DiffState in Neovim — expect windows=1, active_diffs=0. <<<" + echo " With the #248 fix, on_disconnect auto-closes this client's pending diffs." + echo " Pre-fix, the $NUM_DIFFS diff window(s) would have stayed open (the bug)." +fi diff --git a/tests/unit/diff_client_disconnect_spec.lua b/tests/unit/diff_client_disconnect_spec.lua new file mode 100644 index 00000000..84624995 --- /dev/null +++ b/tests/unit/diff_client_disconnect_spec.lua @@ -0,0 +1,118 @@ +--- Tests for issue #248: closing diffs that are orphaned when their owning +--- client disconnects (or via the manual "close all diffs" path). +require("tests.busted_setup") +local diff = require("claudecode.diff") + +describe("issue #248: closing orphaned diffs", function() + local file_a = "/tmp/issue248_a.txt" + local file_b = "/tmp/issue248_b.txt" + + before_each(function() + for _, path in ipairs({ file_a, file_b }) do + local f = io.open(path, "w") + f:write("line 1\nline 2\n") + f:close() + end + end) + + after_each(function() + os.remove(file_a) + os.remove(file_b) + diff._cleanup_all_active_diffs("test_cleanup") + end) + + -- Open a pending diff for a given file/tab/client and return a handle whose + -- `.result` is populated once the diff's coroutine resolves. + local function open_pending(file, tab_name, client_id) + local handle = { result = nil } + handle.co = coroutine.create(function() + handle.result = diff.open_diff_blocking(file, file, "line 1\nline 2\nnew line\n", tab_name, client_id) + end) + local ok, err = coroutine.resume(handle.co) + assert.is_true(ok, "diff coroutine should start: " .. tostring(err)) + assert.equal("suspended", coroutine.status(handle.co), "diff should be pending") + return handle + end + + it("records the owning client_id on the diff state", function() + open_pending(file_a, "tab-A", "clientA") + local active = diff._get_active_diffs() + assert.is_table(active["tab-A"]) + assert.equal("clientA", active["tab-A"].client_id) + end) + + it("close_diffs_for_client rejects + removes only that client's diffs", function() + local a = open_pending(file_a, "tab-A", "clientA") + local b = open_pending(file_b, "tab-B", "clientB") + + local closed = diff.close_diffs_for_client("clientA", "test disconnect") + + assert.equal(1, closed) + -- clientA's diff resolved as rejected and removed from the registry + assert.equal("dead", coroutine.status(a.co)) + assert.is_table(a.result) + assert.equal("DIFF_REJECTED", a.result.content[1].text) + assert.is_nil(diff._get_active_diffs()["tab-A"]) + -- clientB's diff is untouched + assert.equal("suspended", coroutine.status(b.co)) + assert.is_table(diff._get_active_diffs()["tab-B"]) + end) + + it("close_diffs_for_client with an unknown client closes nothing", function() + open_pending(file_a, "tab-A", "clientA") + assert.equal(0, diff.close_diffs_for_client("nobody", "test")) + assert.is_table(diff._get_active_diffs()["tab-A"]) + end) + + it("close_diffs_for_client(nil) is a no-op", function() + open_pending(file_a, "tab-A", "clientA") + assert.equal(0, diff.close_diffs_for_client(nil, "test")) + assert.is_table(diff._get_active_diffs()["tab-A"]) + end) + + it("close_all_diffs rejects every diff and drains active_diffs", function() + local a = open_pending(file_a, "tab-A", "clientA") + local b = open_pending(file_b, "tab-B", "clientB") + + local closed = diff.close_all_diffs("test all") + + assert.equal(2, closed) + assert.equal("dead", coroutine.status(a.co)) + assert.equal("dead", coroutine.status(b.co)) + assert.equal("DIFF_REJECTED", a.result.content[1].text) + assert.equal("DIFF_REJECTED", b.result.content[1].text) + -- registry fully drained (this is the secondary closeAllDiffTabs bug) + assert.is_nil(next(diff._get_active_diffs())) + end) + + -- A status="saved" diff still holds the user's :w'd edits only in its proposed + -- buffer (until Claude writes the file). Auto-cleanup must NOT close it, or those + -- edits are silently destroyed if Claude died before writing. Pending diffs only. + it("close_diffs_for_client leaves SAVED diffs alone (preserves the user's edits)", function() + open_pending(file_a, "tab-A", "clientA") + local saved_buf = diff._get_active_diffs()["tab-A"].new_buffer + diff._resolve_diff_as_saved("tab-A", saved_buf) -- user accepted: status -> saved + assert.equal("saved", diff._get_active_diffs()["tab-A"].status) + + local closed = diff.close_diffs_for_client("clientA", "disconnect") + + assert.equal(0, closed) + assert.is_table(diff._get_active_diffs()["tab-A"]) + assert.equal("saved", diff._get_active_diffs()["tab-A"].status) + end) + + it("close_pending_diffs closes pending diffs but leaves saved ones", function() + local a = open_pending(file_a, "tab-A", "clientA") -- stays pending + open_pending(file_b, "tab-B", "clientB") + local saved_buf = diff._get_active_diffs()["tab-B"].new_buffer + diff._resolve_diff_as_saved("tab-B", saved_buf) -- tab-B -> saved + + local closed = diff.close_pending_diffs("server stopping") + + assert.equal(1, closed) + assert.equal("dead", coroutine.status(a.co)) + assert.is_nil(diff._get_active_diffs()["tab-A"]) -- pending closed + assert.is_table(diff._get_active_diffs()["tab-B"]) -- saved preserved + assert.equal("saved", diff._get_active_diffs()["tab-B"].status) + end) +end) diff --git a/tests/unit/tools/close_all_diff_tabs_spec.lua b/tests/unit/tools/close_all_diff_tabs_spec.lua index 48b9ff9a..ddda1206 100644 --- a/tests/unit/tools/close_all_diff_tabs_spec.lua +++ b/tests/unit/tools/close_all_diff_tabs_spec.lua @@ -115,4 +115,25 @@ describe("Tool: close_all_diff_tabs", function() expect(result.content[1].text).to_be("CLOSED_1_DIFF_TABS") assert.spy(_G.vim.api.nvim_buf_delete).was_called_with(1, { force = true }) end) + + it("drains tracked diffs via diff.close_all_diffs and includes them in the count", function() + -- Stub the diff module so we can assert the handler delegates to it. This is + -- the fix for issue #248's secondary bug: the tool must resolve/clean tracked + -- diffs (which the window/buffer scan never touched), not just close windows. + local saved = package.loaded["claudecode.diff"] + local close_all_spy = spy.new(function() + return 3 + end) + package.loaded["claudecode.diff"] = { close_all_diffs = close_all_spy } + + local success, result = pcall(close_all_diff_tabs_handler, {}) + + package.loaded["claudecode.diff"] = saved + + expect(success).to_be_true() + assert.spy(close_all_spy).was_called() + -- default window/buffer mocks find nothing, so the count is purely the + -- tracked diffs the diff module reported closing. + expect(result.content[1].text).to_be("CLOSED_3_DIFF_TABS") + end) end)