[Architecture] Lifecycle teardown hygiene: save/restore buffer-local keymaps and window options

[esmuellert]

Summary

codediff sets ~28 buffer-local keymaps on user file buffers (original_bufnr + modified_bufnr in WORKING mode) and ~3 window-local options (scrollbind, cursorbind, diff) without ever saving the pre-existing user/plugin state and without restoring it on teardown. When a CodeDiff session ends — or when a buffer escapes the session via gf / <C-w>d without ending it — the user is left with codediff's overrides nailed to their buffer until they restart Neovim.

This is a tracking issue for the lifecycle-cleanup refactor that fixes the class. It collects every reported symptom under one umbrella so they can close via a single coherent fix.

The mechanism (current code)

Symptom area	Set site	Teardown site	Save before set?	Restore on teardown?
~14 view keymaps	lua/codediff/ui/lifecycle/accessors.lua:442 set_tab_keymap	clear_tab_keymaps line 466	❌	❌ delete only
4 hunk-staging keys	lua/codediff/ui/view/keymaps.lua:647,650,653,677 (bypass the chokepoint)	name-collision sweep — ih leaks because line 479 hard-codes mode = "n"	❌	❌
10 conflict keys + do/dp overrides	lua/codediff/ui/view/conflict/keymaps.lua:31..140 (bypass the chokepoint)	never deleted at all	❌	❌
scrollbind / cursorbind window options	lua/codediff/ui/view/side_by_side.lua	only restored on tab-close path; leaks when buffer escapes via gf or <C-w>d	❌	❌

Grep confirms zero occurrences of maparg, mapset, keymap_save, saved_keymap, original_map across the entire tree. No prior attempt at save-and-restore exists.

The gf (open-in-previous-tab) flow with the default close_on_open_in_prev_tab = false is the most insidious case: the diff session stays alive AND the file buffer now appears in another tab still wearing codediff's [c/]c (etc.) bindings. No teardown event ever fires for that escape until the whole session ends.

Proposed fix

Single per-session registry of overridden state, keyed by (bufnr | winid, scope, key), populated at set-time and replayed/del'd at teardown.

-- lua/codediff/ui/lifecycle/accessors.lua (sketch)

local function snapshot_keymap(bufnr, mode, lhs, sess)
  sess.saved_maps = sess.saved_maps or {}
  local key = bufnr .. "\0" .. mode .. "\0" .. lhs
  if sess.saved_maps[key] ~= nil then return end
  local prev
  vim.api.nvim_buf_call(bufnr, function()
    prev = vim.fn.maparg(lhs, mode, false, true)
  end)
  sess.saved_maps[key] = (prev and next(prev)) and prev or false
end

local function restore_keymap(bufnr, mode, lhs, prev)
  pcall(vim.keymap.del, mode, lhs, { buffer = bufnr })
  if prev and prev.buffer == 1 then
    pcall(vim.fn.mapset, mode, false, prev)
  end
end

Then route every keymap set on a real user buffer through set_tab_keymap, fixing three bypass classes:

