Skip to content

fix(diff): user-initiated accept/reject should clean up windows and buffers#241

Open
mightreya wants to merge 1 commit intocoder:mainfrom
mightreya:fix/user-resolve-cleanup
Open

fix(diff): user-initiated accept/reject should clean up windows and buffers#241
mightreya wants to merge 1 commit intocoder:mainfrom
mightreya:fix/user-resolve-cleanup

Conversation

@mightreya
Copy link
Copy Markdown

Problem

:ClaudeCodeDiffAccept and :ClaudeCodeDiffDeny (the accept_current_diff / deny_current_diff user commands) leave the diff buffers, split windows, and active_diffs state behind after resolving. Users have to close the leftover windows and buffers by hand.

This shows up as the symptom in #205 and as the partial behavior described in #238 (rejecting via :q closes one buffer but does nothing else).

Root cause

The two user commands call _resolve_diff_as_saved / _resolve_diff_as_rejected, which only set the diff status and resume the MCP coroutine. The actual UI cleanup lives in _cleanup_diff_state and is triggered exclusively by the close_tab MCP tool.

But close_tab has schema = nil and is documented as "Internal tool — must remain as requested by user" (see lua/claudecode/tools/close_tab.lua). The Claude CLI never invokes it. So when a user resolves a diff via the bundled commands, cleanup never fires.

This was less visible before #175 because the cleanup function did less work; with #175 expanding cleanup to close plugin-created splits and force-delete buffers, the gap between "resolve" and "cleanup" became user-facing as accumulating diff windows.

Fix

Have the user commands call _cleanup_diff_state themselves. They're not waiting for an MCP response from the CLI — they're driven by the user — so the "wait for close_tab" architecture doesn't apply to them.

This PR does not touch close_tab.lua. Its schema = nil is left intact per the existing comment.

It also extends diff_opts.keep_terminal_focus to apply at user-resolve time, not only at diff-open time, so focus returns to the Claude terminal after accept/reject.

Tests

Adds tests/unit/diff_user_command_cleanup_spec.lua covering:

  • accept_current_diff closes the diff window and removes the entry from active_diffs
  • deny_current_diff closes the diff window and removes the entry from active_diffs
  • accept_current_diff is a safe no-op when called outside a diff buffer

I was unable to run make test locally (the Nix-based dev shell wasn't available on my machine). I confirmed Lua syntax via loadfile. Please run the suite in CI / locally before merging.

Closes / refs

…uffers

The user commands :ClaudeCodeDiffAccept and :ClaudeCodeDiffDeny call
_resolve_diff_as_saved/_rejected, which only mark the diff status. Real
cleanup (close windows, force-delete buffers, remove from active_diffs)
is performed by _cleanup_diff_state, currently triggered exclusively by
the close_tab MCP tool.

But close_tab has schema = nil (it is intentionally internal-only, as
documented in the source), so the Claude CLI never invokes it. The result
is that user-initiated accept/reject leaves the diff windows and buffers
behind, requiring manual cleanup.

The fix: have the user commands call _cleanup_diff_state themselves,
since they are not waiting for an MCP response from the CLI. Also
restore terminal focus when diff_opts.keep_terminal_focus is set
(previously this only applied at diff-open time).

Adds unit tests covering both commands and the no-marker no-op case.

Closes coder#205, partially addresses coder#238.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[BUG] Diff panels not cleared when new suggestion is shown — stale diffs accumulate until window focus change

1 participant