Skip to content

fix(diff): open the legacy diff's temp file without a swap file - #501

Merged
ThomasK33 merged 4 commits into
mainfrom
fix/diff-legacy-noswapfile
Nov 11, 2026
Merged

ThomasK33 merged 4 commits into
mainfrom
fix/diff-legacy-noswapfile

Conversation

@ThomasK33

@ThomasK33 ThomasK33 commented Nov 11, 2026 •

Copy link
Copy Markdown
Member

The legacy open_diff raised E325 when a swap file for <path> (New) existed

require("claudecode.diff").open_diff(...) (the public legacy entry point, which wraps _open_native_diff) opens the proposal's temp file in a split and renames the buffer to <path> (New). At the rename Neovim opens a swap file under the new name. If a swap file with that name exists and its header names that buffer, the rename raises E325: ATTENTION. That happens when another Neovim changed a buffer with that name, or when a Neovim that was killed after :preserve left it.

What the caller sees on main: E5108 with a stack trace from diff.lua:814, a half-open diff (one of the two windows is in diff mode), a buffer that has a swap file, and a leftover temp file.

Who is affected

Only code that calls open_diff itself, such as a user's own Lua. No plugin action calls it: openDiff uses _setup_blocking_diff (grep finds no caller of open_diff or _open_native_diff in lua/ besides the wrapper). So this is a small hardening fix, not a regression of a user-facing flow.

The fix

lua/claudecode/diff.lua, one line: the temp file is opened with noswapfile edit. The rename then has no swap file to meet. The buffer becomes nofile with swapfile off a few lines later anyway, so no swap file is lost.

