Merge pull request #26 from mhiro2/refactor/root-resolution-and-validation-hardening

fix(persist): preserve popup hierarchy across restore and harden config validation

Author: mhiro2

lua/peekstack/config/validate/rules/persist.lua

@@ -4,6 +4,7 @@ local M = {}
 
 ---@type PeekstackConfigFieldRule[]
 local PERSIST_RULES = {
+  { key = "enabled", validate = shared.field_type("boolean") },
   { key = "max_items", validate = shared.field_number_range({ min = 1 }) },
 }
 
@@ -34,9 +35,11 @@ function M.validate(cfg, defaults)
 
   shared.apply_rules(persist, "persist", defaults.persist, PERSIST_RULES)
 
-  local session = shared.as_table(persist.session)
-  if session then
-    shared.apply_rules(session, "persist.session", defaults.persist.session, PERSIST_SESSION_RULES)
+  if persist.session ~= nil then
+    local session = shared.ensure_table_field(persist, "session", "persist.session", defaults.persist.session)
+    if session then
+      shared.apply_rules(session, "persist.session", defaults.persist.session, PERSIST_SESSION_RULES)
+    end
   end
 
   if persist.auto ~= nil then

lua/peekstack/config/validate/rules/providers.lua

@@ -2,8 +2,14 @@ local shared = require("peekstack.config.validate.shared")
 
 local M = {}
 
+---@type PeekstackConfigFieldRule[]
+local PROVIDER_ENABLE_RULES = {
+  { key = "enable", validate = shared.field_type("boolean") },
+}
+
 ---@type PeekstackConfigFieldRule[]
 local MARKS_RULES = {
+  { key = "enable", validate = shared.field_type("boolean") },
   { key = "include", validate = shared.field_type("string") },
   { key = "include_special", validate = shared.field_type("boolean") },
 }
@@ -16,9 +22,20 @@ function M.validate(cfg, defaults)
     return
   end
 
-  local marks = shared.as_table(providers.marks)
-  if marks then
-    shared.apply_rules(marks, "providers.marks", defaults.providers.marks, MARKS_RULES)
+  for _, name in ipairs({ "lsp", "diagnostics", "file" }) do
+    if providers[name] ~= nil then
+      local provider = shared.ensure_table_field(providers, name, "providers." .. name, defaults.providers[name])
+      if provider then
+        shared.apply_rules(provider, "providers." .. name, defaults.providers[name], PROVIDER_ENABLE_RULES)
+      end
+    end
+  end
+
+  if providers.marks ~= nil then
+    local marks = shared.ensure_table_field(providers, "marks", "providers.marks", defaults.providers.marks)
+    if marks then
+      shared.apply_rules(marks, "providers.marks", defaults.providers.marks, MARKS_RULES)
+    end
   end
 end
 

lua/peekstack/config/validate/rules/ui.lua

@@ -60,6 +60,37 @@ local QUICK_PEEK_RULES = {
   { key = "close_events", validate = shared.field_event_list() },
 }
 
