fix(diff): scope closeAllDiffTabs to own diffs; never reuse &diff windows (#277)#290
Merged
Merged
Conversation
…dows (#277) closeAllDiffTabs previously closed every window with `&diff` set and force-deleted any buffer named like `*.diff`/`diff://`/`fugitive://`, without checking that claudecode created them. The Claude CLI invokes this tool at the start of every user turn while an IDE is connected, so an open diffview.nvim, fugitive, or native `:diffsplit` review was wiped out on essentially every prompt — only the diffview file panel (not a `&diff` window) survived, matching the reported symptom. Scope the tool to claudecode's own tracked-diff registry only (drop the window sweep and the buffer force-delete). This mirrors the official VS Code extension, which closes only the diff tabs it labelled "[Claude Code] ..." — an ownership check, not an all-diffs sweep. Also guard the second defect: `find_main_editor_window` (in both open_file.lua and diff.lua) did not exclude windows in diff mode, so openFile/openDiff could `:edit` into one half of a foreign diff, which clears that window's `diff` option and drops its partner out of the diff group. Skip `&diff` windows in both finders, the open_file fallback, and the buffer-reuse branch of the diff setup code. Adds a reproduction fixture (fixtures/issue-277, diffview.nvim) and an agent-tty + MCP harness (scripts/repro_issue_277.sh); both defects flip from 3/3 reproduced to 0/3 with this change. Rewrites the two unit tests that pinned the old destructive sweep and adds &diff coverage to the window-finder specs. Change-Id: If023bdcbcd1bcd10b6169282112ce75e09e17c56 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Signed-off-by: Thomas Kosiewski <tk@coder.com>
Member
Author
|
@codex review |
Member
Author
|
@claude review |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f1771dc6a7
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…diff windows (#277) Review follow-up. The &diff guard added to find_main_editor_window stops openDiff from :edit-ing INTO a foreign diff window, but when the current tab consists ENTIRELY of foreign diff windows (vimdiff/diffview.nvim/ fugitive) the finder returns nil and `_setup_blocking_diff` fell through to the issue #231 same-tab split fallback. A tab has a single shared diff set, so the subsequent `diffthis` made Claude's proposal join — and thus corrupt — the user's existing diff, the same class of breakage #277 is about. Detect that case (current_tab_has_diff_window) and route the diff through the existing new-tab path (display_terminal_in_new_tab + created_new_tab cleanup) instead of splitting the foreign diff tab. The terminal-only fallback (#231) is unchanged: a tab with no diff window still splits in place. Verified in real Neovim: openDiff into an all-diff tab now opens a new tab and leaves the user's 2-way diff intact (2 diff windows, untouched). Adds tests/unit/diff_all_diff_tab_spec.lua and nvim_tabpage_list_wins to the vim mock. Change-Id: I5fe6c0700689cf79cfc216a9e7c90860bc17e0a9 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Signed-off-by: Thomas Kosiewski <tk@coder.com>
Member
Author
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8641c21ec8
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…lready has a diff (#277) Generalizes the previous follow-up. Isolating Claude's diff only when the WHOLE tab was diff windows missed the case where the tab holds a user diff plus a normal editor split: find_main_editor_window() returns that normal window, so target_window is set and the all-diff fallback never ran -- yet `diffthis` still joins the tab's shared diff set and corrupts the user's diff (Neovim diffs are tab-scoped). Check the destination tab (the chosen window's tabpage, or the current tab when none was found) and route to a dedicated tab whenever it already contains any foreign &diff window, regardless of whether a usable non-diff window exists there. The terminal-only #231 split fallback now runs only when the tab has no diff to corrupt. Verified in real Neovim: with a user vimdiff + a normal split in one tab, openDiff used to leave that tab with 4 diff windows (Claude's panes joined the user's diff); it now opens a new tab and the user's diff stays at 2 windows. Generalizes diff_all_diff_tab_spec.lua (adds the mixed diff+editor tab case) and replaces _current_tab_has_diff_window with the tabpage-scoped _tabpage_has_diff_window. Change-Id: Iffcb9361d963023db51ab2200a644545b7ef0bb0 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Signed-off-by: Thomas Kosiewski <tk@coder.com>
Member
Author
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: cbe95fcfd4
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…ver joins a diff (#277) Review follow-up. In the all-diff-window fallback, openFile :vsplits off a diff window; Neovim copies window-local options on split, so the new window inherits `diff`. For most files the subsequent `:edit` clears diff (entering a non-diff buffer), so the user's diff was left intact -- but when the opened file is ITSELF one of the diffed buffers, the window stays in diff mode and shows up as an extra pane in the user's diff set. Run `:diffoff` (current window only) on the new split before editing so the opened file can never participate in the user's diff, closing the last edge of the #277 invariant: openFile/openDiff never disturb a foreign diff. Verified in real Neovim against the real open_file handler: opening a file that is part of an active 2-way vimdiff used to leave 3 diff windows; it now leaves the user's diff at 2 and opens the file in a separate non-diff window. open_file_spec asserts the diffoff precedes the edit. Change-Id: I6edfb9744c1cca0cd8fd191c8a5d8abf728cf574 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Signed-off-by: Thomas Kosiewski <tk@coder.com>
Member
Author
|
@codex review |
|
Codex Review: Didn't find any major issues. Breezy! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
Summary
Fixes #277.
claudecode.nvimwas closing and overwriting diff windows it does not own (diffview.nvim, fugitive, native:diffsplit/vimdiff). Two independent defects:closeAllDiffTabswas unscoped. After draining the plugin's own tracked diffs, it additionally closed every window with&diffset and force-deleted any buffer named like*.diff/diff:///fugitive://, with no check that claudecode created them. The Claude CLI fires this tool at the start of every user turn while an IDE is connected, so any open diff review was destroyed on essentially every prompt — only the diffview file panel (not a&diffwindow) survived, matching the reported symptom.find_main_editor_windowdid not exclude&diffwindows. Duplicated inopen_file.luaanddiff.lua, it skipped floats/terminals/sidebars but not diff windows, soopenFile/openDiffcould:editinto one half of a foreign diff — which clears that window'sdiffoption and drops its partner out of the diff group, corrupting the layout.Changes
tools/close_all_diff_tabs.lua: scope to claudecode's tracked-diff registry only (diff.close_all_diffs); remove the&diffwindow sweep and the buffer force-delete. This mirrors the official VS Code extension, which closes only the diff tabs it labelled[Claude Code] ….tools/open_file.lua&diff.lua: skip windows in diff mode in both copies offind_main_editor_window, in theopen_filefallback, and in the buffer-reuse branch of_setup_blocking_diff. When no non-diff window exists, the existing graceful fallbacks (split + scratch) apply — no error.CHANGELOG.md(Unreleased → Bug Fixes) and thecloseAllDiffTabsdescription inCLAUDE.md.Testing
fixtures/issue-277/(with diffview.nvim) +scripts/repro_issue_277.sh, which drives a real Neovim TUI and sends the same MCP calls the CLI sends. Each phase is written soPASS = bug present. With this change, all three phases flip 3/3 reproduced → 0/3:closeAllDiffTabsreturnsCLOSED_0_DIFF_TABS, diffview/vimdiff windows survive, andopenFileopens in a new window beside the diff.*.diff/fugitive://buffers are spared) and added&diffcoverage to the window-finder specs. Addednvim_win_get_option/nvim_win_set_optionto the shared vim mock.mise run allis green: 655/655 tests,luacheck0 warnings, treefmt clean.🤖 Generated with Claude Code