Repository navigation
fix(diff): accept a proposal without a final line break as it is in the split layouts - #542
Conversation
…he split layouts The vertical and horizontal layouts save the lines of the proposal buffer, and add a final line break when the buffer's 'eol' option is on. A new buffer has 'eol' on, so a proposal without a final line break came back with one: "a" was accepted as "a\n", "a\nb" as "a\nb\n", and an empty proposal as "\n". Set the proposal buffer's 'eol' from the proposal text when the buffer is created. The saved text then ends with a line break exactly when the proposal does, and the user's edits in the buffer keep that ending. The unified layout is unchanged.
…the interior blank line - The comment on `blank_end` named a spec file that this branch does not have. It now says that main drops one of those blank lines in the unified layout and that F11 fixes it. - The case for "a\n\nb" had a blank line inside the text, not at its end. Its name now says so.
|
@codex review |
|
@codex security review |
Codex Review Summary: ✅ CompletedThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
Codex Review: Didn't find any major issues. Keep it up! 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". |
🛡️ Codex Security ReviewSecurity review completed. No security issues were found in this pull request. Reviewed commit: Only the user who started this review can view the report in Codex. ℹ️ About Codex security reviews in GitHubThis is an experimental Codex feature. Security reviews are triggered when:
Once complete, Codex will leave suggestions, or a comment if no findings are found. |
main now has the split-layout fix (#542), so the two specs that waited for each other can run in full: - diff_accept_trailing_newlines_spec.lua: drop the pending clause, so the 2 vertical cases without a final line break run (28 cases). - diff_accept_missing_final_newline_spec.lua: drop the `blank_end` skip, so the 9 unified cases that end in blank lines run (94 cases).
…atch the code (coder#543) ## Why Two sections of `ARCHITECTURE.md` show code sketches that do not match the code. Both read as how the plugin works today. They sit under headings that name the module, and nothing calls them examples, so a contributor reads them as fact. - The Lock File System sketch names a `write_file` helper and a `vim.loop.fs_rename` call that `lockfile.lua` does not have. It puts a `VimLeavePre` autocmd and an unconditional `fs_unlink` in `lockfile.lua`, which has no autocmd. coder#537 added one sentence after it and left the rewrite for this PR. - The Diff System sketch writes a temp file with `write_file` and sets the keymaps `<leader>da` and `<leader>dq`. The `openDiff` tool writes no temp file, and the plugin sets no diff keymaps. ## What changed Two sections of `ARCHITECTURE.md`. Each is now short prose with citations, plus one shortened excerpt whose code lines are verbatim from the source file (a script checked each line). **Lock File System** (`lockfile.lua`), six bullets: - Where the file lives: `<port>.lock` in `$CLAUDE_CONFIG_DIR/ide`, or in `~/.claude/ide` when that variable is unset or empty (`lockfile.lua:11-18`). - What it holds: `pid`, `workspaceFolders`, `ideName`, `transport` and `authToken` (`lockfile.lua:129-135`). - Directory mode: a new lock directory gets `0700`, and `create` tries to tighten an existing one (`lockfile.lua:94`, `:104`). - The atomic write: a temp file in the same directory, opened with `wx` and mode `0600`, renamed with `os.rename`. A failed write or rename makes `create` try to remove the temp file, and it ignores a failed removal (`lockfile.lua:154-199`). - Removal: `remove` uses `os.remove` and returns an error when no readable lock file exists and when `os.remove` fails (`lockfile.lua:208-232`). - The lifecycle, which `init.lua` owns: `start` calls `create` after the server binds, `stop` calls `remove` and only logs a warning when it fails, and the `VimLeavePre` autocmd that `setup` creates calls `stop` when the server runs (`init.lua:514-522`, `:586`, `:635-648`). **Diff System** (`diff.lua`, `diff_inline.lua`), a paragraph and six bullets: - `openDiff` calls `open_diff_blocking`, which runs in the tool's coroutine and yields until the diff is resolved. A `ClaudeCodeDiffOpened` handler can resolve the diff during setup, and `open_diff_blocking` then returns the result at once without yielding. When the setup fails (for example, because the file has unsaved changes), it raises an error and does not yield (`tools/open_diff.lua:67`, `diff.lua:1893-1987`). - The layout: `vertical` (the default) and `horizontal` use Neovim's diff mode, with the proposal in a split to the right or below. For a new file, the old side is an empty buffer. `unified` is drawn inline by `diff_inline.lua` and is not a Neovim diff (`diff.lua:279-288`, `:1595`). - The proposal is an unlisted `acwrite` scratch buffer and not a temp file. Its name is `<tab_name> (proposed)` for a native diff of an existing file, `<tab_name> (NEW FILE - proposed)` for a new file, and `<tab_name> (unified diff)` for the unified layout (`diff.lua:1730-1748`, `diff_inline.lua:292-299`). - Tabs: a native diff prefers the current tab and goes to an editor window in another tab only when the current tab has none. It opens in a new tab when `diff_opts.open_in_new_tab` is set or the tab it picks, or the current tab, already has a diff window (`diff.lua:125-136`, `:1625`, `:1700`). - Accept and reject: in the proposal buffer, `:w` (also `:wq`, `:x` and `ZZ`) and `:ClaudeCodeDiffAccept` accept. `:ClaudeCodeDiffDeny`, closing the last window of the proposal, and deleting its buffer reject. The two commands read the diff from the current buffer and only warn in another buffer. The plugin sets no keymap. The README's example config maps `<leader>aa` and `<leader>ad` to the two commands (`README.md:66-67`). - A diff resolves once. Accept replies `FILE_SAVED` with the content, reject replies `DIFF_REJECTED`, and the plugin does not write the target file. Normally Claude writes it after `FILE_SAVED`. When that Claude session ends or drops the answer first, `:ClaudeCodeCloseAllDiffs` puts the accepted text into the file's buffer as unsaved changes. A diff whose buffer cannot take the text (for example, because it has unsaved changes) stays open (`diff.lua:1071`, `:1154`, `:2209-2271`, `:2309`). - Only the non-blocking `open_diff` writes a temp file, and the plugin does not call it. It goes under `stdpath("cache")/claudecode_diffs/<session folder>`, and `vim.fn.tempname()` supplies only a part of the folder name (`diff.lua:522-561`, `:791-860`). ### Statements of the old sections that the code contradicts | Old statement | What the code does | Evidence | |---|---|---| | Lock: `local temp_path = lock_path .. ".tmp"` | The temp path is `<port>.lock.tmp.<pid>.<hrtime>`. | `lockfile.lua:154` | | Lock: `write_file(temp_path, json_data)` | No `write_file` exists. The code opens the temp file with `fs_open "wx"` and mode 0600, loops `fs_write` until every byte is written, then closes it. | `grep -c write_file lockfile.lua` is 0. `lockfile.lua:156-188` | | Lock: `vim.loop.fs_rename(temp_path, lock_path)` | The code calls `os.rename`. | `grep -c fs_rename lockfile.lua` is 0. `lockfile.lua:195` | | Lock: a `VimLeavePre` autocmd in `lockfile.lua` calls `fs_unlink(lock_path)` | `lockfile.lua` has no autocmd. Its `fs_unlink` calls remove only the temp file. The lock file goes through `remove` with `os.remove`, which `stop` calls. The exit autocmd is in `init.lua`. | `grep -c VimLeavePre lockfile.lua` is 0. `lockfile.lua:191`, `:197`, `:221`. `init.lua:514-522`, `:642` | | Diff: "Create temp file with proposed changes" with `vim.fn.tempname()` and `write_file(temp_file, new_content)` | `openDiff` writes no temp file. The proposal is a scratch buffer. `write_file` appears nowhere under `lua/`. The only file write in `diff.lua` is `io.open(tmp_file, "w")` in `create_temp_file`, which the legacy `open_diff` calls. `tempname()` seeds part of the session folder name. | Probe: no `claudecode_diffs*` folder after an `openDiff` diff. `diff.lua:523-524`, `:553`, `:1730-1748` | | Diff: `edit original_file`, `diffthis`, `vsplit temp_file`, `diffthis` | The old file goes in a window with `diffthis`. `create_split` makes a split to the right (`vertical`) or below (`horizontal`). The proposal buffer goes there with `diffthis`. | Probe: right for `vertical`, below for `horizontal`. `diff.lua:279-288`, `:715-730` | | Diff: "Open diff in current tab to reduce clutter" | The current tab is preferred and not guaranteed. A new tab opens when `open_in_new_tab` is set or the picked tab or the current tab already has a diff window. With no usable editor window in the current tab, the diff uses one in another tab. | Probe: 1 tab by default, 2 tabs for each of the two cases. `diff.lua:125-136`, `:1625`, `:1700`. `tests/integration/diff_current_tab_spec.lua:136` | | Diff: `vim.keymap.set("n", "<leader>da", accept_all_changes)` and `<leader>dq` with `exit_diff_mode` | The plugin sets no diff keymap. `accept_all_changes` and `exit_diff_mode` do not exist. Accept and reject go through `:w`, the two commands, and closing the proposal. | `grep keymap diff.lua diff_inline.lua` finds nothing. `init.lua:1209-1219`. Probe of all five accept paths and three reject paths | | Diff: "Native Neovim diff implementation" | True for `vertical` and `horizontal`. It is incomplete: `unified` is an inline renderer and not a Neovim diff. | Probe: 0 windows with `'diff'` set for `unified`. `diff.lua:1595` | ## Scope Only these two sections of `ARCHITECTURE.md`. I did not audit the rest of the file. The line numbers in this description refer to `main` at `5acc1002`. `ARCHITECTURE.md` cites symbols and no line numbers, so later commits to `diff.lua` do not make it stale. coder#542 added three lines to `diff.lua` after this branch left `main`. - **coder#537** landed as `5acc1002` and conflicted with the lock section in one hunk. This branch keeps its own lifecycle bullet, which says what the sentence of coder#537 said, with citations. The comment fix of coder#537 in the Resource Cleanup snippet is kept. - **Resource Cleanup snippet** (not changed). It is a simplified form of the real autocmd. The real one has the `ClaudeCodeShutdown` group and calls `stop` only when a server runs (`init.lua:514-522`). It is not false. - **`README.md:251-252`** (not changed). It lists "Accept: `:w` or `<leader>aa`" and "Reject: `:q` or `<leader>ad`" without saying that the two keys come from the example config at `README.md:66-67`. Nothing under `lua/` or `plugin/` sets them. That is a possible follow-up for the README. ## Blast Radius One documentation file. Nothing loads it. A wrong sentence here misleads a contributor and breaks no behavior. ## Verification I ran the real modules in headless Neovim 0.12.3 with an isolated home (`env -i`, private `HOME`, `XDG_*` and `CLAUDE_CONFIG_DIR`, the stub `claude` first on `PATH`). Lock file section: - A new lock directory has mode 0700 and the lock file has mode 0600. - The file holds the keys `authToken`, `ideName`, `pid`, `transport` and `workspaceFolders`, with `ideName` `Neovim` and `transport` `ws`. - No temp file stays after `create`. - `fs_open "wx"` fails with `EEXIST` for an existing file and for a dangling symlink. - `remove` returns `Lock file does not exist` for a missing file and `Invalid port number` for a bad port. - With `CLAUDE_CONFIG_DIR` empty or unset, the lock directory is `~/.claude/ide`. Diff System section: - `vertical`: the proposal is right of the old file. `horizontal`: below it. Both windows have `'diff'` set, there is 1 tab, and the proposal buffer is `acwrite` and unlisted. No `claudecode_diffs` folder exists. - `:w`, `:wq`, `:x`, `ZZ` and `:ClaudeCodeDiffAccept` each reply `FILE_SAVED` with the new text, and the target file is unchanged. - `:ClaudeCodeDiffDeny`, closing the last window of the proposal, and `bwipeout` of its buffer each reply `DIFF_REJECTED`. - A `ClaudeCodeDiffOpened` handler that rejects the diff makes `open_diff_blocking` return `DIFF_REJECTED` at once: the coroutine is dead after its first resume and never yielded. - `:ClaudeCodeDiffDeny` and `:ClaudeCodeDiffAccept` in the old-file buffer only warn `No active diff found in current buffer`, and the diff stays pending. - The proposal buffers are named `n-exist (proposed)`, `n-new (NEW FILE - proposed)` and `u1 (unified diff)` for an existing file, a new file and the unified layout. - After `:w` and `close_saved_diffs`, the target file is unchanged and its buffer holds the accepted text as unsaved changes. When the buffer already has unsaved changes, the diff stays open (`kept` says `its buffer has unsaved changes`) and the plugin does not touch the buffer. - A dirty target buffer makes `open_diff_blocking` raise `Diff setup failed`. The coroutine is dead after its first resume. - A new-file diff started from an empty buffer reuses that buffer as its old side. Started from a file buffer, it shows an empty `nofile` placeholder named `<path> (NEW FILE)`. - `open_in_new_tab = true` gives 2 tabs. A diff already in the current tab gives 2 tabs. `unified` gives 1 tab, 0 windows with `'diff'` set, and an inline diff buffer. - The legacy `open_diff` writes `target.txt.new` under `stdpath("cache")/claudecode_diffs/<session folder>`. Checks: - Citation check over the seven READMEs and `ARCHITECTURE.md` (I added the file to the validator's list): 787 citations, 1 flagged. The flagged one is `[plugin/claudecode.lua:pcall(require, "claudecode")]`, an expression that `main` already cites. A negative control with two wrong symbols flagged both. - `mise run format-check`: exit 0 (355 files, 0 changed). - I did not run `mise run all`. The change is docs only, and no test, script or CI file reads `ARCHITECTURE.md` (I checked `tests`, `scripts`, `.github` and `mise.toml`). CI runs the full suite on the pushed head. - `git merge-tree --write-tree` of the head and `origin/main` is clean. --- _Generated with [`xum`](https://github.com/coder/xum) • Model: `anthropic:claude-sonnet-5-5` • Thinking: `xhigh`_
…split layouts (coder#551) ## Why When a user empties the proposal buffer (`ggdG` or `:%delete`) and saves it with `:w`, Claude must get what Neovim's own `:w` writes. Neovim writes no bytes for an emptied buffer. With `'eol'` on, the `vertical` and `horizontal` layouts answered `" "`, so Claude wrote a one-byte file where the user had asked for an empty one. The F14 verifier found this. PR coder#542 listed it as a known, pre-existing defect that this PR fixes. ## What changed `_resolve_diff_as_saved` in `lua/claudecode/diff.lua` saves the lines of the proposal buffer, joined with line breaks. It adds a final line break when the buffer's `'eol'` is on. A buffer with no lines at all (Neovim shows "--No lines in buffer--") still gives one empty string through `nvim_buf_get_lines`. So the save could not tell it from a buffer with one empty line, and it added the line break. The save now counts the bytes of the buffer with `vim.fn.wordcount().bytes`, run inside `nvim_buf_call`. The count is 0 for an empty buffer and at least 1 for a buffer with one empty line when `'eol'` is on. The line break is added only when the count is not 0 and `'eol'` is on. That replaces the check `#content_lines > 0`, which was always true. I tried `nvim_buf_get_offset` and `line2byte` first. Both gave wrong sizes for a buffer that `ggdG` had emptied (a stale line cache), so I did not use them. Other changes: - `tests/mocks/vim.lua` gets `wordcount()` and an `empty = true` flag for a mock buffer. `nvim_buf_call` in the mock now names the buffer that the callback acts on. - `tests/unit/diff_ui_cleanup_spec.lua` gets one fixture line. The mock's `nvim_buf_call` refuses a buffer that does not exist, and that spec saved a buffer that the mock never had. - `lua/claudecode/README.md`: one bullet replaces coder#542's bullet about the saved content of the split layouts. It states both rules, `'eol'` from the proposal and the empty buffer. It also says that a buffer with one empty line is saved as `"\n"` when `'eol'` is on and as `""` when it is off. - `.agents/skills/verify-diff-lifecycle/covered.lock` is re-recorded (see Blast Radius). ## Scope - The `vertical` and `horizontal` layouts share one code path, so one change fixes both. - The `unified` layout is unchanged. It answers with the proposal, not with the buffer, so what the user does to its buffer never reaches Claude. The new spec runs it as a control. - With `'eol'` on, a buffer with one empty line is still saved as `"\n"`. With `'eol'` off, it is saved as `""`, as before. A buffer with any line is saved as before. - On `main` (with coder#542), this PR changes the answer only for a buffer that the user emptied. It changes no answer for an unedited proposal: coder#542 already makes an empty proposal give `""`, and the new spec has that case as a control. ### Minor findings that I declined Two verifiers (Opus and GPT) found these. None of them changes an answer that a user reaches with the default settings. - **A buffer set to `bomb` (Opus F3).** With `ggdG`, then `:setlocal bomb`, then `:w`, Neovim writes the 3-byte BOM. Both `main` and this PR answer without the BOM (`main` answers `"\n"` when `'eol'` is on and `""` otherwise, this PR answers `""`). The plugin never sets `'bomb'` on its scratch proposal buffer, and no answer of the plugin ever holds a BOM, so a user has to set a non-default option on a scratch buffer first. I left it. - **The `nvim_buf_call` wrapper is covered by the mock spec only (Opus F4).** If I delete the wrapper, only the mock unit spec fails. In the paths that save (`BufWriteCmd` and `accept_current_diff`) the current buffer is always the proposal buffer, so the wrapper is defensive and no live answer changes. I kept the wrapper, because `wordcount()` reads the current buffer, and I added no live test for a case that no path reaches. - **Cost of `wordcount()` (Opus F5).** It adds a second pass over the buffer. For a 33 MB proposal that is about 0.1 to 0.2 s (0.12.3: +112 ms, 0.8.0: +216 ms), about the cost of `nvim_buf_get_lines` plus the join. I accept that for one save. ## Blast Radius - User-visible: a proposal buffer that the user emptied is accepted as an empty string. Every other buffer gives the same text as before. - `wordcount()` runs in the proposal buffer through `nvim_buf_call`. It does not change the user's current buffer or window. Both functions exist in Neovim 0.8. I ran the specs on Neovim 0.12.3 only. The CI job `integration-tests (v0.8.0)` runs the new spec on 0.8. - `covered.lock`: the change touches `_resolve_diff_as_saved`, a covered block. `check.sh` printed STALE for the `diff.lua` file hash and the `resolve_saved` block hash only. It printed no changed call site, and all 21 code facts stay TRUE. The guard that lets only a `pending` diff resolve is unchanged, and so are the status change to `saved` and the one call of the resolution callback. The change only decides whether the `text` string of the `FILE_SAVED` answer ends in a line break. The model stores which kind of answer a call got, not its text, so no modelled transition changes. The model passed (`main`: 89205 distinct states, `live-DisconnectedCallsAnswered`: 5269). `check.sh --record` wrote the new lock and `check.sh --hash-only` reports that it matches. ### Merge with coder#542, coder#539 and coder#535 coder#542 (F14) landed first, on 2026-11-12 at 04:36Z, and coder#539 (F12) at 04:46Z. coder#538 and coder#550 later changed `diff.lua` too, and coder#549 renamed "proposed buffer" to "proposal buffer" in the README. This PR merged `origin/main` (9bfc39c) with merge commits. It ran `check.sh --record` after each change to `diff.lua` and kept main's README wording in the last one. Only the `diff.lua` file hash and the `resolve_saved` block hash differ from the lock on `main`. The full `check.sh` passes and `check.sh --hash-only` matches. - One README bullet replaces this branch's bullet and coder#542's bullet, because the two said almost the same sentence and this branch's sentence about one empty line was false once `'eol'` can be off. - coder#535 (F11) is still open. It touches neither `diff.lua` nor this README bullet. Its current head (`a5dfc81b`, with `main` merged) merges with this PR without a conflict. ## Verification Red before the fix, green after (same specs, only `lua/claudecode/diff.lua` swapped, on the merged head, so the red column is `main` at 9bfc39c with coder#542): | Spec | `main` (with coder#542) | with the fix | | --- | --- | --- | | `tests/unit/diff_saved_text_spec.lua` (4 cases, mock) | 3 successes, 1 failure | 4 successes, 0 failures | | `tests/integration/diff_accept_emptied_buffer_spec.lua` (40 cases, real Neovim, `:write`) | 28 successes, 12 failures | 40 successes, 0 failures | Against the older merge base (5acc100, before coder#542), the 38-case spec of round a had 24 red cases. Today's 40-case spec has 26 there: the 24 emptied cases plus the new control for an unedited empty proposal in both layouts. coder#542 fixes the 12 cases from proposals without a final line break (it sets `'eol'` off), so 12 stay red on `main`. The integration spec opens a real `openDiff` and edits the proposal window. For the `vertical` and `horizontal` layouts it runs 6 proposals (two lines and a final line break, one line and a final line break, one blank line, one line with no final line break, two lines with no final line break, and no text) with 2 ways to empty the buffer (`ggdG` and `:%delete`). Each case compares the answer with what Neovim's own `:w` writes: the spec edits a real file buffer with the same text and commands, runs `:write`, and reads the file bytes, which are empty. Five controls per layout give the right text before and after (a blank line left in the buffer, no edit, an empty proposal without an edit, one line deleted, and text typed into the emptied buffer). In the `unified` layout, 6 controls check that the answer is the proposal. Gates on the final commit, which merges `origin/main` (9bfc39c) into the branch (private XDG folders): - `mise run all`: native exit 0. luacheck: 0 warnings / 0 errors in 183 files. busted: 1710 successes / 0 failures / 0 errors / 26 pending. treefmt: 0 files changed. - `mise run format-check`: native exit 0 (360 files, 0 changed). - Diff specs (`tests/unit/diff*_spec.lua`, `opendiff_blocking_spec`, `new_file_reject_then_reopen_spec`, and the `open_diff_mcp`, `close_tab` and `close_all_diff_tabs` specs): 226 successes, 0 failures. - `scripts/run_integration_tests_individually.sh`: 457 successes, 0 failed, 0 errors. The new spec gives 40 successes. - `.agents/skills/verify-diff-lifecycle/check.sh --hash-only`: covered.lock matches the code. Two independent verifiers (Opus and GPT) checked the code at `ef8be297`. Both found the code correct. Their blockers were the README sentence and the stale base, and this head fixes both. --- _Generated with [`xum`](https://github.com/coder/xum) • Model: `anthropic:claude-sonnet-5-5` • Thinking: `xhigh`_
…coder#535) ## Why A unified diff must hand Claude back the exact text it proposed when the user accepts it. A proposal that ends in a blank line lost one line break. A proposed file ending `"a\n\n"` was accepted as `"a\n"`, so Claude wrote a file one blank line shorter than the one the user approved. ## What changed `resolve_inline_as_saved` in `lua/claudecode/diff_inline.lua` joined the accepted lines with `"\n"`. It added the final `"\n"` only when the join did not already end in one. A proposal that ends in blank lines has a blank last line, so the join already ended in `"\n"` and the final line break was never added back. The function now adds the final `"\n"` whenever the proposal ends with one. The lines never carry a line break after the last one, so this is always one line break short. Other changes: - `lua/claudecode/README.md` states the `unified` contract on the line that describes the `FILE_SAVED` content. - `.agents/skills/verify-diff-lifecycle/covered.lock` is re-recorded (see Blast Radius). ## Scope Only the `unified` layout changes. The `vertical` and `horizontal` layouts keep their code and their results. A separate defect used to stay out of this PR. In both the `vertical` and the `horizontal` layout, a proposal without a final line break was accepted with one. F14 (coder#542) fixed it and is now in `main`. The new integration spec runs the `vertical` layout, and its 2 cases without a final line break run as active tests (they were pending before F14 landed). ## Blast Radius - User-visible: an accepted unified diff keeps every blank line at the end of the proposal. Proposals that ended in a single line break, in no line break, or in blank lines inside the file give the same text as before. - `covered.lock`: the change touches `resolve_inline_as_saved`, a covered block. It changes only the `text` string in the `FILE_SAVED` answer. It does not change the saved status, the answer count, the resolution callback order or disconnect handling. Diff commands make the same decisions. `:ClaudeCodeCloseAllDiffs` puts the answer text into the file's buffer, so for a saved unified diff it now keeps the trailing blank lines too. The model does not represent answer text: it stores which kind of answer a call got, so no modelled transition changes. `check.sh` printed STALE for exactly two entries (the file hash and the `inline_saved` block hash of `diff_inline.lua`) and no changed call site. The model passed (`main`: 89205 distinct states, `live-DisconnectedCallsAnswered`: 5269). `check.sh --record` wrote the new lock and `check.sh --hash-only` then reports that it matches. ### Merge with F14 (coder#542) F14 (coder#542) landed first, as 49df75a. The two PRs left a test that each other made stale, so this PR owed the merge rule, and its merge commit with `origin/main` (0b0de5b) applies it: 1. F11's `tests/integration/diff_accept_trailing_newlines_spec.lua`: the 7-line `local case = it` pending clause is replaced with one `it(` line, so the 2 vertical no-final-line-break cases run. 2. F14's `tests/integration/diff_accept_missing_final_newline_spec.lua`: the `blank_end` skip is removed (the `if not (layout == "unified" and proposal.blank_end) then` line and its `end`, the 3 `blank_end = true` fields and their comment), so the 9 unified blank-line cases run. On the merged head, F11's spec gives 28 successes, 0 failures and 0 pending. F14's spec gives 94 successes, 0 failures and 0 pending. Without this PR's fix (`diff_inline.lua` from `main`), F11's spec gives 20 successes and 8 failures, and F14's spec gives 85 successes and 9 failures. All of those failures are unified blank-line cases. ## Verification Red before the fix, green after (same specs, only `diff_inline.lua` swapped, on the merged head, so `main` already has F14): | Spec | origin/main | with the fix | | --- | --- | --- | | `tests/unit/diff_inline_spec.lua` (24 new cases) | 38 successes, 12 failures | 50 successes, 0 failures | | `tests/integration/diff_accept_trailing_newlines_spec.lua` (28 cases, real Neovim) | 20 successes, 8 failures (all `unified`) | 28 successes, 0 failures, 0 pending | | `tests/integration/diff_accept_missing_final_newline_spec.lua` (F14's spec, 94 cases, real Neovim) | 85 successes, 9 failures (all `unified`) | 94 successes, 0 failures, 0 pending | The unit cases run `compute_inline_diff` and `resolve_inline_as_saved` for each proposal and compare the accepted text byte for byte. The integration spec opens a real `openDiff` in both layouts, runs `:write` in the proposal window and compares the answer byte for byte. It uses proposals ending in `"\n\n"`, `"\n\n\n"`, no line break, a single `"\n"`, `"\n"` only, `"\n\n"` only and a blank line inside plus one at the end, each against a file with other text and against a file one line shorter. The audit's own probe in a real Neovim shows the same change: proposal `"a\n\n"` was accepted as `"a\n"` and is now accepted as `"a\n\n"`. Gates on the final commit, which merges `origin/main` (9bfc39c) into the branch (the merge rule was applied in the earlier merge commit 0b0de5b) (private XDG folders): - `mise run all`: native exit 0. luacheck: 0 warnings / 0 errors in 182 files. busted: 1730 successes / 0 failures / 0 errors / 26 pending. treefmt: 0 files changed. - `mise run format-check`: native exit 0 (359 files, 0 changed). - Sibling specs (`diff_inline_spec`, `diff_inline_new_tab_spec`, `diff_inline_quit_spec`, `diff_spec`, `opendiff_blocking_spec`): 83 successes, 0 failures. - `scripts/run_integration_tests_individually.sh`: 454 successes, 0 failed, 0 errors. - `.agents/skills/verify-diff-lifecycle/check.sh --hash-only`: covered.lock matches the code. Rounds 1 and 2 (heads 7e6f819 and 0b0de5b): CI green and both Codex lanes clean. Round 3 is this head, after a merge with `main` for a README conflict (coder#549). Two independent verifiers (Opus and GPT) checked the code commit before the first merge with `origin/main`. Both found no introduced defect. They reran the red and green specs, reverted the code change alone to see the new specs fail, and compared the accepted bytes through a real WebSocket call. --- _Generated with [`xum`](https://github.com/coder/xum) • Model: `anthropic:claude-sonnet-5-5` • Thinking: `xhigh`_
…diff closes (coder#494) ## Why After a split diff closes, a window that still shows the file can stay in diff mode with no partner window. The next `openDiff` then sees a window in diff mode, opens in a new tab, and closes the user's window together with that tab. coder#491 ended diff mode in the windows that the cleanup knew about. This closes the windows that it did not. Refs coder#491, coder#483. ## Cause Neovim saves a window's options for a buffer when the buffer leaves the window ('diff', 'scrollbind', 'foldmethod' and the rest included), and sets them again when that buffer enters the window. A window that never showed the buffer can also take diff mode on, by copying the options of a window that shows the buffer (`:split <file>` copies the window that it splits). While a split diff is open, the file's buffer is in diff mode in the original-side window. So when the cleanup closes the proposal window and deletes the proposal buffer, a window can end up showing the file in diff mode with no partner. Two ways, both reproduced on current main with real Neovim 0.12.3 and v0.8.0: 1. `:b <proposal>` in the original window, then `close_tab`. The cleanup deletes the proposal buffer, the window falls back to the file, and the saved options turn diff mode on. One window, `diff=true`. 2. `:split <file>` from the original window during the diff, then `close_tab`. The copy took diff mode from its source, the cleanup closes the proposal window, and the copy keeps diff mode alone. It stays on when `close_tab` runs from a plain window of the same tab, or from another tab. ## What changed The rule: when the cleanup is done, a window of the original side's tab page that shows the file leaves diff mode only if it is the only window with diff mode on in that tab page. A diff needs a partner window, so a lone diff window holds the leftover of the diff that ended, and a diff of the user's own over two or more windows stays. - `lua/claudecode/diff.lua`: two local functions and one call. `end_diff_mode_if_alone(win)` runs `diffoff` in a window while `'diff'` is on there and no other window of its tab page has `'diff'` on. It catches the refusal that Neovim gives in the command-line window (E11). `end_diff_mode_in_windows_that_show_the_file(diff_data)` runs that check for each window of the original side's tab page that shows the file. The cleanup calls it after the buffer deletes and before `ClaudeCodeDiffClosed` fires, so nothing that a handler of that event starts is looked at. Nothing is recorded and no autocmd is added. - `lua/claudecode/README.md`: states the rule, and which cases stay as on main (below). The sentences on saved options and on the unified layout's `'signcolumn'` are rewritten from plain-Neovim probes. - `tests/integration/diff_left_window_spec.lua` (real Neovim): 33 cases. They cover the two ways above, a window that the diff never left, a copy that has a partner and two copies that the user compares (kept), the options that the user set after ending diff mode, a lone diff window of another tab (not touched), the order of the look and the event (a diff that a `ClaudeCodeDiffClosed` handler starts stays), `close_tab` from the command-line window (it answers TAB_CLOSED and the copy keeps its diff mode, because Neovim refuses `diffoff` there), and the cases below that stay as on main. - Verify skill: `journey-diff-close-save.sh` shows the proposal in the original window with `:b` and closes the diff for an existing file, and `features/open-diff.md` and `SKILL.md` describe it. - `covered.lock` is re-recorded after merging main. Against main's lock only the `diff.lua` file hash and the `cleanup_state` block hash change. `check.sh` passes with TLC (89205 states). ## What stays as on main A file that comes back to a window later gets the options that Neovim saved or copies, diff mode included, as on main: - `:e other.txt` in the proposal window, `close_tab`, then `:e <file>` there, or `:e third.txt` in the original window, `close_tab`, then `:e #`. Diff mode is on again. This is the common case. It gets its own follow-up PR that fixes it at the source: the plugin splits the window before it runs `diffthis`, in both open paths, so the saved set never holds diff mode. - `:split <file>` from a window that left the file after the diff. - A window of another tab page that took diff mode from the diff's windows during the diff (`:tabnew <file>`, `:tab sbuffer`, `:tabnew` then `:buffer <file>`). The look covers only the tab page of the original side. - A `ClaudeCodeDiffClosed` handler that brings the file back into a window. - `close_tab` from the command-line window (`q:`) when the original-side window shows the file fails with -32000 and E11 in main's own `diffoff`, as on main. ## Before and after Real headless Neovim, the plugin's own `openDiff` and `close_tab` tool handlers, a stub `claude` first on PATH: | Scenario | main | this branch | | --- | --- | --- | | 1. original window: `:b <proposal>`, `close_tab` | one window, `diff=true` | one window, `diff=false` | | 2. `:split <file>` from the original window during the diff, `close_tab` from a plain window of the same tab | copy `diff=true`, the plain window's `'foldmethod'`, `'scrollbind'` and `'wrap'` kept | copy `diff=false`, the plain window's options kept | | 3. same copy, `close_tab` from another tab that holds the user's `diffthis` over two windows | copy `diff=true` | copy `diff=false`, the other tab's diff kept | | 4. a window of the first diff that the user switched to another file, then a second `openDiff`, `:edit <file>` in that window, and the second `close_tab` | one window left in diff mode | no window in diff mode | | 5. a copy that has a partner, or two copies that the user compares | kept | kept | | 6. `:e other.txt` in the proposal window, `close_tab`, `:e <file>` | `diff=true` | `diff=true` (follow-up PR) | | a diff with no window switch | `diff=false` | `diff=false` | The same rows give the same result on 0.12.3 and v0.8.0. ## Validation Head 06bde3e (round 9: a merge-only push of main 884f7e2 that re-records `covered.lock`, with no Lua change and no Codex review; the Lua patch-id equals rounds 7 and 8). On it: `check.sh --hash-only` and `check.sh` pass, the coder#494 spec passes 33/33 on Neovim 0.12.3 and v0.8.0, `mise run all` passes (1737 successes), the integration runner passes (527 on 0.12.3 after one rerun of two load timeouts, 390 on v0.8.0), and CI is green. Head e80a5a3, a merge of main 8b77e7e into c219b15 (round 8: a merge-only push, no code change). The PR's Lua diff is the same before and after the merge: `git diff c9e57d1..c219b15 -- '*.lua'` and `git diff origin/main...HEAD -- '*.lua'` give the same patch-id (`76da21f7b53bfdadb02303aa6ef201c9f445de4e`) and the same text without the `index` lines and hunk numbers (main moved `diff.lua`, so those differ). The merge conflicted in `covered.lock` and in `lua/claudecode/README.md`. The README conflict was main's edits around the cleanup bullet (coder#549, coder#542 and others) against the one bullet that this PR rewrites: I kept every line of main and put this PR's bullet in place of the old one, so the README differs from main by that one line. I re-recorded `covered.lock` with `check.sh --record` on the merge result. Against main's lock only the `diff.lua` file hash and the `cleanup_state` block hash differ. Size against main: 7 files, 613 insertions(+), 7 deletions(-). Code and README: 2 files, 42 insertions(+), 3 deletions(-). The spec is most of the rest. - `mise run clean && mise run all` on the merge result: exit 0. luacheck 0 warnings, 0 errors in 182 files. busted 1706 successes, 0 failures, 0 errors, 26 pending. - New spec, 33 cases, on 0.12.3 and on v0.8.0: 0 fail on this branch and 3 fail on main (cases 1 and 2 above, and the one for `close_tab` from another tab). It also passes 33 of 33 on both versions on the merge of this head with the newest main (e4c56fd), which merges without a conflict, and `check.sh --hash-only` passes on that merge. - Integration runner with a stub `claude` first on PATH: 0.12.3 450 successes and v0.8.0 342, 0 failed, 0 errors (28 files). Under `TMPDIR="<dir>/odd tmp 50%"` the same: 450 and 342, 0 failed, and the cache folder stays untouched. - Journey `journey-diff-close-save.sh` in agent-tty with the scripted client (run on c219b15, before the merge): 4 of 4 runs pass (split and unified, existing and new file). Existing file: 24 checks. New file: 23. - Live probes through the plugin's own tool handlers (run on c219b15 against main c9e57d1, before the merge, which changes none of the PR's Lua), on 0.12.3 and v0.8.0, compared by scenario and check name (checks that fail on the head and pass on main: 0 in every set): | Probe set | checks | fail on head | fail on main | fail on head only | fixed on head | | --- | --- | --- | --- | --- | --- | | round-3 verifier | 432 | 97 | 101 | 0 | 4 | | gpt round-3 | 92 | 14 | 14 | 0 | 0 | | round-4 verifier | 106 | 21 | 29 | 0 | 8 | | round-5 verifier | 116 | 56 | 56 | 0 | 0 | | gpt round-5 novel | 46 | 8 | 8 | 0 | 0 | | other-tab | 18 | 12 | 12 | 0 | 0 | | Opus round-6 (copies, handlers, command-line window) | 84 | 20 | 24 | 0 | 2 | | gpt round-6 | 60 | 12 | 12 | 0 | 0 | The 8 fixed checks of the round-4 set are the `:split` copy cases (row 2 and 3), the 4 of the round-3 set are `:b <proposal>` (row 1) and the second diff (row 4), the 2 of the Opus set are the second diff. - Order matrices (handlers that switch focus, raise or open windows, before or after, for `BufEnter`, `BufWinEnter`, `WinNew`, `WinEnter`): gpt's 96 orders per version, Opus's 96 orders (48 per version) and my 40 orders (20 per version) give the same answer, in every field, on this head and on main. A focus probe with a user `BufWinEnter` and `BufEnter` handler that moves the focus to another tab gives the same answer on both. - Probes of the defects that review found in the earlier design (a `:split` copy with a lone user `:diffthis` that gets the file back with `:edit!`, `:e other` and `:e #`, and a window that got the file without a `BufWinEnter`): this head keeps the user's diff in all, as main does. - Mutation: 14 mutants of the new code (no call, no `diffoff`, no `'diff'` check, no partner check, partner in any tab, look in any tab, skip the original-side window, no catch of the refusal, no `nvim_win_call`, the partner must show another buffer, the call after the event, the call before the deletes, no buffer and window guard, `diffoff!`). 13 are caught by the spec on both versions: 12 as failed cases and the one without the guard as an "Invalid window id" error. The survivor is `diffoff!` for `diffoff`. It is equivalent: the check runs first, so no other window in the tab has `'diff'` on, and `diffoff!` acts only on windows with `'diff'` on. - `check.sh --hash-only`: covered.lock matches the code. `check.sh` (TLC, 89205 states, and 5269 for the live model): pass. ## Round limit The maintainer waived the review-round limit for one push (round 7): coder#494 (comment). A second waiver covers one merge-only push (round 8, this head): coder#494 (comment). The push replaces the earlier `BufWinEnter` waiter and its window records with the single look above and merges current main. There is no further push to this PR. A new security flaw, data loss or broken core behavior compared with main ends it as blocked. A third waiver covers one bounded merge-only push (round 9) with no Codex review, and declines the round-8 Codex P1 with a reason: coder#494 (comment). --- _Generated with [`xum`](https://github.com/coder/xum) • Model: `anthropic:claude-sonnet-5-5` • Thinking: `xhigh`_
…ak spec (coder#571) ## Why Two integration specs ran the same test for the accepted text of a diff. `diff_accept_trailing_newlines_spec.lua` (88 lines, from coder#535) opens a diff, accepts it unedited, and compares the answer with the proposal. `diff_accept_missing_final_newline_spec.lua` (from coder#542) does the same over a larger table of layouts, old files and proposals. Both had the same setup and the same four assertions. Twelve of the 28 cases of the trailing spec were also rows of the other one. A second copy of the loop means two places to fix when the accept path changes. ## What changed Only two files change, both under `tests/integration/`. No file under `lua/` or `plugin/` changes. 1. `diff_accept_missing_final_newline_spec.lua` gets two rows. These were the only cases that the trailing spec ran and the table did not: - the proposal `"a\n\nb\n\n"` ("a blank line inside and one after the last line"), and - the old file `"a\n"` ("a file that is a line shorter"). 2. `diff_accept_trailing_newlines_spec.lua` is deleted. 3. The header comment of the table now states which proposals the unified layout lost a line break for. Only a proposal that ends in `"\n\n"` lost one (`"a\n\n"` came back as `"a\n"`). A proposal that ends in a single `"\n"`, such as `"a\n"` or `"\n"`, kept it. The merged spec has 136 tests: 3 layouts × 4 old files × 11 proposals, plus the 4 edit tests that were already there. Before, the two specs had 94 + 28 = 122 tests. ## Nothing is lost 1. All 28 cases of the deleted spec are in the merged spec. All 90 matrix cases of the table before this PR are still in it. 2. The four assertions of the deleted spec are in the table's own helpers: openDiff waits for the user (`call._deferred`), exactly one answer, the answer is `FILE_SAVED`, and the accepted text equals the proposal. 3. Nothing else in the tree names the deleted file (`git grep` for its name, its describe title and `trailing_newlines_spec`: 0 hits at the head and on `origin/main`). ## Verification: the merged spec still catches the bug of coder#535 I reverted coder#535 (the reverse patch of `884f7e22` on `lua/claudecode/diff_inline.lua`) and ran each spec alone on a copy of the head. | Spec | head | coder#535 reverted | |---|---|---| | old trailing spec | 28 ok | 20 ok, 8 failed | | old table spec | 94 ok | 85 ok, 9 failed | | merged spec (this PR) | 136 ok | 120 ok, 16 failed | (The first two rows ran the two specs of the base of this PR, `37ee1e35`, on Neovim 0.12.3 and 0.8.0, with the same counts on both. The merged spec's row ran on Neovim 0.12.3, 0.11.5, 0.10.4, 0.9.5 and 0.8.0, with the same counts on all five.) 1. The 16 failures are the unified layout × 4 old files × the four proposals that end in `"\n\n"`: `"a\n\n"`, `"a\n\n\n"`, `"\n\n"` and `"a\n\nb\n\n"`. Proposals that end in a single `"\n"` pass with coder#535 reverted, as they did before coder#535. 2. Every case that failed in either old spec with coder#535 reverted also fails in the merged spec (the 14 failures of the two old specs, 3 of them shared, are all among the 16). 3. Two of the 16 are new: `"a\n\nb\n\n"` against `"old"` (a file without a final line break) and against a file that does not exist. They come from the new proposal row. ## Gates The head is `1f736d0d95154574376390c92f55bbcf3ad308a3`. It merges `origin/main` at `5c7a4db9`. The full gates ran at `76b50be9`, the merge before it. The commit after it brings only `terminal.lua`, `scripts/`, docs and the new `tests/integration/terminal_cwd_spec.lua` from main, and nothing in the two specs of this PR. At the head I ran `format-check`, `check.sh --hash-only` of all four skills, and the merged spec on Neovim 0.12.3 and 0.8.0 (136 ok each). 1. `mise run all` (`BUSTED_SEED=97531`) exits 0: 1832 successes, 0 failures, 0 errors, 29 pending. luacheck: 0 warnings, 0 errors in 189 files. 2. `mise run format-check` exits 0 at both heads (432 files, 0 changed at the last one). 3. `scripts/run_integration_tests_individually.sh` exits 0 on Neovim 0.12.3 (Total Success 614, Failed 0, Errors 0) and on Neovim 0.8.0 (Total Success 596, Failed 0, Errors 0). 4. `check.sh --hash-only` of `verify-diff-lifecycle`, `verify-mention-queue`, `verify-selection-tracking` and `verify-terminal-lifecycle` exits 0, at both heads. The full `check.sh` of `verify-diff-lifecycle` exits 0 (`main` 93477 distinct states, `live-DisconnectedCallsAnswered` 5557). The PR changes no covered file. One comment line of the table spec is 123 columns wide. It comes from coder#542 and I left it as it was. --- _Generated with [`xum`](https://github.com/coder/xum) • Model: `anthropic:claude-sonnet-5-5` • Thinking: `xhigh`_
…oder#577) ## Why In the `vertical` and `horizontal` layouts, a proposal whose line breaks are all CRLF was loaded into the proposal buffer with `'fileformat'` `unix` and a CR at the end of each line. The save joined the lines with LF. So a line that the user typed, or the join of two lines, came back with LF, or with a CR in the middle of a line, where Neovim's `:w` writes CRLF. A CRLF proposal that equals a CRLF file also showed every line as changed, because the file's own buffer is `dos` and holds its lines without CRs. I found it while verifying coder#551. ## What changed 1. `_setup_blocking_diff` (`lua/claudecode/diff.lua`) loads a proposal as Neovim loads a file. When `'fileformats'` includes `dos`, and the proposal has a line break and only CRLF line breaks, the buffer gets `'fileformat'` `dos` and lines without their CRs. Any other proposal gets `'fileformat'` `unix` and is split at LF, as before. The diff sets `'fileformat'` in both cases, because a new buffer takes the first entry of `'fileformats'`, and that entry can be `dos`. 2. `_resolve_diff_as_saved` joins the lines with the line break of the buffer's `'fileformat'` (LF, CRLF for `dos`, CR for `mac`), and adds the final one when `'eol'` is on. Before, it always used LF. So `:setlocal fileformat=dos` on an LF proposal now answers CRLF, as `:w` writes it. 3. The `unified` layout does not change. Its buffer is read-only, and its answer is the proposal. 4. `lua/claudecode/README.md` states both rules, and that one empty line is saved as one line break of the buffer's `'fileformat'`. 5. `tests/integration/diff_accept_crlf_spec.lua` is new (159 tests). `tests/mocks/vim.lua` gives the mock `vim.o` the default `fileformats = "unix,dos"`, because the unit specs of the diff now read it. 6. `covered.lock` of `verify-diff-lifecycle` records the new hashes of `diff.lua` (the file, `resolve_saved` and `setup_blocking`). ## What a user sees 1. A line that the user types into an accepted CRLF proposal, or two lines that the user joins, comes back with CRLF, as `:w` writes it. For the proposal `"a\r\nb\r\n"` and a typed line after the last line, main answers `"a\r\nb\r\nx\n"` and this PR answers `"a\r\nb\r\nx\r\n"`. 2. A CRLF proposal that equals the CRLF file shows no difference. Main marked every line. 3. An unedited proposal still comes back as exactly the proposal, in every layout, under every `'fileformats'` value that I ran. 4. Nothing else changes for the default `'fileformats'` (`unix,dos`): the controls of the spec (an LF proposal, a proposal with CRLF and LF line breaks, a lone CR, no line break, unedited, emptied) give the same bytes on main and here. 5. `'fileformats'` `unix` (no `dos`) keeps a CRLF proposal as `unix` with its CRs, as the file's own buffer does. The equal-text view shows no difference there, and a typed line gets LF, as `:w` writes it. ## Verification The spec compares the answer with the bytes that Neovim's own `:w` writes for the same text after the same commands (an oracle that reads the text as a file under the same `'fileformats'`), or with an exact expected text. 1. Red, then green, run on Neovim 0.12.3, 0.11.5, 0.10.4, 0.9.5 and 0.8.0 (all five give the same counts): 159 tests pass with the fix. With `origin/main`'s `diff.lua`, 105 pass and 54 fail. The unified cases run on 0.8.0 too (coder#564). 2. Fourteen mutants of the changed lines are each red on Neovim 0.12.3 (the last seven also on 0.8.0, with the same counts). Failed tests per mutant: - Split and join: the CRs stay in the lines 70, the join always uses LF 66, the final break is always LF 56, `mac` is ignored 2, dos for any text with a CRLF 12, dos for a text with no line break 2, dos even with a lone LF 32. - `'fileformats'` check: no check (always dos) 8, an exact match of `unix,dos` or `dos,unix` 4, dos for any value except `unix` 4, dos only when it is the first entry 42, `'fileformat'` always `unix` 64, always `dos` 48, set only for `dos` 12. 3. A probe of 1860 cases per run on Neovim 0.12.3 and 0.8.0 (three layouts, four old files, 15 proposals, 15 edits) compares each answer with the bytes of a fresh `nvim --clean` that reads the proposal as a file under the same `'fileformats'`, runs the same commands and writes with `:w`. The columns give mismatches. The two Neovim versions give the same numbers. | `'fileformats'` | main | this PR | unedited answers that differ from the proposal (main / this PR) | |---|---|---|---| | `unix,dos` | 748 | 144 | 0 / 0 | | `dos,unix` | 788 | 184 | 0 / 0 | | `unix` | 304 | 192 | 0 / 0 | | `dos` | 1104 | 516 | 0 / 0 | | `unix,mac` | 608 | 544 | 0 / 0 | Every mismatch that remains is one of the items under "Not fixed here", or a fold (see below). Cleanup leaves no state, buffer, window or tab behind in any case. Under `unix,dos`, `dos,unix` and `unix`, the equal-text view marks 0 lines in every case. 4. The only cases where this PR has a mismatch that main does not have are `dup_line` and `dd_first` (`yyp` and `dd` on line one) against an old file that equals the proposal, for CRLF proposals: 28 cases for each of `unix,dos`, `dos,unix` and `dos`. Diff mode folds every line of an equal text, so those commands act on the whole fold. Main does the same for an equal LF text (`same|lf|dup_line`), and showed every line of an equal CRLF text as changed, so it did not fold. This follows from item 2 of "What a user sees". 5. An independent verifier ran its own probe (2604 cases per run) under `unix,dos`, `dos,unix`, `unix` and `dos` on all five Neovim versions, and under `unix,dos,mac`, `unix,mac` and `dos,mac` on 0.12.3. In every run, 0 unedited answers differ from the proposal. ## Not fixed here These items are tracked in coder#581. All of them exist on `origin/main` already. An independent probe that compares each answer with `:w` of the same buffer found this PR never worse than main. 1. A CR inside a line, then `:setlocal ff=mac`. `:w` writes that CR as NL, and the save keeps the CR. The proposal `"a\rb\n"` gives `"a\rb\r"`, and `:w` gives `"a\nb\r"`. Main joined with LF, which was also wrong. 2. With `'fileformats'` `unix,mac`, Neovim reads a text such as `"a\r\nb\r\r\n"` as `mac`. The diff never detects `mac` (main neither). In 24 cases (an `ff=mac` edit only), main's answer equals the file-read oracle and this PR's answer does not, because main ignores `'fileformat'` when it joins. Against `:w` of the same buffer, both are wrong in these 24 cases. 3. An empty proposal, then typed lines. `'eol'` stays off (coder#542), so `Gox` gives `"\nx"`, where `:w` of an empty file gives `"\nx\n"`. 4. `'fileformats'` that starts with `dos`, and a proposal with no LF (`""`, `"a"`, `"a\r"`). Neovim reads it as `dos`. The diff keeps `unix`, so a typed line gets LF where `:w` writes CRLF. 5. `'fileformats'` with the single entry `dos`. Neovim then reads every file as `dos`, an LF file too. An LF or mixed proposal stays `unix`, so a typed line gets LF where `:w` of the file writes CRLF. Neovim reads the equal file as `dos`, so the equal-text view still marks lines for a text that is not all CRLF: this PR marks 2 of the 15 probe texts (a mixed text, and one with a leading lone LF), and main marks 11 of 15. ## Gates The head is `1c7b5adcfc165963ffc250ab9ebeedad00ddf954`. It merges `origin/main` at `7dbad04b`. Main moved three times while this PR was in progress, and each time only `covered.lock` conflicted. The gates below ran in full at `4525dad1` (the merge with `origin/main` at `c43b6a30`). The last merge brought coder#574. At the head I ran the checks of item 6, and CI runs the rest on this head. 1. `mise run all` (`BUSTED_SEED=97531`) exits 0: 1837 successes, 0 failures, 0 errors, 34 pending. luacheck: 0 warnings, 0 errors in 194 files. 2. `mise run format-check` exits 0. 3. `scripts/run_integration_tests_individually.sh` exits 0 on Neovim 0.12.3 (Total Success 828, Failed 0, Errors 0) and on Neovim 0.8.0 (Total Success 810, Failed 0, Errors 0). The new spec alone gives 159 passes on Neovim 0.12.3, 0.11.5, 0.10.4, 0.9.5 and 0.8.0. 4. `check.sh --hash-only` of the four `verify-*` skills exits 0. The full `check.sh` of `verify-diff-lifecycle` exits 0 (`main` 93477 distinct states, `live-DisconnectedCallsAnswered` 5557). The merge with main conflicted in `covered.lock` only. I took my side, ran the full `check.sh` on the merged tree (all 23 facts TRUE, the model passes), and then ran `--record`. The last merge moved only `file` hashes (`diff.lua`, `init.lua`, `selection.lua`) and no block. Against `origin/main`, the lock moves 3 lines: the file hash of `diff.lua`, and the blocks `resolve_saved` and `setup_blocking`. No guard moved. The change decides which bytes the accepted text holds. It does not change a diff's status, who gets its answer, or when. 5. At the head, after the last merge: the full `check.sh` of `verify-diff-lifecycle` (all 23 facts TRUE, `main` 93477, `live-DisconnectedCallsAnswered` 5557) and then `--record`. coder#574 changed the blocks `open_blocking`, `handle_invoke` and `setup_blocking` on main, and the lock of this PR moves only the 3 lines of this PR against `origin/main` (the file hash of `diff.lua`, `resolve_saved`, `setup_blocking`). 6. At the head: `format-check` exits 0, `check.sh --hash-only` of the four skills exits 0, the new spec gives 159 passes on Neovim 0.12.3 and 0.8.0, and the 44 unit specs of the diff, tools and server modules give 752 successes, 0 failures. --- _Generated with [`xum`](https://github.com/coder/xum) • Model: `anthropic:claude-sonnet-5-5` • Thinking: `xhigh`_
Why
The vertical and horizontal diff layouts must hand Claude back the exact text it proposed when the user accepts. A proposal without a final line break came back with one.
"a"was accepted as"a\n","a\nb"as"a\nb\n", and an empty proposal as"\n". Claude then wrote a file with a line break the user never saw in the diff.What changed
_resolve_diff_as_savedinlua/claudecode/diff.luasaves the lines of the proposal buffer, joined with line breaks. It adds a final line break when the buffer's'eol'option is on. A new Neovim buffer has'eol'on, so every saved proposal ended with a line break.The diff now sets
'eol'on the proposal buffer from the proposal text, right after it sets the buffer's lines in_setup_blocking_diff. The saved text then ends with a line break exactly when the proposal does. The saving code is the same. The user's edits in the buffer still reach Claude and keep the proposal's ending.lua/claudecode/README.mdgets one new bullet that states this. It sits after the bullet about writing the proposed buffer, so it does not touch the line that the unified fix (F11) edits.Scope
verticalandhorizontallayouts share one code path, so one change fixes both.unifiedlayout is unchanged. Its no-final-line-break cases are controls in the new spec and pass before and after."\n"gets the answer"\n", while Neovim writes an empty file for an empty buffer. It exists on main, and it is not changed here. The follow-up F16 fixes it.Blast Radius
'diffopt'and'fixeol', diff mode shows the same highlights, and the proposal buffer is modified as before. I probed both in a real Neovim. Withset nofixeolanddiffopt-=internal, one case highlights differently, which is a real difference between the endings.covered.lock: the change touches_setup_blocking_diff, a covered block.check.shprinted STALE for thediff.luafile hash and thesetup_blockingblock hash only. It printed no changed call site, and all 21 code facts stay TRUE. The change adds onenvim_buf_set_optioncall on a buffer that the same function created a few lines before. It sits next to the other buffer option calls and runs in the samepcall. It does not change when the diff registers, when it answers, or how it resolves or closes. So I did not change the model (model/DiffLifecycle.tla). The model passed (main: 89205 distinct states,live-DisconnectedCallsAnswered: 5269).check.sh --recordwrote the new lock andcheck.sh --hash-onlyreports that it matches.How this PR merges with F11 and F12
fix/server-auth-log-and-cwd-lookup) merges with this branch without a conflict. I checked it withgit merge-tree --write-tree.Merge rule for F11 (#535) and F14
F11 and F14 merge without a text conflict, but each leaves a test that the other makes stale. F11's spec lists 2 vertical cases as pending, because main still adds a final line break there. F14's spec skips 9 unified cases that end in blank lines, because main still drops one of those line breaks.
Whichever of the two PRs lands second must do this in its own merge-from-main commit:
tests/integration/diff_accept_trailing_newlines_spec.lua. It replaces the 7-linelocal case = itpending clause with oneit(line, so the 2 vertical no-final-line-break cases run.tests/integration/diff_accept_missing_final_newline_spec.lua. It removes theblank_endskip (theif not (layout == "unified" and proposal.blank_end) thenline and itsend, the 3blank_end = truefields and their comment), so the 9 unified blank-line cases run.On a trial merge of the two branches with both diffs applied, F11's spec gives 28 successes, 0 failures and 0 pending. F14's spec gives 94 successes, 0 failures and 0 pending.
The two diffs
Verification
Red before the fix, green after (same spec, only
lua/claudecode/diff.luaswapped):tests/integration/diff_accept_missing_final_newline_spec.lua(85 cases, real Neovim,:write)The spec runs ten proposals against three old files (other text, no final line break, and no file yet) in the vertical and horizontal layouts. The proposals end in no line break (
"a","a\nb", an empty proposal,"a\n\nb"), in one line break ("a\n","a\nb\n","\n") or in blank lines ("a\n\n","a\n\n\n","\n\n"). Each case compares the answer to the proposal byte for byte. The 26 red cases are the 24 no-line-break cases of the two split layouts and the 2 edit cases. The unified layout runs the 7 proposals that do not end in blank lines against the same three old files (21 cases, all green before and after). Four more cases append a line to the buffer after it opens, in both split layouts. They show that edits still reach Claude and keep the ending.Gates on the final commit, which merges
origin/main(036427a) into the branch (private XDG folders):mise run all: native exit 0. luacheck: 0 warnings / 0 errors in 179 files. busted: 1682 successes / 0 failures / 0 errors / 25 pending. treefmt: 0 files changed.mise run format-check: native exit 0 (356 files, 0 changed).tests/unit/diff*_spec.lua,opendiff_blocking_spec,new_file_reject_then_reopen_spec, and theopen_diff_mcp,close_tabandclose_all_diff_tabsspecs): 222 successes, 0 failures.scripts/run_integration_tests_individually.sh: 416 successes, 0 failed, 0 errors. The new spec gives 85 successes..agents/skills/verify-diff-lifecycle/check.sh --hash-only: covered.lock matches the code.Two independent verifiers (Opus and GPT) checked the code commit before the later spec-comment commit and the merge with
origin/main. Both found no introduced defect. Their minor findings are in the branch (the spec comment and a case name) and in this body.Generated with
xum• Model:anthropic:claude-sonnet-5-5• Thinking:xhigh