+---@type PeekstackConfigFieldRule[]
+local POPUP_AUTO_CLOSE_RULES = {
+  { key = "enabled", validate = shared.field_type("boolean") },
+  { key = "idle_ms", validate = shared.field_number_range({ min = 1 }) },
+  { key = "check_interval_ms", validate = shared.field_number_range({ min = 1 }) },
+  { key = "ignore_pinned", validate = shared.field_type("boolean") },
+}
+
+---@type PeekstackConfigFieldRule[]
+local FEEDBACK_RULES = {
+  { key = "highlight_origin_on_close", validate = shared.field_type("boolean") },
+}
+
+---@type PeekstackConfigFieldRule[]
+local PROMOTE_RULES = {
+  { key = "close_popup", validate = shared.field_type("boolean") },
+}
+
+---@type PeekstackConfigFieldRule[]
+local TITLE_RULES = {
+  { key = "enabled", validate = shared.field_type("boolean") },
+  { key = "format", validate = shared.field_type("string") },
+}
+
+---@type PeekstackConfigFieldRule[]
+local TITLE_CONTEXT_RULES = {
+  { key = "enabled", validate = shared.field_type("boolean") },
+  { key = "max_depth", validate = shared.field_number_range({ min = 1 }) },
+  { key = "separator", validate = shared.field_type("string") },
+}
+
 ---@type PeekstackConfigFieldRule[]
 local TITLE_ICON_RULES = {
   { key = "enabled", validate = shared.field_type("boolean") },
@@ -119,28 +150,45 @@ end
 ---@param ui table
 ---@param defaults PeekstackConfigUI
 local function validate_popup(ui, defaults)
-  local popup = shared.as_table(ui.popup)
+  if ui.popup == nil then
+    return
+  end
+  local popup = shared.ensure_table_field(ui, "popup", "ui.popup", defaults.popup)
   if not popup then
     return
   end
 
   shared.apply_rules(popup, "ui.popup", defaults.popup, POPUP_RULES)
 
-  local source = shared.as_table(popup.source)
-  if source then
-    shared.apply_rules(source, "ui.popup.source", defaults.popup.source, POPUP_SOURCE_RULES)
+  if popup.source ~= nil then
+    local source = shared.ensure_table_field(popup, "source", "ui.popup.source", defaults.popup.source)
+    if source then
+      shared.apply_rules(source, "ui.popup.source", defaults.popup.source, POPUP_SOURCE_RULES)
+    end
   end
 
-  local history = shared.as_table(popup.history)
-  if history then
-    shared.apply_rules(history, "ui.popup.history", defaults.popup.history, POPUP_HISTORY_RULES)
+  if popup.history ~= nil then
+    local history = shared.ensure_table_field(popup, "history", "ui.popup.history", defaults.popup.history)
+    if history then
+      shared.apply_rules(history, "ui.popup.history", defaults.popup.history, POPUP_HISTORY_RULES)
+    end
+  end
+
+  if popup.auto_close ~= nil then
+    local auto_close = shared.ensure_table_field(popup, "auto_close", "ui.popup.auto_close", defaults.popup.auto_close)
+    if auto_close then
+      shared.apply_rules(auto_close, "ui.popup.auto_close", defaults.popup.auto_close, POPUP_AUTO_CLOSE_RULES)
+    end
   end
 end
 
 ---@param ui table
 ---@param defaults PeekstackConfigUI
 local function validate_path(ui, defaults)
-  local path = shared.as_table(ui.path)
+  if ui.path == nil then
+    return
+  end
+  local path = shared.ensure_table_field(ui, "path", "ui.path", defaults.path)
   if path then
     shared.apply_rules(path, "ui.path", defaults.path, UI_PATH_RULES)
   end
@@ -162,25 +210,34 @@ end
 ---@param ui table
 ---@param defaults PeekstackConfigUI
 local function validate_preview(ui, defaults)
-  local inline_preview = shared.as_table(ui.inline_preview)
-  if inline_preview then
-    shared.apply_rules(inline_preview, "ui.inline_preview", defaults.inline_preview, INLINE_PREVIEW_RULES)
+  if ui.inline_preview ~= nil then
+    local inline_preview = shared.ensure_table_field(ui, "inline_preview", "ui.inline_preview", defaults.inline_preview)
+    if inline_preview then
+      shared.apply_rules(inline_preview, "ui.inline_preview", defaults.inline_preview, INLINE_PREVIEW_RULES)
+    end
   end
 
-  local quick_peek = shared.as_table(ui.quick_peek)
-  if quick_peek then
-    shared.apply_rules(quick_peek, "ui.quick_peek", defaults.quick_peek, QUICK_PEEK_RULES)
+  if ui.quick_peek ~= nil then
+    local quick_peek = shared.ensure_table_field(ui, "quick_peek", "ui.quick_peek", defaults.quick_peek)
+    if quick_peek then
+      shared.apply_rules(quick_peek, "ui.quick_peek", defaults.quick_peek, QUICK_PEEK_RULES)
+    end
   end
 end
 
 ---@param ui table
 ---@param defaults PeekstackConfigTitle
 local function validate_title(ui, defaults)
-  local title = shared.as_table(ui.title)
+  if ui.title == nil then
+    return
+  end
+  local title = shared.ensure_table_field(ui, "title", "ui.title", defaults)
   if not title then
     return
   end
 
+  shared.apply_rules(title, "ui.title", defaults, TITLE_RULES)
+
   if title.icons ~= nil and type(title.icons) ~= "table" then
     notify.warn("ui.title.icons must be a table, got " .. type(title.icons) .. ". Falling back to defaults")
     title.icons = vim.deepcopy(defaults.icons)
@@ -195,6 +252,13 @@ local function validate_title(ui, defaults)
       icons.map = vim.deepcopy(defaults.icons.map)
     end
   end
+
+  if title.context ~= nil then
+    local context = shared.ensure_table_field(title, "context", "ui.title.context", defaults.context)
+    if context then
+      shared.apply_rules(context, "ui.title.context", defaults.context, TITLE_CONTEXT_RULES)
+    end
+  end
 end
 
 ---@param ui table
@@ -248,6 +312,20 @@ function M.validate(cfg, defaults)
   validate_preview(ui, defaults.ui)
   validate_title(ui, defaults.ui.title)
   validate_layout(ui, defaults.ui)
+
+  if ui.feedback ~= nil then
+    local feedback = shared.ensure_table_field(ui, "feedback", "ui.feedback", defaults.ui.feedback)
+    if feedback then
+      shared.apply_rules(feedback, "ui.feedback", defaults.ui.feedback, FEEDBACK_RULES)
+    end
+  end
+
+  if ui.promote ~= nil then
+    local promote = shared.ensure_table_field(ui, "promote", "ui.promote", defaults.ui.promote)
+    if promote then
+      shared.apply_rules(promote, "ui.promote", defaults.ui.promote, PROMOTE_RULES)
+    end
+  end
 end
 
 return M

lua/peekstack/core/history.lua

@@ -19,6 +19,7 @@ end
 ---@return PeekstackHistoryEntry
 function M.build_entry(item, idx)
   return {
+    popup_id = item.id,
     location = item.location,
     title = item.title,
     title_chunks = item.title_chunks,
@@ -52,16 +53,77 @@ local function emit_popup_event(event, popup_model, root_winid)
   user_events.emit(event, user_events.build_popup_data(popup_model, root_winid))
 end
 
+---Check whether a popup with the given id exists in the stack.
+---@param stack PeekstackStackModel
+---@param popup_id integer
+---@return boolean
+local function popup_exists_in_stack(stack, popup_id)
+  for _, p in ipairs(stack.popups) do
+    if p.id == popup_id then
+      return true
+    end
+  end
+  return false
+end
+
+---Resolve parent_popup_id for a history entry being restored.
+---Uses id_remap (for bulk restore) and verifies the target exists.
+---@param stack PeekstackStackModel
+---@param entry PeekstackHistoryEntry
+---@param id_remap? table<integer, integer>
+---@return integer?
+local function resolve_parent_id(stack, entry, id_remap)
+  local parent_id = entry.parent_popup_id
+  if not parent_id then
+    return nil
+  end
+  if id_remap and id_remap[parent_id] then
+    parent_id = id_remap[parent_id]
+  end
+  if popup_exists_in_stack(stack, parent_id) then
+    return parent_id
+  end
+  return nil
+end
+
+---Return the stack-level id remap table, creating it if needed.
+---@param stack PeekstackStackModel
+---@return table<integer, integer>
+local function ensure_id_remap(stack)
+  if not stack._id_remap then
+    stack._id_remap = {}
+  end
+  return stack._id_remap
+end
+
+---Record the mapping from old popup_id to new model id.
+---@param stack PeekstackStackModel
+---@param entry PeekstackHistoryEntry
+---@param model PeekstackPopupModel
+local function record_remap(stack, entry, model)
+  if entry.popup_id then
+    ensure_id_remap(stack)[entry.popup_id] = model.id
+  end
+end
+
 ---Restore a history entry into the stack.
 ---@param stack PeekstackStackModel
 ---@param entry PeekstackHistoryEntry
+---@param id_remap? table<integer, integer>  extra old popup id -> new popup id mapping
 ---@return PeekstackPopupModel?
-function M.restore_entry(stack, entry)
+function M.restore_entry(stack, entry, id_remap)
   deps()
+  -- Merge stack-level remap with any caller-provided remap.
+  local merged = ensure_id_remap(stack)
+  if id_remap then
+    for k, v in pairs(id_remap) do
+      merged[k] = v
+    end
+  end
   local create_opts = {
     buffer_mode = entry.buffer_mode or "copy",
     origin_winid = stack.root_winid,
-    parent_popup_id = entry.parent_popup_id,
+    parent_popup_id = resolve_parent_id(stack, entry, merged),
   }
   -- Only pass title override for user-renamed popups (no title_chunks).
   -- Auto-generated titles are regenerated by build_title() to preserve
@@ -112,6 +174,7 @@ function M.restore_last(stack)
   if not restored then
     return nil
   end
+  record_remap(stack, entry, restored)
   table.remove(stack.history)
   return restored
 end
@@ -122,10 +185,15 @@ end
 function M.restore_all(stack)
   local restored = {}
   local remaining = {}
+  ---@type table<integer, integer>
+  local id_remap = {}
   while #stack.history > 0 do
     local entry = table.remove(stack.history)
-    local model = M.restore_entry(stack, entry)
-    if model then
+    local model = M.restore_entry(stack, entry, id_remap)
+    if model and entry.popup_id then
+      id_remap[entry.popup_id] = model.id
+      table.insert(restored, model)
+    elseif model then
       table.insert(restored, model)
     else
       table.insert(remaining, 1, entry)
@@ -148,6 +216,7 @@ function M.restore_from_history(stack, idx)
   if not restored then
     return nil
   end
+  record_remap(stack, entry, restored)
   table.remove(stack.history, idx)
   return restored
 end
@@ -163,6 +232,7 @@ end
 ---@param stack PeekstackStackModel
 function M.clear(stack)
   stack.history = {}
+  stack._id_remap = nil
 end
 
 return M