1. view/keymaps.lua lines 647/650/653/677 — direct vim.keymap.set calls (hunk-staging + ih text-object). Also fixes the mode = "n" hard-code by iterating saved_maps (which carries the actual mode) in clear_tab_keymaps.
2. All of view/conflict/keymaps.lua — direct sets + active pcall(vim.keymap.del, "n", view_keymaps.diff_get, …) (which currently obliterates the user's prior do/dp without saving).
3. Per-buffer restore inside open_in_prev_tab (view/keymaps.lua around line 263): when the user gfs out and close_on_open_in_prev_tab=false, pop and restore the escaped buffer's saved keymaps so the file lives outside the diff with the user's original bindings intact.

Extend the same registry pattern to window-local options (scrollbind, cursorbind, diff) for #254. The escape-on-gf hook is the same.

Items this addresses

Directly closes (refactor itself is the fix)

• [c and ]c don't work after closing #289 — [c and ]c don't work after closing. Comment thread (@ibhagwan, @ruicsh) describes the exact missing save/restore plus the gf-without-close edge case.
• next_hunk keymaps permanently override existing keymaps #334 — next_hunk keymaps permanently override existing keymaps. Strict subset of [c and ]c don't work after closing #289; same root cause.

Likely closes (same root cause, currently masked)

• [Bug] scroll mirroring buffer option persists when opening buffer in new tab #254 — scrollbind persists when buffer opened in new tab via <C-w>d. Same family of lifecycle gap (window-local-option leak); the per-buffer/per-window escape hook this issue forces us to design covers it.
• Wrong highlight in 3-way merge conflict #387 — Wrong highlight in 3-way merge until focus changes. Symptom of out-of-order conflict-keymap setup; routing all conflict-mode keymap setup through the single chokepoint imposes a uniform attach order.

Partial — refactor unblocks but doesn't fully fix

• Integration issues with other major Neovim Git plugins #121 — Integration issues with other major Neovim Git plugins. The "custom regular mappings from other plugins don't work" complaint in that thread is exactly the override-without-save bug; refactor fixes it. Other concerns in Integration issues with other major Neovim Git plugins #121 (oil.nvim git_root, fugitive auto-refresh) are unrelated and belong with [Architecture] Unify path handling: separate repo-relative vs absolute forms in SessionConfig #392.
• gf keybinding open the selected explorer file in the previous tab #322 — gf opens file in wrong tab in explorer. Primary fix is wiring open_in_prev_tab to explorer focus, but the per-buffer restore hook this refactor adds is the right place to layer that wiring.
• feat: allow keymaps to be a single key (string) or multiple (table) #365 (PR) — feat: keymaps as string or table. Co-landing reduces merge conflict — the PR changes the call shape inside set_tab_keymap.

Latent bugs the refactor prevents (no issue filed yet)

• view/keymaps.lua:677 — hunk_textobject ih registered in {o, x} modes is never cleaned up because clear_tab_keymaps:479 hard-codes mode = "n". Even codediff's own mapping leaks.
• conflict/keymaps.lua:31,34 actively dels the user's prior do/dp mappings without saving them — every conflict-mode session permanently breaks any user do/dp mapping.
• Any future bug report about <leader>h* (gitsigns), [h/]h (mini.diff), ]x/[x (vim-unimpaired), etc. — all currently latent.

Suggested PR sequencing

1. Add the save/restore helper (snapshot_keymap, restore_keymap) on set_tab_keymap chokepoint. Behavior-neutral by itself.
2. Migrate the four bypass sites in view/keymaps.lua to route through set_tab_keymap.
3. Migrate all of conflict/keymaps.lua similarly.
4. Fix the mode = "n" hard-code by iterating saved_maps.
5. Add per-buffer restore in open_in_prev_tab (closes the gf-without-close gap).
6. Extend the registry pattern to window-local options (scrollbind, cursorbind); closes [Bug] scroll mirroring buffer option persists when opening buffer in new tab #254.
7. Regression test: a gitsigns-style stub pre-sets [c/]c buffer-local, runs :CodeDiff, closes, asserts vim.fn.maparg(...) post-close matches pre-open. Same shape for <C-w>d / gf escape paths.

Total target: ~150 LOC, 1 day. Single PR is acceptable; the work is one coherent design.

Acceptance criteria

• Zero direct vim.keymap.set / nvim_buf_set_keymap calls on original_bufnr / modified_bufnr outside of set_tab_keymap
• clear_tab_keymaps iterates saved_maps (covers all modes, not just n); ih text-object is correctly torn down
• open_in_prev_tab with close_on_open_in_prev_tab=false leaves the escaped buffer with the user's original mappings (or no buffer-local mapping if they had none)
• Window-local scrollbind / cursorbind are restored on the same lifecycle hooks
• New tests/ui/keymap_lifecycle_spec.lua asserts pre/post symmetry across :tabclose, q, gf, <C-w>d, and :CodeDiff re-invocation paths
• Issues [c and ]c don't work after closing #289, next_hunk keymaps permanently override existing keymaps #334, [Bug] scroll mirroring buffer option persists when opening buffer in new tab #254 close without their own follow-up patches

Cross-cluster items also touched by #392 (path-handling refactor)

#	Path refactor contribution	Keymap refactor contribution
#121	git_root detection for oil.nvim/fugitive	"custom mappings don't work" sub-complaint
#322	which-tab-owns-the-file resolution	per-buffer keymap restore on gf escape
#297	virtual-URI handling	natural place for LSP-detach lifecycle
#254	tab/window ownership tracking	per-window option-restore registry

These four argue that this refactor and #392 should co-land in the same milestone. The shared design (per-buffer/per-window save-restore registry, escape hooks for gf/<C-w>d) means doing them together is cheaper than the sum of the parts.

References

• Full audit: keymap-289 agent transcript (2026-05-16)
• Backlog impact analysis: backlog-vs-keymap agent transcript (2026-05-16) — 2 closed, 2 likely, 3 partial, 80 unrelated
• Sibling architecture issue: [Architecture] Unify path handling: separate repo-relative vs absolute forms in SessionConfig #392 (path-handling)

[esmuellert]

Adding #322 to the consolidated tracking. Root cause: set_tab_keymap in lifecycle/accessors.lua:441-455 runs from setup_all_keymaps BEFORE lifecycle.set_explorer attaches the explorer to the session (view/panel.lua:39), so the explorer.bufnr branch is skipped and gf is never bound on the explorer buffer. Same ordering bug applies to every view keymap that should also work in the explorer (not just gf). The per-session keymap registry pattern in this epic — re-binding tracked keymaps when a new buffer (explorer, history, result) attaches to the session — fixes #322 generically rather than per-keymap.