Skip to content

fix(diff): rename a diff buffer that a plugin reload orphaned - #512

Open
ThomasK33 wants to merge 23 commits into
mainfrom
fix/reload-proposal-name-e95
Open

ThomasK33 wants to merge 23 commits into
mainfrom
fix/reload-proposal-name-e95

Conversation

@ThomasK33

@ThomasK33 ThomasK33 commented Nov 11, 2026 •

Copy link
Copy Markdown
Member

Why

After a plugin reload (:Lazy reload), an openDiff for a tab name whose old diff buffer is still loaded fails with Vim:E95: Buffer with this name already exists. The user gets no diff. The reload clears the diff module, so the new copy has no active diff. The old proposal buffer stays loaded under <tab_name> (proposed), and naming the new proposal buffer collides with it. For a new file the empty original buffer (<file> (NEW FILE)) collides the same way. A setup that fails in the command-line window leaves its proposal buffer behind (E11 stops the delete), and the next openDiff with that tab name fails the same way.

The root verifier of #511 found the reload case, and it is on main too. A user reaches it when Claude sends the same tab name again after a reload. I did not verify how often that happens. I reproduced the failing state with real Neovim.

What changed

  1. Before a diff names its buffers, _setup_blocking_diff renames the buffers that would collide (diff.lua, rename_orphaned_diff_buffers), in both layouts. It renames an unlisted acwrite buffer under <tab_name> (proposed), <tab_name> (NEW FILE - proposed) or <tab_name> (unified diff), and an unlisted nofile buffer under <file> (NEW FILE), to <name> (orphaned <bufnr>). It skips every buffer that an active diff owns.
  2. A name counts with and without the symlinks of its folder resolved, because Neovim stores a name with its folder resolved. Two tests pin the two spellings. The first opens a new file in a folder that a symlink reaches. The second opens a new file in a folder that does not exist yet under a symlinked parent, where Neovim keeps the name unresolved.
  3. The old buffer keeps its windows and its text. After an accept, that text can be the only copy of the accepted edits, and the module README already says the proposal buffers stay after a reload. So the fix renames and does not wipe.
  4. Every buffer that no active diff owns loses its claudecode_diff_tab_name for that tab name, whatever its name. Without this, :ClaudeCodeDiffAccept in an old window resolved the new diff with the old text. That held for the unified layout, for a layout change across the reload, and after a :cd. Now it says "No active diff found in current buffer".
  5. The unified layout names its buffer inside a pcall, so a collision left the new unified buffer unnamed. The rename frees that name too, and the new unified buffer now gets it.
  6. When the original buffer cannot be loaded, the setup keeps main's window handling. An earlier version of this PR closed the window that the split created. It chose that window by what the split cloned, and a user's WinNew handler that opened or closed windows during the split made it lose the user's window or text. The round-5 verifier reported that at 05c568cc. This version leaves _create_diff_view_from_window as it is on main. The failed-load cases now check that no window that existed before the call closes. One extra window can stay after such a failure, as on main.
  7. A second openDiff for a tab name whose first setup has started but has not registered its diff yet (a user handler runs the event loop during the setup, so Claude's queued call runs inside it) gets a -32000 "Diff tab in use" error and touches nothing. The refusal holds for that tab name only. A call for another tab name runs as on main, and a test pins that. Main answered this nested call with E95 in the split layout and left it unanswered in the unified layout. Main also leaves a nested call unanswered in the split layout in some orders: C three calls deep, a BufNew or BufFilePre handler before naming without a leftover buffer, and a TabEnter handler with open_in_new_tab. A table in _G marks the setup (setups_in_flight), so a plugin reload inside the setup keeps the mark. _register_diff_state clears it, and so does the line after the setup's pcall, so a ClaudeCodeDiffOpened handler can still replace the diff.
  8. Outside that nested case, every other collision keeps main's behavior. In the split layout, a listed buffer, a buffer of another kind, or a buffer that a live diff owns under such a name still raises E95.
  9. tests/mocks/vim.lua gains fn.resolve (identity), which the rename calls. lua/claudecode/README.md describes the rename, the window rule and the refusal.

This head contains origin/main at 81ddcb3c, which has the replace loop of #536 in open_diff_blocking. The loop in this head is main's text: I compared the function up to -- Set up blocking diff operation and it is byte-identical. The merge had three conflicts. In lua/claudecode/README.md I kept main's line for close_saved_diffs and this PR's line for active_diffs. In tests/mocks/vim.lua I kept fn.resolve and main's fn.simplify and fn.wordcount. In covered.lock I took main's file and recorded it again in its own commit.

covered.lock has new hashes for diff.lua and for the setup_blocking and register_state blocks. The change resolves, closes and answers no diff differently outside the refusal. The refusal is a setup error, which the model leaves out. The model has no buffers or windows, so it still describes the code. check.sh passes (93477 and 5557 states).

Evidence

All numbers below come from my runs on this head (1ec2909c) and on origin/main at 81ddcb3c, with real headless Neovim 0.12.3 and v0.8.0, scratch XDG dirs, a stub in place of the claude CLI, and tools.handle_invoke plus the real commands.

I reproduced E95 first on main. With a partial reload (only claudecode.diff cleared) and with a full reload, an openDiff for a tab name with a leftover proposal buffer fails with E95 on main (I1 and V2 below). On this head the diff opens, one active diff, the new proposal buffer under the name, and the old buffer renamed to ... (proposed) (orphaned 2) with its text and window.

Spec tests/integration/diff_leftover_buffer_name_spec.lua: 47 cases (45 run on Neovim 0.8, because two need 0.9).

Code under the spec Neovim 0.12.3 Neovim v0.8.0
origin/main 30 fail, 17 pass 28 fail, 17 pass
this head 47 pass 45 pass, 2 pending

The spec of #536 (diff_answer_every_call_spec.lua) passes 9 of 9 on both versions on this head.

Mutation: 26 mutants of diff.lua, run against the spec. 25 are red on 0.12.3 and 24 on v0.8.0. They drop the rename call, each ownership check, the buftype and buflisted checks, each of the four names, each spelling, the unbind (also the version that ignores ownership), the unified call, the refusal, the mark, each of its clears, the _G table, and they restore the window code of 05c568cc. Two survive. K08 (no clear in _register_diff_state) is red on 0.12.3 and survives on v0.8.0, because its case is pending there. K26 (the mark set inside the pcall after the dirty check) survives on both versions. Only filereadable and the dirty check run between the two places, so the mark has no other effect there. The two new tests kill the two mutants that the round-6 verifier found alive, on both versions. K15 (only the resolved spelling) turns the new symlink test red, and K23 (refusal for any tab name) turns both layouts of the other-tab test red. K16 (only the plain spelling) was already red.

Rows on this head and on main (probes of the round-6 verifiers and my own):

Row main 81ddcb3c this head
H1: leftover buffer, X a new file, a close handler runs a queued call B X rejected, A fails with E95, B fails with E95, 1 tab X and B rejected, A opens and is answered at the accept, 1 tab
F-A: a WinNew or WinClosed handler during the split, 4 orders the user's window and text stay in 4 of 4 same as main (05c568cc loses them in 3 of 4)
R1: reload inside A's setup, a queued call B, 4 orders no wrong text. In the unified layout B is served and the old-window accept answers it no wrong text. B is refused. The old-window accept answers nothing (A stays open in the old module, as on main)
I1, V2, W1, N1 (leftover, a handler on the naming event pumps the queued call B) A and B fail with E95, or B is served and A rejected A opens, B is refused, A is answered at close-all, one answer each
Three deep (A, B, C) split: A and B fail, C is never answered. Unified: B gets two answers A rejected, B refused, C opens and is answered at the accept, one answer each
Three queued calls every call answered once, no wrong text same

Scenario sets of the round-6 verifier, one run per scenario, version and tree (head against main): core (112 runs per tree) has 26 better, 84 equal and 2 worse. The 2 worse are R1L, see below. R1 after-actions (14) has 10 worse, all R1L. V3 (12) has 2 better and 10 equal. Three deep (12) has 6 better and 6 equal. The event matrix (768) has 160 better and 608 equal, none worse. The default-notify matrix (384) has 116 better, 264 equal and 4 worse, all V3x (see below). F-A keeps the user's work in 16 of 16 runs. No row on this head returns another call's text.

Gate on this head: mise run clean && mise run all exit 0 (1908 successes, 0 failures, 38 pending, luacheck 0 warnings in 202 files). The integration runner passes with 912 successes and 0 failures on 0.12.3 and 906 and 0 on v0.8.0, each also under a TMPDIR with a space and a %. All five check.sh pass, and each --hash-only is fresh.

Not in this PR (follow-ups)

The two items R1L and V3x are worse than main, and the maintainer chose to ship them as stated limits. R1L ships because #536 was narrowed and dropped the reload-during-setup step that was meant to land first. Both, and F2, are tracked in #603.

  • R1L: a reload inside the setup of A, with a leftover buffer under A's name, leaves A open in the old module. The rename lets A open where main fails it with E95, and then main's reload semantics apply: the accept and close-all of the new module do not answer A, and its tab and windows stay (1 to 2 tabs and 3 to 5 windows after close-all). :w in A's window answers A with its own text. Main leaves A open the same way when no leftover exists (R1 without a leftover, F1). I did not fix that here, because it needs a hand-over of the old module's diff to the new one.
  • V3x: on Neovim v0.8.0 only, in the unified layout, with a FileType handler that pumps for 100 ms, default vim.notify and a leftover buffer, the first call gets an error that carries the refusal text, B is refused, no diff opens, and A's hidden unlisted (unified diff) buffer stays loaded (it is renamed by the next openDiff for the tab name). Main serves A and never answers B. 4 of 384 default-notify runs.
  • F2: an openDiff from a TextYankPost handler fails with E565 (textlock). In the split layout a later openDiff for the same tab name then gets E95, on main and on this head, because the rename does not free that name.
  • Minor, left as is: the rename also runs when no collision would happen (an old-module diff's (NEW FILE) buffer is renamed when another tab opens the same new file after a partial reload). The renamed buffer's window loses its alternate file. A user's unlisted acwrite buffer named exactly <cwd>/<tab> (proposed) is renamed with its text and window kept.

Generated with xum • Model: anthropic:claude-sonnet-5-5 • Thinking: xhigh

A reload clears the diff module, so the new copy knows no diff while the old
proposal buffer stays loaded under its name. An openDiff for the same tab name
then failed at nvim_buf_set_name with E95 (Buffer with this name already
exists) in the split layout, for an existing file and for a new file (the
empty original buffer is named too).

Before it names its buffers, a split diff now renames an unlisted buffer of the
plugin's kind under those names that no active diff owns, to
"<name> (orphaned <bufnr>)". The old buffer keeps its windows and text, which
can be the only copy of accepted edits, and loses its claudecode_diff_tab_name,
so :ClaudeCodeDiffAccept in its window cannot resolve the new diff. A listed
buffer, or one of another kind, under such a name keeps the E95 error.

_Generated with xum • Model: anthropic:claude-sonnet-5-5 • Thinking: xhigh_
_Generated with xum • Model: anthropic:claude-sonnet-5-5 • Thinking: xhigh_
The change renames buffers before a split diff names its own. It reads
active_diffs and resolves, closes, registers and answers no diff, so the model
still describes the code.

_Generated with xum • Model: anthropic:claude-sonnet-5-5 • Thinking: xhigh_
@ThomasK33
ThomasK33 marked this pull request as ready for review November 11, 2026 13:33
@ThomasK33

Copy link
Copy Markdown
Member Author

@codex review

@ThomasK33

Copy link
Copy Markdown
Member Author

@codex security review

@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Nov 11, 2026 •

Copy link
Copy Markdown
Codex Review Summary: ✅ Completed

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-11-13T13:20:26.355608Z 1ec2909 Manual request
🔒 Security Review ✅ Completed 2026-11-13T13:20:16.797804Z 1ec2909 Manual request
ℹ️ 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" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. Keep them coming!

Reviewed commit: 3d55aa51fd

ℹ️ 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".

@chatgpt-codex-connector

Copy link
Copy Markdown

🛡️ Codex Security Review

Security review completed. No security issues were found in this pull request.

Reviewed commit: 3d55aa51fd

View security finding report

Only the user who started this review can view the report in Codex.

ℹ️ About Codex security reviews in GitHub

This is an experimental Codex feature. Security reviews are triggered when:

  • You comment "@codex security review"
  • A regular code review gets triggered (for example, "@codex review" or when a PR is opened), and you’re opted in so security review runs alongside code review

Once complete, Codex will leave suggestions, or a comment if no findings are found.

…ommand-line window

A setup that fails in the command-line window cannot delete its proposal buffer
(E11), so the next openDiff for that tab name failed with E95. The rename of an
orphaned buffer frees the name for this case too. Rename the spec and retitle it
for both leftovers, and say so in the module README.

_Generated with xum • Model: anthropic:claude-sonnet-5-5 • Thinking: xhigh_
…ayout

A name that Neovim stores with its folder's symlinks resolved was never found
by the rename, so a new file in such a folder still failed with E95 after a
reload. Compare both spellings, close the window that the setup split off when
the new file's original buffer cannot take its name, and run the rename for
both layouts with the unified buffer's name included. The unified layout hid its
E95 behind a pcall and left the new buffer unnamed, and an old window of either
layout could resolve a new diff of the same tab name.

_Generated with xum • Model: anthropic:claude-sonnet-5-5 • Thinking: xhigh_
The rename call moved before the layout dispatch in _setup_blocking_diff. It
reads active_diffs and resolves, closes, registers and answers no diff, so the
model still describes the code.

_Generated with xum • Model: anthropic:claude-sonnet-5-5 • Thinking: xhigh_
… mock resolve

A reload followed by :cd left the old buffer under a name that the rename could
not match, so :ClaudeCodeDiffAccept in its window still resolved the new diff.
Clear the tab name on every buffer that no active diff owns, whatever its name,
before the rename. The README no longer claims an E95 for the unified layout or
that the original buffer must be empty. The spec restores the working folder
after each case, and the vim mock gains fn.resolve, which the rename now calls.

_Generated with xum • Model: anthropic:claude-sonnet-5-5 • Thinking: xhigh_
The change reads active_diffs and buffer variables and clears one buffer
variable. It resolves, closes, registers and answers no diff, so the model
still describes the code.

_Generated with xum • Model: anthropic:claude-sonnet-5-5 • Thinking: xhigh_
@ThomasK33

Copy link
Copy Markdown
Member Author

@codex review

@ThomasK33

Copy link
Copy Markdown
Member Author

@codex security review

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. Can't wait for the next one!

Reviewed commit: 8c74abdbd4

ℹ️ 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".

@chatgpt-codex-connector

Copy link
Copy Markdown

🛡️ Codex Security Review

Security review completed. No security issues were found in this pull request.

Reviewed commit: 8c74abdbd4

View security finding report

Only the user who started this review can view the report in Codex.

ℹ️ About Codex security reviews in GitHub

This is an experimental Codex feature. Security reviews are triggered when:

  • You comment "@codex security review"
  • A regular code review gets triggered (for example, "@codex review" or when a PR is opened), and you’re opted in so security review runs alongside code review

Once complete, Codex will leave suggestions, or a comment if no findings are found.

The failed-setup cleanup closed original_window, which a user's WinNew handler
can make its own window, and lost that window's unsaved text while the plugin's
split stayed. Close only the new windows that show the buffer the split cloned.

A second openDiff for a tab name whose first setup had made its buffers but had
not registered its diff yet (a user handler that runs the event loop during the
setup) renamed the first diff's buffer and stranded itself. It now gets a "Diff
tab in use" error and touches nothing. The flag clears when the diff registers
or the setup fails, so a handler of ClaudeCodeDiffOpened can still replace it.

Drop the root-folder spelling, which the plain spelling already covers, and add
the tests that kill the surviving mutants.

_Generated with xum • Model: anthropic:claude-sonnet-5-5 • Thinking: xhigh_
The flag only refuses a second openDiff before it creates anything, and
_register_diff_state clears it. No diff is resolved, closed or answered
differently, and the refusal is a setup error, which the model leaves out, so
the model still describes the code.

_Generated with xum • Model: anthropic:claude-sonnet-5-5 • Thinking: xhigh_
… doc right

Closing only the windows that still show the cloned buffer left the split's
window open when a handler raised after the original buffer loaded into it, and
closed a window of the user that showed the clone. Remember the split's own
window, the first new one, and close that. Say "started" for the in-flight flag,
drop the unused answer counters from the spec, and add the two regressions.

_Generated with xum • Model: anthropic:claude-sonnet-5-5 • Thinking: xhigh_
_Generated with xum • Model: anthropic:claude-sonnet-5-5 • Thinking: xhigh_
_Generated with xum • Model: anthropic:claude-sonnet-5-5 • Thinking: xhigh_
@ThomasK33

Copy link
Copy Markdown
Member Author

@codex review

@ThomasK33

Copy link
Copy Markdown
Member Author

@codex security review

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. Breezy!

Reviewed commit: 8e8d4d2dae

ℹ️ 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".

@chatgpt-codex-connector

Copy link
Copy Markdown

🛡️ Codex Security Review

Security review completed. No security issues were found in this pull request.

Reviewed commit: 8e8d4d2dae

View security finding report

Only the user who started this review can view the report in Codex.

ℹ️ About Codex security reviews in GitHub

This is an experimental Codex feature. Security reviews are triggered when:

  • You comment "@codex security review"
  • A regular code review gets triggered (for example, "@codex review" or when a PR is opened), and you’re opted in so security review runs alongside code review

Once complete, Codex will leave suggestions, or a comment if no findings are found.

Resolve the covered.lock conflict with #501 by recording the merged code. #501
changes the legacy diff's temp file, which the model does not describe, and this
branch changes the setup of a diff, which it reads only through active_diffs.

_Generated with xum • Model: anthropic:claude-sonnet-5-5 • Thinking: xhigh_
@ThomasK33

Copy link
Copy Markdown
Member Author

@codex review

@ThomasK33

Copy link
Copy Markdown
Member Author

@codex security review

@chatgpt-codex-connector

Copy link
Copy Markdown

🛡️ Codex Security Review

Security review completed. No security issues were found in this pull request.

Reviewed commit: 592dffaea4

View security finding report

Only the user who started this review can view the report in Codex.

ℹ️ About Codex security reviews in GitHub

This is an experimental Codex feature. Security reviews are triggered when:

  • You comment "@codex security review"
  • A regular code review gets triggered (for example, "@codex review" or when a PR is opened), and you’re opted in so security review runs alongside code review

Once complete, Codex will leave suggestions, or a comment if no findings are found.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 592dffaea4

ℹ️ 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".

Comment thread lua/claudecode/diff.lua Resolved
A WinNew handler that closes the window the split created and opens another one
made the new cleanup take the user's window for the split's, and a failed load
then closed it with its unsaved text. Take the first new window that shows the
buffer the split cloned, so a window that shows something else is never the
split's.

_Generated with xum • Model: anthropic:claude-sonnet-5-5 • Thinking: xhigh_
_Generated with xum • Model: anthropic:claude-sonnet-5-5 • Thinking: xhigh_
@ThomasK33

Copy link
Copy Markdown
Member Author

@codex review

@ThomasK33

Copy link
Copy Markdown
Member Author

@codex security review

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. 👍

Reviewed commit: 05c568ccf3

ℹ️ 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".

@chatgpt-codex-connector

Copy link
Copy Markdown

🛡️ Codex Security Review

Security review completed. No security issues were found in this pull request.

Reviewed commit: 05c568ccf3

View security finding report

Only the user who started this review can view the report in Codex.

ℹ️ About Codex security reviews in GitHub

This is an experimental Codex feature. Security reviews are triggered when:

  • You comment "@codex security review"
  • A regular code review gets triggered (for example, "@codex review" or when a PR is opened), and you’re opted in so security review runs alongside code review

Once complete, Codex will leave suggestions, or a comment if no findings are found.

…t table in _G

A failed original-buffer load closed the split's window by a rule on the window
list. A user handler can own that window and the text in it, so a user's
unsaved text was lost where main kept it. The window code is main's again.

The in-flight table of tab names lived in the diff module. A plugin reload
during a setup gave the new module an empty table, so a second openDiff for the
same tab name ran inside the first setup and took its proposal buffer. The table
now lives in _G, shared by every module generation.

_Generated with xum • Model: anthropic:claude-sonnet-5-5 • Thinking: xhigh_
… the queued-call refusals

R1: a plugin reload inside the first setup (diff module only, or every module,
in both layouts) refuses the second openDiff, and the first diff's buffer keeps
its name and tab name. The window cases run against main's window code: a user's
unsaved text stays when a handler closes the split's window, or puts text in it,
and a later handler raises. The refusal cases queue a second call (and a third)
inside the first setup through BufFilePost, BufFilePre, BufNew, BufWipeout and
BufEnter handlers, and check that each call is answered once, with its own text.

_Generated with xum • Model: anthropic:claude-sonnet-5-5 • Thinking: xhigh_
…main's window code

Only the diff.lua file hash changed. The change restores main's window code in
_create_diff_view_from_window and moves the in-flight table to _G. Neither
resolves, closes or answers a diff, and the model leaves setup errors out.

_Generated with xum • Model: anthropic:claude-sonnet-5-5 • Thinking: xhigh_
@ThomasK33

Copy link
Copy Markdown
Member Author

Review-cap waiver (maintainer answer in the Xum chat, Option A, 2026-11-12T16:01Z).

This PR has used 10 Codex review requests (each code review request and each security review request counts as one), past the cap of 6. The maintainer waived requests 11 and 12 for exactly one push, made after #536 merges:

  • The push holds candidate a1 (it fixes the lost user text and the wrong text sent to Claude), two tests, and a merge of main.
  • It gets one Codex code review request and one Codex security review request, one Opus verifier and the clean-context advisor before the merge.
  • Stop condition: a new security flaw, data loss or broken core behavior ends this PR as blocked, and it is closed. No further push follows under this waiver.

Generated with xum • Model: anthropic:claude-opus-5-5 • Thinking: high

…he per-tab scope of the refusal

The first test opens a new-file diff in a folder that does not exist yet under a
symlinked parent, after a reload. Neovim stores that name unresolved, so the
rename of the leftover buffer must know the unresolved spelling too. The second
test opens an openDiff for another tab name inside the setup of a diff, and it
must open: the refusal holds for the tab name that is being set up only.

_Generated with xum • Model: anthropic:claude-sonnet-5-5 • Thinking: xhigh_
# Conflicts:
#	.agents/skills/verify-diff-lifecycle/covered.lock
#	lua/claudecode/README.md
#	tests/mocks/vim.lua
…ffer rename after the merge of main

The hashed blocks setup_blocking and register_state changed: the setup marks its
tab name in a table that outlives a reload, and _register_diff_state clears the
mark. All code facts stay TRUE, and the replace loop of open_diff_blocking is
main's text.

_Generated with xum • Model: anthropic:claude-sonnet-5-5 • Thinking: xhigh_
@ThomasK33

Copy link
Copy Markdown
Member Author

Maintainer decision (ThomasK33, 2026-11-13, in the agent chat): option A for this PR.

The one waived push (requests 11 and 12, comment above) carries local head 1ec2909c: candidate a1, the two tests for the unresolved spelling and the per-tab scope, and a merge of main with #536. Before the push, that head was measured against main. It is worse in two rare cases, which the body states as limits:

  • R1L: a plugin reload inside a diff's setup, with a leftover buffer under the diff's name. Main fails the diff with E95. This head opens it in the old module, as main does without a leftover, and close-all leaves an extra tab and windows. It ships because the narrowed fix(diff): answer a call that a close handler lets register while a diff is replaced #536 dropped the reload-during-setup step that was meant to fix this first.
  • V3x: Neovim 0.8.0 only, unified layout, a FileType handler that runs the event loop and a leftover buffer. A hidden proposal buffer stays loaded until the next openDiff renames it.

Both are tracked in #603. The waiver's stop condition still holds: a new security flaw, data loss or broken core behavior from Codex or the verifier on the pushed head closes this PR.


Generated with xum • Model: anthropic:claude-opus-5-5 • Thinking: high

@ThomasK33

Copy link
Copy Markdown
Member Author

@codex review

@ThomasK33

Copy link
Copy Markdown
Member Author

@codex security review

@chatgpt-codex-connector

Copy link
Copy Markdown

🛡️ Codex Security Review

Security review completed. No security issues were found in this pull request.

Reviewed commit: 1ec2909cb1

View security finding report

Only the user who started this review can view the report in Codex.

ℹ️ About Codex security reviews in GitHub

This is an experimental Codex feature. Security reviews are triggered when:

  • You comment "@codex security review"
  • A regular code review gets triggered (for example, "@codex review" or when a PR is opened), and you’re opted in so security review runs alongside code review

Once complete, Codex will leave suggestions, or a comment if no findings are found.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 1ec2909cb1

ℹ️ 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".

Comment thread lua/claudecode/diff.lua
Comment on lines +1094 to +1095
local owned = {}
for _, diff_data in pairs(active_diffs) do

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Refresh ownership after buffer-name autocmds

When renaming a reloaded new-file diff’s proposal fires BufFilePre/BufFilePost, a handler can pump another openDiff under a different tab name for the same missing file; that nested setup registers its placeholder after this owned snapshot was taken. The outer pass then sees the nested diff’s <file> (NEW FILE) buffer as unowned, renames it as an orphan, and creates a second placeholder, leaving two live diffs targeting the same new file. Recheck active_diffs after each autocmd-running rename, or immediately before mutating each candidate.

AGENTS.md reference: AGENTS.md:L35-L37

Useful? React with 👍 / 👎.

hareki pushed a commit to hareki/claudecode.nvim that referenced this pull request Nov 14, 2026
…iff is replaced (coder#536)

## Why

Claude waits for an answer to every `openDiff`. When the plugin never
answers a call, Claude waits forever. One order on `main` leaves a call
without an answer. A handler of the user runs the event loop
(`vim.wait`), so that Claude's queued second call for the same tab name
runs inside it. The verifiers of coder#512 found this order while they
checked a candidate. That candidate newly reaches it, but `main` has it
without any of its code, so I fix it here first.

**A close handler.** An `openDiff` A for a tab name replaces the open
diff X. A `ClaudeCodeDiffClosed` handler of the user runs the event loop
while X closes, so a queued call B registers a diff for the tab name.
When X is a new file, A then cleans up the diff that is registered for
the tab name, and that is B's. Nothing answers B, and its tab or windows
can stay. When X is an existing file, B can instead stay open with no
answer, or A fails with E95. When X is an existing file and B shares A's
proposal name, A fails with E95 as well, and A's failure cleanup removes
B the same way.

## What changed

1. The replace step of `open_diff_blocking` is a loop. It rejects the
diff that is registered for the tab name (if it is pending) and cleans
it up, as before, and repeats while another diff is registered. It
cleans up only the diff that it handled, so a diff that registered
meanwhile is never removed without an answer. Each pending call that A
replaces gets one `DIFF_REJECTED` answer, B included, and A sets up on a
free tab name. The change is 11 added and 5 removed lines in
`lua/claudecode/diff.lua`.
2. `tests/integration/diff_answer_every_call_spec.lua` has 9 cases. Four
of them run the close handler order in both layouts, with X a new file
and with X an existing file. Five pin what `main` does today after a
plugin reload inside a setup (see "A reload" below).
3. `lua/claudecode/README.md` and the `verify-diff-lifecycle` skill
describe the loop. The skill lists the order as not modeled, its
`ReplaceRejects` and `ReplaceKeepsOthersSaved` facts read `while
existing_diff do`, and `covered.lock` has the new hashes.

**A reload.** An earlier version of this PR also registered a diff in
the new module after a plugin reload inside a setup. I removed that
step. A reload inside a setup behaves as on `main`, with and without the
event loop in the reload: the diff finishes its setup in the old module,
so `:ClaudeCodeDiffAccept`, `:ClaudeCodeDiffDeny` and
`:ClaudeCodeCloseAllDiffs` of the new module do not reach it. `:w`, `:q`
and `:bwipeout!` in its window still answer it, and the user can end it
from there. This PR does not promise one `ClaudeCodeDiffClosed` per diff
after a reload, because `main` does not either.

Every order that the scenarios below cover, except the close handler
order, ends as it does on `main`.

## Evidence

Real headless Neovim, one instance per scenario, `tools.handle_invoke`
and the real commands, a stub in place of the `claude` CLI. The rows
below come from the head before the last main merge (`c7d4f61b`) and
`origin/main` at `48ca93be`. On the final head `b001b741`, which merges
`origin/main` at `f28eba7d`, an independent verifier reran the close
handler probe (0 of 224 runs with a problem, 220 of 224 on `main`) and a
probe of the newly merged split and CRLF paths. No row is worse than
`main`. Neovim 0.12.3 and v0.8.0 give the same rows.

| Order | `main` | this head |
| --- | --- | --- |
| Close handler, split layout, X a new file | X rejected, B never
answered, A opens | X and B rejected, A opens and is answered at accept
|
| Close handler, split layout, X an existing file | A fails with E95, B
never answered, 2 tabs | X and B rejected, A opens, 1 tab |
| Close handler, unified layout, X new or existing, with or without a
leftover buffer | B never answered | B rejected, A opens |
| Close handler, split layout, with a leftover buffer, B an existing
file that shares A's proposal name | A and B fail with E95 | same as
`main` |
| Close handler, split layout, with a leftover buffer, B a new file | A
fails with E95, B never answered, 2 tabs | A fails with E95, B rejected,
1 tab |
| Reload inside A's setup (every reload scenario) | unchanged | same as
`main` |

**Spec.** 9 cases. On `main`, 4 fail and 5 pass, on both Neovim
versions. The 4 that fail are the close handler order: `answers a call
that a ClaudeCodeDiffClosed handler lets register while the old new-file
diff is replaced` and the same title with `existing-file`, each in the
vertical and the unified layout. The 5 that pass are the controls: `:w`
in A's window after a reload answers A once, and `:q` or `:bwipeout!` on
a new-file proposal after a reload, with and without the event loop in
the reload, fires one open and one close event, also after `close_tab`
and close-all. This head passes all 9.

**Mutation.** 6 mutants of the loop, all red on both Neovim versions:
the replace step runs once, it cleans up what it did not handle, it does
not read the registry again, it does not reject a pending diff, it does
not clean up, and it rejects only on the first pass. Removing the
`assert` after the cleanup changes no result, because it only states
what the cleanup already did.

**Reload equals `main`.** The verifier's N1 spec (a reload inside A's
setup, a queued call B in the new module, and a close handler that
closes A's window) gives the same rows on `main` and on this head, on
both versions (2 cases, both pass). The verifier's D1 probe (a reload
inside A's setup, then `:q`, `:bwipeout!`, deny or write in A's window,
then `close_tab`; 16 runs per tree) gives the same 16 rows on both
trees.

**Other orders.** I ran 776 scenarios on `main` and on this head, on
both versions (1552 runs per tree): the 657 scenarios of the first
verifier of coder#512 (the core set, the event matrices with and without
notifications, three-deep, three queued calls, the window orders, the V3
orders and the reload-after-action orders), the reload, I1, V2, W1, N1,
three-deep and three-queued scripts of the second verifier and its
90-scenario harness, and my own close handler orders. 1538 runs give the
same output on both trees. 14 differ, which are 7 scenarios on each
version, and all 7 are the close handler order: the four split and
unified orders without a leftover buffer, the two unified orders with
one, and `f3_diffclosed_pump`. In each, `main` leaves a call unanswered
and this head answers every call once with its own text and leaves no
extra window or tab.

**Gate** on the final head (`b001b741`, which contains `origin/main` at
`f28eba7d`, coder#558 and coder#573 included): `mise run clean && mise run all`
exit 0 (1904 successes, 0 failures, 37 pending, luacheck 0 warnings in
201 files). The spec runs 9 of 9 on Neovim 0.12.3 and on v0.8.0. The
coder#558 specs pass too: `tests/unit/diff_exit_reject_spec.lua` 6 of 6, and
`tests/integration/diff_exit_reject_spec.lua` 34 of 34 on both versions.
The coder#573 specs `diff_close_save_spec` (77 of 77) and
`diff_left_window_spec` (40 of 40) pass on both versions. The
integration runner passes with 865 successes and 0 failures on Neovim
0.12.3 and 861 and 0 on v0.8.0, each also under a `TMPDIR` with a space
and a `%`. `check.sh` PASS (93477 and 5557 states) and `--hash-only` is
fresh for all five verify skills. The 6 mutants of the loop are red on
both versions. On `main` the spec fails 4 of 9 cases and passes 5.

## Not in this PR

- A reload inside a setup, and a reload after a setup has finished: the
old module keeps the diff, as the module README says. A fix needs a
design for what the new module may take over. The leftover-buffer case
is coder#512's.
- A `WinClosed`, `BufWipeout` or `BufHidden` handler that runs the event
loop while the diff X is cleaned up (not in the `ClaudeCodeDiffClosed`
event) also lets B register. The last step of `_cleanup_diff_state` then
removes B without an answer in some variants (a `BufWipeout` or
`BufHidden` handler on a new file still answers B), and X's
`ClaudeCodeDiffClosed` fires twice, on `main` and on this head. A guard
on that step reaches every cleanup, not only a replace, so it changes
more orders than the one here.
- A second `openDiff` that registers while the first one is still inside
its setup in the same module, without a reload: B is never answered and
an extra tab can stay, on `main` and on this head. coder#512 handles it with
a refusal.
- The four leftovers that the last verifier found (the `accept_other`
order, one SKILL.md line, a `FileType` handler that closes the proposal
window during setup, and a call that registers during exit) are tracked
in coder#603. The last two are on main too.

---
_Generated with [`xum`](https://github.com/coder/xum) • Model:
`anthropic:claude-sonnet-5-5` • Thinking: `xhigh`_
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.

1 participant