Merge pull request #14 from mhiro2/fix/prevent-keymap-leakage

fix(ui): Prevent keymap leakage and harden persist/config reliability

Author: mhiro2

README.md

@@ -62,15 +62,15 @@ Using lazy.nvim:
     })
 
     -- LSP: peek at definitions and references
-    vim.keymap.set("n", "pd", function() peekstack.peek.definition() end)
-    vim.keymap.set("n", "pr", function() peekstack.peek.references() end)
+    vim.keymap.set("n", "<leader>pd", function() peekstack.peek.definition() end)
+    vim.keymap.set("n", "<leader>pr", function() peekstack.peek.references() end)
 
     -- Diagnostics & files: peek at diagnostics or files under cursor
-    vim.keymap.set("n", "pl", function() peekstack.peek.diagnostics_cursor() end)
-    vim.keymap.set("n", "pf", function() peekstack.peek.file_under_cursor() end)
+    vim.keymap.set("n", "<leader>pl", function() peekstack.peek.diagnostics_cursor() end)
+    vim.keymap.set("n", "<leader>pf", function() peekstack.peek.file_under_cursor() end)
 
     -- Marks: browse buffer marks (requires marks provider enabled)
-    vim.keymap.set("n", "pm", function() peekstack.peek.marks_buffer() end)
+    vim.keymap.set("n", "<leader>pm", function() peekstack.peek.marks_buffer() end)
   end,
 }
 ```

lua/peekstack/config.lua

@@ -199,17 +199,35 @@ local config = vim.deepcopy(M.defaults)
 
 ---@param path string
 ---@param value any
-local function validate_event_list(path, value)
+---@param default string[]
+---@return string[]
+local function sanitize_event_list(path, value, default)
   if type(value) ~= "table" then
-    notify.warn(string.format("%s must be a list of strings", path))
-    return
+    notify.warn(string.format("%s must be a list of strings. Falling back to defaults", path))
+    return vim.deepcopy(default)
   end
-  for idx, event in ipairs(value) do
-    if type(event) ~= "string" then
-      notify.warn(string.format("%s[%d] must be a string, got %s", path, idx, type(event)))
-      return
+
+  ---@type string[]
+  local events = {}
+  local invalid_count = 0
+  for _, event in ipairs(value) do
+    if type(event) == "string" and event ~= "" then
+      events[#events + 1] = event
+    else
+      invalid_count = invalid_count + 1
     end
   end
+
+  if invalid_count > 0 then
+    notify.warn(string.format("%s contains %d invalid entries. Ignoring invalid values", path, invalid_count))
+  end
+
+  if #events == 0 then
+    notify.warn(string.format("%s must contain at least one valid event. Falling back to defaults", path))
+    return vim.deepcopy(default)
+  end
+
+  return events
 end
 
 ---@alias PeekstackConfigFieldValidator fun(path: string, value: any, default: any): any
@@ -268,9 +286,8 @@ end
 
 ---@return PeekstackConfigFieldValidator
 local function field_event_list()
-  return function(path, value, _default)
-    validate_event_list(path, value)
-    return value
+  return function(path, value, default)
+    return sanitize_event_list(path, value, default)
   end
 end
 

lua/peekstack/persist/auto.lua

@@ -54,8 +54,9 @@ local function resolve_root_winid()
 end
 
 ---@param root_winid? integer
+---@param opts? { sync?: boolean }
 ---@return boolean
-local function save_session(root_winid)
+local function save_session(root_winid, opts)
   if not is_enabled() then
     return false
   end
@@ -66,6 +67,7 @@ local function save_session(root_winid)
     scope = "repo",
     root_winid = normalize_root_winid(root_winid),
     silent = true,
+    sync = opts and opts.sync or false,
   })
   return true
 end
@@ -162,7 +164,7 @@ function M.save_on_leave(opts)
     save_timer:stop()
   end
 
-  return save_session(root_winid)
+  return save_session(root_winid, { sync = true })
 end
 
 function M.setup()

lua/peekstack/persist/init.lua

@@ -66,9 +66,10 @@ end
 
 ---Save the current stack to persistent storage with optional name
 ---@param name? string
----@param opts? { scope?: string, root_winid?: integer, silent?: boolean, on_done?: fun(success: boolean) }
+---@param opts? { scope?: string, root_winid?: integer, silent?: boolean, sync?: boolean, on_done?: fun(success: boolean) }
 function M.save_current(name, opts)
   local silent = opts and opts.silent or false
+  local sync = opts and opts.sync or false
   local on_done = opts and opts.on_done or nil
   local function finish(success)
     if on_done then
@@ -77,6 +78,7 @@ function M.save_current(name, opts)
   end
 
   if not ensure_enabled(silent) then
+    finish(false)
     return
   end
 
@@ -100,42 +102,62 @@ function M.save_current(name, opts)
     data_items = vim.list_slice(data_items, #data_items - max_items + 1, #data_items)
   end
 
-  store.read(scope, {
-    on_done = function(read_data)
-      local data = migrate.ensure(read_data)
-      local now = os.time()
+  ---@param data PeekstackStoreData
+  ---@return PeekstackStoreData
+  local function upsert_session(data)
+    local now = os.time()
+    if data.sessions[resolved_name] then
+      data.sessions[resolved_name].items = data_items
+      data.sessions[resolved_name].meta.updated_at = now
+    else
+      data.sessions[resolved_name] = {
+        items = data_items,
+        meta = {
+          created_at = now,
+          updated_at = now,
+        },
+      }
+    end
+    return data
+  end
 
-      if data.sessions[resolved_name] then
-        data.sessions[resolved_name].items = data_items
-        data.sessions[resolved_name].meta.updated_at = now
+  local function notify_save_result(success)
+    if not silent then
+      if success then
+        vim.notify("Session saved: " .. resolved_name, vim.log.levels.INFO)
       else
-        data.sessions[resolved_name] = {
-          items = data_items,
-          meta = {
-            created_at = now,
-            updated_at = now,
-          },
-        }
+        vim.notify("Failed to save session: " .. resolved_name, vim.log.levels.WARN)
       end
+    end
+    if success then
+      user_events.emit("PeekstackSave", {
+        session = resolved_name,
+        item_count = #data_items,
+      })
+    end
+  end
+
+  if sync then
+    local data = upsert_session(migrate.ensure(store.read_sync(scope)))
+    local success = store.write_sync(scope, data)
+    if success then
+      update_cache(data)
+    end
+    notify_save_result(success)
+    finish(success)
+    return
+  end
+
+  store.read(scope, {
+    on_done = function(read_data)
+      local data = upsert_session(migrate.ensure(read_data))
 
       store.write(scope, data, {
         on_done = function(success)
           if success then
             update_cache(data)
           end
-          if not silent then
-            if success then
-              vim.notify("Session saved: " .. resolved_name, vim.log.levels.INFO)
-            else
-              vim.notify("Failed to save session: " .. resolved_name, vim.log.levels.WARN)
-            end
-          end
-          if success then
-            user_events.emit("PeekstackSave", {
-              session = resolved_name,
-              item_count = #data_items,
-            })
-          end
+          notify_save_result(success)
           finish(success)
         end,
       })
@@ -156,6 +178,7 @@ function M.restore(name, opts)
   end
 
   if not ensure_enabled(silent) then
+    finish(false)
     return
   end
 

lua/peekstack/persist/store.lua

@@ -7,6 +7,34 @@ local function empty_data()
   return { version = 2, sessions = {} }
 end
 
+---@param data PeekstackStoreData
+---@return string?
+local function encode_data(data)
+  local ok, encoded = pcall(vim.json.encode, data)
+  if not ok then
+    vim.notify("Failed to encode session data", vim.log.levels.WARN)
+    return nil
+  end
+  return encoded
+end
+
+---@param path string
+---@return boolean
+local function ensure_parent_dir(path)
+  local dir = vim.fs.dirname(path)
+  if vim.uv.fs_stat(dir) then
+    return true
+  end
+
+  local mkdir_ok = pcall(vim.fn.mkdir, dir, "p")
+  if not mkdir_ok then
+    vim.notify("Failed to create directory: " .. dir, vim.log.levels.WARN)
+    return false
+  end
+
+  return true
+end
+
 ---@param scope string
 ---@param opts { on_done: fun(data: PeekstackStoreData) }
 function M.read(scope, opts)
@@ -94,22 +122,17 @@ function M.write(scope, data, opts)
   end
 
   local path = fs.scope_path(scope)
-  local ok, encoded = pcall(vim.json.encode, data)
-  if not ok then
-    vim.notify("Failed to encode session data", vim.log.levels.WARN)
+  local encoded = encode_data(data)
+  if not encoded then
     finish(false)
     return
   end
-  local dir = vim.fs.dirname(path)
-  local dir_stat = vim.uv.fs_stat(dir)
-  if not dir_stat then
-    local mkdir_ok = pcall(vim.fn.mkdir, dir, "p")
-    if not mkdir_ok then
-      vim.notify("Failed to create directory: " .. dir, vim.log.levels.WARN)
-      finish(false)
-      return
-    end
+
+  if not ensure_parent_dir(path) then
+    finish(false)
+    return
   end
+
   local tmp_path = path .. ".tmp"
   vim.uv.fs_open(tmp_path, "w", 438, function(open_err, fd)
     if open_err or not fd then
@@ -145,4 +168,43 @@ function M.write(scope, data, opts)
   end)
 end
 
+---@param scope string
+---@param data PeekstackStoreData
+---@return boolean
+function M.write_sync(scope, data)
+  local path = fs.scope_path(scope)
+  local encoded = encode_data(data)
+  if not encoded then
+    return false
+  end
+
+  if not ensure_parent_dir(path) then
+    return false
+  end
+
+  local tmp_path = path .. ".tmp"
+  local fd = vim.uv.fs_open(tmp_path, "w", 438)
+  if not fd then
+    vim.notify("Failed to write session data: " .. path, vim.log.levels.WARN)
+    return false
+  end
+
+  local write_ok = vim.uv.fs_write(fd, encoded, 0)
+  pcall(vim.uv.fs_close, fd)
+  if not write_ok then
+    vim.notify("Failed to write session data: " .. path, vim.log.levels.WARN)
+    pcall(vim.uv.fs_unlink, tmp_path)
+    return false
+  end
+
+  local rename_ok = vim.uv.fs_rename(tmp_path, path)
+  if not rename_ok then
+    vim.notify("Failed to write session data: " .. path, vim.log.levels.WARN)
+    pcall(vim.uv.fs_unlink, tmp_path)
+    return false
+  end
+
+  return true
+end
+
 return M

lua/peekstack/ui/keymaps.lua

@@ -26,6 +26,12 @@ end
 
 ---@param popup table
 function M.apply_popup(popup)
+  if popup.buffer_mode == "source" then
+    -- Source mode uses the real editing buffer, so buffer-local mappings
+    -- would leak into normal editing after popup close.
+    return
+  end
+
   local keys = config.get().ui.keys
   local popup_id = popup.id
 

tests/config_spec.lua

@@ -204,17 +204,45 @@ describe("config", function()
       assert.equals(config.defaults.ui.inline_preview.enabled, cfg.ui.inline_preview.enabled)
       assert.equals(config.defaults.ui.inline_preview.max_lines, cfg.ui.inline_preview.max_lines)
       assert.equals(config.defaults.ui.inline_preview.hl_group, cfg.ui.inline_preview.hl_group)
+      assert.same(config.defaults.ui.inline_preview.close_events, cfg.ui.inline_preview.close_events)
     end)
 
     it("warns on invalid quick peek config", function()
-      config.setup({
+      local cfg = config.setup({
         ui = {
           quick_peek = {
             close_events = { 1, 2 },
           },
         },
       })
       assert.is_true(has_message("ui.quick_peek.close_events"))
+      assert.same(config.defaults.ui.quick_peek.close_events, cfg.ui.quick_peek.close_events)
+    end)
+
+    it("sanitizes mixed quick peek close_events values", function()
+      local cfg = config.setup({
+        ui = {
+          quick_peek = {
+            close_events = { "CursorMoved", 1, "", "InsertEnter" },
+          },
+        },
+      })
+
+      assert.is_true(has_message("ui.quick_peek.close_events"))
+      assert.same({ "CursorMoved", "InsertEnter" }, cfg.ui.quick_peek.close_events)
+    end)
+
+    it("falls back when quick peek close_events is empty", function()
+      local cfg = config.setup({
+        ui = {
+          quick_peek = {
+            close_events = {},
+          },
+        },
+      })
+
+      assert.is_true(has_message("ui.quick_peek.close_events"))
+      assert.same(config.defaults.ui.quick_peek.close_events, cfg.ui.quick_peek.close_events)
     end)
 
     it("warns on invalid path config", function()