Proof

  • New real-Neovim spec tests/integration/diff_legacy_swap_spec.lua. A second Neovim changes a buffer named <path> (New) and is killed with SIGKILL after :preserve, which leaves a swap file whose header names that buffer. Then the spec calls open_diff, and asserts that it returns success, that the proposal buffer has no swap file, and that its name is <path> (New).
    • On main: fails with diff.lua:814: Vim:E325: ATTENTION on Neovim 0.12.3 and on v0.8.0.
    • With this change: passes on both.
  • All other tests/integration/diff_*_spec.lua files pass on 0.12.3 and v0.8.0.
  • mise run format-check and mise run check are clean. mise run test: 1488 successes, 0 failures, 20 pending (the new spec is pending under busted, because it needs a real Neovim). tests/unit/diff_spec.lua still passes (its native-diff case only looks for an edit command, and noswapfile edit contains it).
  • .agents/skills/verify-diff-lifecycle/covered.lock is re-recorded (after merging origin/main at bad488f and again at d63c9b3 (fix(server): judge a silent Claude only after reading its socket #488): each merge conflicted in the lock, so I took main's lock and ran --record). Only the hash of diff.lua changed, and _open_native_diff is outside the model (the model covers openDiff through _setup_blocking_diff). The model passes (89,205 distinct states).

The probe, the patch and the first version of the spec come from the verifier's evidence.


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

The public legacy `require("claudecode.diff").open_diff` (through `_open_native_diff`) opens the proposal's temp file,
then renames the buffer to "<path> (New)". Neovim opens a swap file under the new name at the rename. If a swap file for
that name exists and its header names that buffer (another Neovim changed it, or a killed one after :preserve),
the rename raises E325. The user got E5108 with a stack trace, a half-open diff (one of two windows in diff mode), a
buffer with a swap file and a leftover temp file.

The temp file is opened with `noswapfile edit`, so the rename has no swap file to meet. The buffer is `nofile` and has
`swapfile` off right after anyway.

Test: tests/integration/diff_legacy_swap_spec.lua leaves a swap file for "<path> (New)" with a Neovim that is killed
after :preserve, then calls open_diff. It fails on main (E325 at diff.lua:814) on Neovim 0.12.3 and v0.8.0 and passes
with the change. The verify-diff-lifecycle lock is re-recorded: only the hash of diff.lua changed, and
`_open_native_diff` is outside the model.

---
_Generated with [`xum`](https://github.com/coder/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

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-11T13:31:16.550521Z ecf7e6e Manual request
🔒 Security Review ✅ Completed 2026-11-11T13:33:11.529958Z ecf7e6e 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. You're on a roll.

Reviewed commit: 1873cdf8ab

ℹ️ 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: 1873cdf8ab

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.

@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. Keep them coming!

Reviewed commit: 51f71c69a0

ℹ️ 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: 51f71c69a0

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.

@ThomasK33

Copy link
Copy Markdown
Member Author

@codex review

@ThomasK33

Copy link
Copy Markdown
Member Author

@codex security review

@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: ecf7e6ea1f

ℹ️ 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: ecf7e6ea1f

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.

@ThomasK33
ThomasK33 added this pull request to the merge queue Nov 11, 2026
@github-merge-queue
github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Nov 11, 2026
@ThomasK33
ThomasK33 added this pull request to the merge queue Nov 11, 2026
@github-merge-queue
github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Nov 11, 2026
@ThomasK33
ThomasK33 added this pull request to the merge queue Nov 11, 2026
Merged via the queue into main with commit 09b78fd Nov 11, 2026
6 checks passed
@ThomasK33
ThomasK33 deleted the fix/diff-legacy-noswapfile branch November 11, 2026 15:35
ThomasK33 added a commit that referenced this pull request Nov 11, 2026
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_
hareki pushed a commit to hareki/claudecode.nvim that referenced this pull request Nov 12, 2026
)

## Why

`scripts/run_integration_tests_individually.sh` stops a file's Neovim
after 30 s (`timeout -k 5s 30s`).
`tests/integration/selection_held_spec.lua` is a real-timer spec: it
sleeps 21.6 s itself, in 77 `vim.wait` calls (counted by wrapping `wait`
in a copy of the spec: `WAITED_MS=21640 CALLS=77`). One run of coder#494 on a
loaded host reached the 30 s limit and passed on rerun.

I could not reproduce that run, but the arithmetic says why it can
happen. The spec has 8.4 s of slack in 30 s, which is 0.11 s per
`vim.wait` call. A loaded host that makes each call return 0.11 s late
is enough.

## What changes

The per-file limit is 40 s instead of 30 s. That leaves each of the 77
calls 0.24 s of slack (18.4 s / 77), twice the slack it had.

- `scripts/run_integration_tests_individually.sh`: one variable,
`FILE_TIMEOUT=40s`, sets both the `timeout` limit and the number in the
`TIMEOUT:` line, so they cannot drift.
- `scripts/README.md` line 22 names the new limit and the 50 s ceiling.
- `tests/unit/integration_runner_spec.lua`: the two assertions that pin
the limit take the new value. Two new tests: one fails when the limit
plus the kill grace reaches 50 s, and one fails when the `TIMEOUT:` line
names another limit than the one given to `timeout`.

I did not shorten the spec's waits. They are the debounce (100 ms) plus
a margin on each step. Cutting them would trade a slow runner for a
flaky spec.

### Why the limit stays under 50 s

plenary's `test_harness` applies its own per-file timeout of 50 s
(`timeout = 50000` in `test_harness.lua`), and `test_file` does not pass
on the runner's options. So with a limit of 50 s or more, the runner's
`timeout` never decides. A hung file ends at about 51 s with exit 1,
**no `TIMEOUT:` line**, and its nvim left running with parent pid 1. A
first version of this PR used 60 s and had exactly that problem (found
by the verifier). With 40 s, the limit plus the 5 s kill grace is 45 s,
under 50 s.

This is existing behavior on `main`, and this PR does not change
plenary's 50 s. Passing a timeout in the harness options is a separate
follow-up.

A hung file now costs 40 s instead of 30 s before the runner reports it.

## Evidence

| Hung spec (sleeps 120 s), real runner script | Result |
|---|---|
| `main` (30 s) | `TIMEOUT:` line at 30.8 s, no nvim left |
| 60 s (first version of this PR) | exit 1 at 51.0 s, no `TIMEOUT:`
line, nvim left running (ppid 1) |
| this head (40 s) | exit 1 at 40.3 s, `TIMEOUT: hang_spec.lua was
stopped (exit 124): nvim did not exit within 40s`, saved nvim pid not
running |

| `selection_held_spec.lua` | Result |
|---|---|
| through the runner, alone, 3 runs | 22.6 s, 22.5 s, 22.5 s, exit 0, no
`TIMEOUT:` line |
| two full runs at the same time, host load average 15 to 18 on 17 cores
| 22.9 s and 22.8 s (verifier rerun; the spec's own waits add up to 21.6
s) |
| every `vim.wait` 0.2 s longer (+15.4 s), limit 30 s | nvim exit 124 at
30.3 s (the runner exits 1), 10 `Success` rows, no `Fail` row, no
summary |
| every `vim.wait` 0.2 s longer, limit 40 s | exit 0 at 38.2 s, 14
passed, 0 failed |

So the failure mode of a slow host is the runner's limit, not an
assertion: the file is killed while its tests still pass.

Mutants of the script, each run through
`tests/unit/integration_runner_spec.lua` before coder#502 was merged in (15
tests, all pass unmutated):

| Mutant | Result |
|---|---|
| limit 60 s | 3 fail: the two pins and the 50 s test |
| limit 45 s (45 + 5 reaches 50) | 3 fail, including the 50 s test |
| kill grace 10 s (40 + 10 reaches 50) | 3 fail, including the 50 s test
|
| `TIMEOUT:` line names 30 s | 1 fails: the new line test |

Gate on the head with `main` `f932d7b` (coder#502) merged in: `mise run clean
&& mise run all` exit 0 (seed 1794412306, 1517 passed, 0 failed, 20
pending). `tests/unit/integration_runner_spec.lua` passes 40 of 40 on
that merge, and the diff against `main` is these 3 files.

## The success count

Before coder#502, the runner printed `Total Success: 311` where 290 cases
ran: it counted each file's `Success:` summary line as one more passed
test (290 `Success ||` rows plus 21 summary lines). coder#502 reads only the
summary count, and its spec checks the totals. I ran coder#502's script on a
tree with this change: `Total Success: 290`. That was before coder#501
merged; at this head, with 22 files, a local run and the CI stable job
print `Total Success: 291` (the v0.8.0 job prints 204). This PR does not
touch that code.

---
_Generated with [`xum`](https://github.com/coder/xum) • Model:
`anthropic:claude-sonnet-5-5` • Thinking: `xhigh`_
hareki pushed a commit to hareki/claudecode.nvim that referenced this pull request Nov 12, 2026
…ups of coder#510 and coder#513 (coder#520)

## What you see

1. **A space in `TMPDIR`.** 3 tests of
`tests/integration/diff_hand_over_spec.lua` ("names a swap file step
...") fail. They fail on `main` too. The plugin's message names the step
`:split <path>` with the path escaped (`a\ b`), and the test's pattern
did not match it at all, so the step was nil (`Passed in: (nil)`).
2. **No CI job uses an unusual `TMPDIR`.** The `%` bug that coder#510 fixed
(an Ex command in a test that took a temp path unescaped) stayed green
in CI. Nothing would catch it again.
3. **The integration runner writes into your real cache folder.** One
spec, `tests/integration/diff_legacy_swap_spec.lua` (from coder#501), lets
the legacy diff create `claudecode_diffs` under the real
`stdpath("cache")`, so every integration run adds
`~/.cache/nvim/claudecode_diffs`.

## What changes

1. `diff_hand_over_spec.lua` reads the step with `:(split .-) %(answer
the swap file prompt`, so an escaped path survives.
2. `tests/unit/tmpdir_suite_spec.lua` compared the child's output with
`find("1 success")`, which also matches `11 successes`. It now compares
the parsed count to 1.
3. `tests/unit/luacov_config_spec.lua`: no change. `load_runner` on
`main` already replaces the `os` table of the `.luacov` chunk through
its environment (coder#513 as merged). It never swaps `os.getenv`.
4. `mise.toml`: the two comments named "PUC Lua 5.1" for the pin `lua =
"5.5.0"`. They now say PUC Lua 5.5.0 is installed for its bundled
LuaRocks, and that the 5.1 yield limit is that of stock PUC Lua 5.1
(LuaJIT is the test runtime). Comment-only. CI's mise and rocks caches
key on this file, so this PR's own run builds cold once. They refill
after the merge.
5. `.github/workflows/test.yml`: the integration job runs the runner a
second time with `TMPDIR` set to a created folder named `odd tmp 50%`.
One more step, on both Neovim legs.
6. `diff_legacy_swap_spec.lua` points `XDG_CACHE_HOME` at its temp
folder for the test and restores it. A second new step fails the job
when the runs left a folder under the real cache path. A fresh runner
has none, so the check needs no allow list. It cannot run locally with a
real home: a developer's own sessions fill that folder.

## Evidence

Integration runner, all files, plenary at CI's pinned commit, stub
`claude`:

| Tree | Neovim | `TMPDIR` | Result |
|---|---|---|---|
| `main` | 0.12.3 | space | 313 passed, 3 failed (`diff_hand_over_spec`)
|
| `main` | 0.12.3 | space and `%` | 313 passed, 3 failed (same file) |
| this branch | 0.12.3 | unset, `%`, space, space and `%` | 316 passed,
0 failed, each |
| this branch | v0.8.0 | unset, `%`, space, space and `%` | 229 passed,
0 failed, each |

In each of those 8 runs on this branch, `XDG_CACHE_HOME` was a fresh
folder, and no `nvim` folder appeared in it.

The CI step turns red when a `fnameescape` of coder#510 is reverted: with the
one in `diff_new_tab_mode_spec.lua` reverted, the runner under `odd tmp
50%` gives 294 passed, 22 failed, exit 1. Without the revert it is green
(rows above).

The cache check, run with the real home swapped for an empty folder:

| Spec file | Check |
|---|---|
| `diff_legacy_swap_spec.lua` as on `main` | red: finds
`.cache/nvim/claudecode_diffs` (the spec passes) |
| `diff_legacy_swap_spec.lua` of this branch | green: no `.cache/nvim` |

I ran each of the 24 integration specs alone with a fresh
`XDG_CACHE_HOME`. Only `diff_legacy_swap_spec.lua` created anything
(`nvim/claudecode_diffs`). The busted unit suite (1586 tests) with a
fresh `XDG_CACHE_HOME` created nothing.

CI time: on CI at this head the extra runner pass took 42.6 s on the
stable leg and 37.1 s on the v0.8.0 leg. The two legs run in parallel,
so the job gets under 1 minute longer. The cache check takes under a
second.

Gate: `mise run clean && mise run all` exit 0 (seed 1794418531, 1586
passed, 0 failed, 22 pending). `actionlint` passes.

---
_Generated with [`xum`](https://github.com/coder/xum) • Model:
`anthropic:claude-sonnet-5-5` • Thinking: `xhigh`_
hareki pushed a commit to hareki/claudecode.nvim that referenced this pull request Nov 13, 2026
…tep fails (coder#516)

## The legacy `open_diff` left its temp file behind when a step failed

`require("claudecode.diff").open_diff(...)` (the public legacy entry
point, which wraps `_open_native_diff`) writes the proposal to a temp
file, then opens the original and the proposal side by side. It
registers the autocmd that deletes the temp file as its last step. A
step that raises before that leaves the temp file in
`stdpath("cache")/claudecode_diffs`, and sometimes a proposal buffer.

Four paths raise after the temp file exists. The first three were found
by the verifier of coder#501:

1. A second `open_diff` for a path whose proposal is still open: the
rename to `<path> (New)` raises `E95: Buffer with this name already
exists`.
2. A changed swap file for the real file (another Neovim changed it, or
a killed one after `:preserve`): `edit` of the original raises `E325`.
3. An error in a `FileType` autocmd for lua. Neovim's filetype detection
ignores the `.new` ending of the temp file, so `FileType` fires for it
too. Which `edit` raises depends on the layout:
- The original is the current buffer: `edit` reads it again and
`FileType` fires for the original. The error stops the diff at its first
step.
- The original is loaded but hidden: `edit` does not read it again, so
`FileType` fires for the proposal's temp file, after the diff has split
a window and turned diff mode on.
- After the rename the plugin sets the filetype again. An error in
`FileType` there reaches the caller on Neovim 0.8 and is only a message
on 0.12.
4. The original has unsaved changes and is the current buffer of the
target window: `edit` of it raises `E37: No write since last change`. I
ran this by hand on 0.12.3 and v0.8.0: the call raises `E37`, and no
temp file, window or buffer is left over. The spec has no case for it.

Any other error that one of these steps raises is cleaned up the same
way, because the fix wraps the whole sequence.

Who is affected: only code that calls `open_diff` itself. Nothing in the
plugin calls this path: the `openDiff` tool uses `open_diff_blocking`.

## The fix

`lua/claudecode/diff.lua`: the steps after `create_temp_file` move into
`show_native_diff`, unchanged. `_open_native_diff` calls it with
`pcall`. If it raises, `remove_failed_proposal` does two things, then
`_open_native_diff` raises the same error again (the message has one
position, as before):

1. It wipes the buffer that has the temp file's name, unless a window
shows that buffer or it has unsaved changes. It finds the buffer by the
name or by the real path, because Neovim resolves a link in a buffer's
name, so a cache folder that is a link changes the name.
2. It deletes the temp file and its folder.

Every step is protected, so an error in one cannot stop the next or
replace the error that made the diff fail.

It does nothing else. It closes no window, moves no focus or tab, and
turns no diff mode off. A failed `open_diff` can still leave the windows
that it had already split, and the diff mode it had turned on, as `main`
does. A proposal buffer that a window shows stays too, and so does a
proposal that was already renamed to `<path> (New)`. The window check
looks at every tab. Because a window that shows the proposal stays, the
temp file behind it is gone, and the next `:checktime` prints `E211:
File "..." no longer available` once (I checked this on 0.12.3 and
v0.8.0). No text is lost. `lua/claudecode/README.md` says so.

## Why this PR does not undo the windows

Earlier versions of this PR also undid the windows, the diff mode and
the focus of a failed diff. In four review rounds, Codex and the
verifier each found one more way for that undo to reach something that
was not the diff's own: another diff's mode, a window that an autocmd
opened, the split of another tab, a buffer of the user's with unsaved
text. Each one was an autocmd that changes the layout while the diff
runs. I removed the undo instead of fixing a fifth case. All Codex
threads are resolved. Two of the three round-4 threads (4543276628,
4543276636) are about code that this PR no longer has. The third
(4543276642) is about the spec's `after_each`, which this PR keeps: it
now runs `tabonly!` first.

## Spec hygiene

`diff_legacy_swap_spec.lua` (coder#501) and the cleanup spec leave a swap
file by killing a child Neovim with SIGKILL after `:preserve`. A killed
Neovim cannot delete its socket, so each run of each spec left one
folder behind: `$TMPDIR/nvim.$USER/<random>/`, or `nvim.<pid>.0` in
`$XDG_RUNTIME_DIR` when that is set. The child now starts with `TMPDIR`
and `XDG_RUNTIME_DIR` set to a folder inside the spec's own directory,
which `after_each` deletes. I measured both specs with an empty private
`TMPDIR` and `XDG_RUNTIME_DIR`, on 0.12.3 and v0.8.0: one folder per run
before the change, none after.

The cleanup spec also points `XDG_CACHE_HOME` at its own folder, as the
swap spec does since coder#520. `open_diff` writes under `stdpath("cache")`,
and the CI step from coder#520 fails the job when a spec leaves a folder in
the real cache. Run with a private `HOME`, the integration runner leaves
no `~/.cache/nvim`.

## Proof

`tests/integration/diff_legacy_cleanup_spec.lua` has 18 real-Neovim
cases. Each asserts that the call raises and that no temp file is left,
and each one that has a user's window, buffer or diff mode in it asserts
that this stays.

| # | Case | What it checks |
|---|---|---|
| 1 | E95, a second diff of the same path | The first diff stays open
and in diff mode. No window is closed. The error has one position, as
before. |
| 2 | E325 | A swap file for the original. |
| 3 | `FileType` error, original current | The diff stops at its first
step. |
| 4 | `FileType` error for the proposal, original hidden | No window is
closed. |
| 5 | The cache folder is a link, and a step fails after the rename |
The unlisted buffer under the temp file's name is found by its real path
and wiped. |
| 6 | Autocmds raise as that buffer is wiped | The cleanup finishes and
the first error is raised. |
| 7 | A step fails after the rename (`BufFilePost` raises) | No buffer
has the temp file's name. |
| 8 | A buffer has the proposal's name, original current | No window is
closed. The buffer that was there stays. |
| 9 | Sidebar on the left and a diff already open | That diff stays on.
The sidebar stays. No window is closed. |
| 10 | The user's own `:diffthis` in two windows | That diff stays on.
No window is closed. |
| 11 | A diff is open, and the call starts in a prompt window of another
tab | The open diff stays on. |
| 12 | The diff starts in a prompt window and picks the editor window of
another tab | The prompt window stays. No window is closed. |
| 13 | An autocmd opens a window, then raises | That window stays. |
| 14 | A `WinNew` handler opens a window with unsaved text | The text
stays. No window is closed. |
| 15 | A `WinEnter` handler sends the cursor back to the editor window |
The editor window stays. No window is closed. |
| 16 | A `BufEnter` handler wipes the proposal and leaves a new buffer
with text | The buffer of the user and its text stay. |
| 17 | A `BufEnter` handler changes the proposal and moves the window to
another buffer | The proposal stays, with its text. |
| 18 | A `BufEnter` handler shows the proposal in a new tab and then
gives the split another buffer, so only a window of another tab shows it
| That window stays. No window is closed. |

With the `diff.lua` of `main`, 18 of 18 fail on Neovim 0.12.3 and on
v0.8.0. With this head, 18 pass on both.

Mutants: I changed `remove_failed_proposal` and `_open_native_diff` in
16 ways, one at a time, and ran the spec on both versions. 13 are
killed: no wipe, no temp file delete, no cleanup call, no re-raise,
`error(failure)` for `error(failure, 0)`, wipe a buffer with unsaved
changes, wipe a buffer that a window shows, no real-path match, delete
before wipe, a cleanup that turns diff mode off or closes other windows,
and two window checks that look at the current tab only (`bufwinnr` and
`bufwinid` for `win_findbuf`). Those last two passed the first 17 cases
and fail only case 18, on both versions. Three survive. No `pcall`
around the wipe, because an autocmd that raises during `nvim_buf_delete`
is only a message on both versions. No `pcall` around the temp file
delete, because `cleanup_temp_file` reports a failed delete with
`vim.notify` and does not raise. Wiping the current buffer when no
buffer has the name, because the unsaved-changes check and the window
check each keep a buffer of the user's.

The verifier's probe (`probe.sh` and `drive.lua`, 22 scenarios) on this
head and on `main`, on both versions: the windows, the buffers, the diff
modes and the focus after the call are the same in all 22 scenarios. The
only differences are the temp files, which are gone (so a window that
still shows the proposal sees `E211` once on the next `:checktime`),
and, in one scenario, an unlisted `foo.lua.new` buffer that no window
showed, which is wiped.

Other gates:
- `mise run format-check` and `mise run check` (0 warnings in 177 files)
are clean.
- `mise run test`: 1655 successes, 0 failures, 24 pending (the cleanup
spec is pending under busted, because it needs a real Neovim).
- The integration runner passes on both versions with 0 failures, once
normally and once under a `TMPDIR` named `odd tmp 50%` (the two runs
that the CI job makes since coder#520), in a private `HOME` with no
`XDG_CACHE_HOME`: 337 tests per run on 0.12.3 and 250 per run on v0.8.0.
The real cache folder stays untouched. The 15 `diff_*_spec.lua` files
(the two legacy specs and 13 others) ran in each run: 30 runs in each
mode, 0 failures.
- `.agents/skills/verify-diff-lifecycle/covered.lock` is re-recorded.
Only the hash of `diff.lua` changed, and `_open_native_diff` is outside
the model. The model passes (89,205 distinct states).

## Findings that `main` already has

The verifier of this PR found two defects that `main` has too, and this
PR does not change them:

1. A `WinEnter` autocmd that raises inside the diff's own split leaves
that split open.
2. A failed diff does not give the target window back the buffer that it
showed before.

Both are about windows. Nothing in the plugin calls this path, so they
stay as they are.

---
_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