Skip to content

fix(diff): invalidate stage buffers when a newly added file is unstaged - #298

Merged
dlyongemallo merged 1 commit into
dlyongemallo:mainfrom
Phaen:fix/stale-stage-buffer-on-unstage
Jul 20, 2026
Merged

fix(diff): invalidate stage buffers when a newly added file is unstaged#298
dlyongemallo merged 1 commit into
dlyongemallo:mainfrom
Phaen:fix/stale-stage-buffer-on-unstage

Conversation

@Phaen

@Phaen Phaen commented Jul 17, 2026

Copy link
Copy Markdown

Problem

Unstaging a newly added file (status AM??) while its diff is open leaves the view permanently out of sync. Depending on timing, one of two symptoms appears:

  1. Stale diff — the :0: (index) side keeps showing the old staged content on every refresh. R / :DiffviewRefresh doesn't help; only closing the tab and reopening does.
  2. Crash chainFailed to create diff buffer ':0:<path>' followed by E5560: nvim_buf_is_valid must not be called in a fast event context, when the stage buffer is (re)created after the blob has left the index.

Which symptom you get depends on whether blob_hash was successfully captured when the stage buffer was created.

Root cause

Three stacked defects, all downstream of the same event — the path leaving the index entirely (unlike unstaging a tracked file, which merely resets the stage-0 blob back to the HEAD blob):

  1. GitAdapter:file_blob_hash() used fail_on_empty = true, so the legitimate "path has no blob at this rev" answer (empty output, exit 0) was treated as a retryable job failure: 3 retries, error log spam on every refresh, and a nil return indistinguishable from a real failure.
  2. FileEntry:validate_stage_buffers() gated disposal on if new_hash and new_hash ~= f.blob_hash, so a nil hash silently skipped invalidation and the stale buffer survived every refresh.
  3. The NOOP branch of DiffView:update_files() keeps an entry when its path and revs are unchanged, copying only status onto it. But each side's nulled flag is fixed at construction, and it derives from the status (Diff2.should_null): the M? transition must flip the a-side to nulled. The kept entry instead retained a live a-side pointing at a blob that no longer exists — the source of both the stale buffer and the failed reload.

Fix

  • file_blob_hash(): drop fail_on_empty; map empty output to nil explicitly. "No blob at this rev" is a real answer, not a failure — this also removes the retry storm and log spam.
  • validate_stage_buffers(): treat a nil new hash as a content change and dispose the buffer (the unmodified-buffer guard is unchanged).
  • update_files() NOOP branch: force entry replacement when any side's nulled state would change, so the a-side of a now-untracked file renders as a proper null buffer instead of attempting to load a gone blob.

Repro

With a Diffview tab open the whole time:

  1. Create a new file (not in HEAD) with some content.
  2. :DiffviewOpen, select the file — left side is empty (null buffer), right side is the working tree file, as expected for an untracked file.
  3. git add the file, then edit it further in the working tree.
  4. View the file again — left side now shows the staged version, right side the working tree. Still correct.
  5. git reset the file (it becomes untracked again).
  6. View the file — left side still shows the staged version, even though no staged version exists anymore. Expected: left side empty again, as in step 2. Refreshing (R / :DiffviewRefresh) doesn't help; only closing the tab and reopening does.

** Full disclosure**: This PR was created with AI-assistance. The bug was found, reproduced and flagged by a human. I'm one of those silly web devs, don't take my lua-based PR on face value. :)

Unstaging a newly added file (AM -> ??) removes its path from the index
entirely, unlike unstaging a tracked file which resets the stage-0 blob
back to the HEAD blob. Three stacked defects left the open diff view
permanently out of sync with no recovery short of closing the tab:

1. GitAdapter:file_blob_hash() used fail_on_empty, so the legitimate
   "path has no blob at this rev" answer (empty output, exit 0) burned
   retries, spammed the log on every refresh, and returned nil.
2. FileEntry:validate_stage_buffers() gated disposal on a non-nil new
   hash, so the stale stage buffer survived every refresh, including
   explicit ones via refresh_files.
3. The NOOP branch of DiffView:update_files() copies status onto a kept
   entry, but each side's nulled flag is fixed at construction and
   derives from the status: the M -> ? transition must flip the a-side
   to nulled. The kept entry retained a live a-side pointing at a gone
   blob, producing either the stale buffer or a failed buffer reload
   (E5560 fast-event crash chain), depending on whether blob_hash was
   captured at buffer creation.

Fix: treat empty rev-parse output as a real "no blob" answer, dispose
stage buffers when the blob hash goes away, and force entry replacement
whenever a side's nulled state would change so the a-side of a now-
untracked file renders as a proper null buffer.
@dlyongemallo
dlyongemallo merged commit 62dc5ad into dlyongemallo:main Jul 20, 2026
7 of 8 checks passed
@dlyongemallo

Copy link
Copy Markdown
Owner

Thanks for the fix @Phaen!

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.

2 participants