-
Notifications
You must be signed in to change notification settings - Fork 197
fix(diff): auto-close orphaned diffs on client disconnect (#248) #261
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Changes from all commits
Commits
Show all changes
5 commits
Select commit
Hold shift + click to select a range
cb70c48
fix(diff): auto-close orphaned diffs on client disconnect (#248)
ThomasK33 92f0dc1
test(diff): add remote-diff repro fixture and MCP driver for #248
ThomasK33 a84113e
fix(diff): only auto-close pending diffs to avoid data loss (#248)
ThomasK33 4034776
fix(diff): make :ClaudeCodeCloseAllDiffs preserve saved edits (#248)
ThomasK33 0708560
docs(diff): fix stale comments after close_pending_diffs rewire (#248)
ThomasK33 File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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`). | ||
| - `<leader>as` — run `:DiffState`. | ||
| - `<leader>aa` / `<leader>ad` — accept / deny the focused diff. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,4 @@ | ||
| This is file A. | ||
|
|
||
| Keep this window focused. | ||
| This simulates "you are looking at nvim". |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,2 @@ | ||
| line1 | ||
| line2 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,5 @@ | ||
| local M = {} | ||
| function M.hello() | ||
| return "hello" | ||
| end | ||
| return M |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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", "<leader>aa", "<cmd>ClaudeCodeDiffAccept<cr>", { desc = "Accept diff" }) | ||
| vim.keymap.set("n", "<leader>ad", "<cmd>ClaudeCodeDiffDeny<cr>", { desc = "Deny diff" }) | ||
| vim.keymap.set("n", "<leader>as", "<cmd>DiffState<cr>", { desc = "Show diff state" }) | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.