diff --git a/lua/peekstack/core/location.lua b/lua/peekstack/core/location.lua index 8958daa..60a977b 100644 --- a/lua/peekstack/core/location.lua +++ b/lua/peekstack/core/location.lua @@ -120,13 +120,34 @@ local function resolve_realpath(fname, cache) return resolved end +---Normalize a single position, falling back to {0, 0} for corrupt fields. +---@param pos any +---@return { line: integer, character: integer } +local function normalize_position(pos) + if type(pos) ~= "table" then + return { line = 0, character = 0 } + end + return { + line = type(pos.line) == "number" and pos.line or 0, + character = type(pos.character) == "number" and pos.character or 0, + } +end + +---Normalize a range into a well-formed PeekstackRange. +---This is the validation chokepoint for untrusted data (e.g. ranges read from +---disk): a corrupt shape (non-table range/start/end, non-numeric line/character) +---is coerced to a safe default instead of crashing downstream consumers. ---@param range? PeekstackRange ---@return PeekstackRange local function normalize_range(range) - if not range then + if type(range) ~= "table" then return { start = { line = 0, character = 0 }, ["end"] = { line = 0, character = 0 } } end - return range + local start = normalize_position(range.start) + -- Default a missing/non-table end to start so it stays a coherent zero-width range. + local finish = type(range["end"]) == "table" and normalize_position(range["end"]) + or { line = start.line, character = start.character } + return { start = start, ["end"] = finish } end ---@param loc table diff --git a/lua/peekstack/persist/service.lua b/lua/peekstack/persist/service.lua index 0bf899e..a84e3a8 100644 --- a/lua/peekstack/persist/service.lua +++ b/lua/peekstack/persist/service.lua @@ -7,6 +7,56 @@ local notify = require("peekstack.util.notify") local M = {} +---Validate the coarse shape of a session item read from disk. +---Deeper structural validation (range fields) happens in location.normalize. +---@param item any +---@return boolean +local function is_valid_item(item) + return type(item) == "table" and type(item.uri) == "string" and type(item.range) == "table" +end + +---Restore a single session item into the current stack. +---Records the original->restored popup id mapping so children can re-link to +---their parent. Assumes the item already passed is_valid_item. +---@param item PeekstackSessionItem +---@param id_remap table +---@return boolean restored whether a popup was actually created +local function restore_item(item, id_remap) + local loc = location.normalize({ uri = item.uri, range = item.range }, item.provider or "persist") + if not loc then + return false + end + + local parent_id = item.parent_popup_id + if parent_id then + if id_remap[parent_id] then + parent_id = id_remap[parent_id] + else + -- Parent was not restored (e.g. trimmed by max_items). + -- Drop the stale reference to avoid accidental collisions. + parent_id = nil + end + end + + local model = stack.push(loc, { + title = item.title, + buffer_mode = item.buffer_mode, + parent_popup_id = parent_id, + defer_reflow = true, + }) + if not model then + return false + end + + if item.pinned then + model.pinned = true + end + if item.popup_id then + id_remap[item.popup_id] = model.id + end + return true +end + ---@param success boolean ---@param name string ---@param items PeekstackSessionItem[] @@ -97,47 +147,35 @@ function M.restore(name, opts) ---@type table local id_remap = {} + local restored_count = 0 for _, item in ipairs(session.items) do - local loc = location.normalize({ uri = item.uri, range = item.range }, item.provider or "persist") - if loc then - local parent_id = item.parent_popup_id - if parent_id then - if id_remap[parent_id] then - parent_id = id_remap[parent_id] - else - -- Parent was not restored (e.g. trimmed by max_items). - -- Drop the stale reference to avoid accidental collisions. - parent_id = nil - end - end - local model = stack.push(loc, { - title = item.title, - buffer_mode = item.buffer_mode, - parent_popup_id = parent_id, - defer_reflow = true, - }) - if model then - if item.pinned then - model.pinned = true - end - if item.popup_id then - id_remap[item.popup_id] = model.id - end + -- Isolate each item: a single corrupt entry (bad type or a push failure) + -- must not abort restoring the rest of the session. + if is_valid_item(item) then + local ok, restored = pcall(restore_item, item, id_remap) + if ok and restored then + restored_count = restored_count + 1 end end end stack.reflow() + local skipped = #session.items - restored_count + if not silent then - notify.info("Session restored: " .. resolved_name) + if skipped > 0 then + notify.warn(string.format("Session restored with %d skipped item(s): %s", skipped, resolved_name)) + else + notify.info("Session restored: " .. resolved_name) + end end user_events.emit("PeekstackRestore", { session = resolved_name, - item_count = #session.items, + item_count = restored_count, }) - finish(true) + finish(restored_count > 0) end) end diff --git a/lua/peekstack/ui/inline_preview.lua b/lua/peekstack/ui/inline_preview.lua index ca66bad..bf6c857 100644 --- a/lua/peekstack/ui/inline_preview.lua +++ b/lua/peekstack/ui/inline_preview.lua @@ -3,6 +3,7 @@ local fs = require("peekstack.util.fs") local notify = require("peekstack.util.notify") local NS_NAME = "PeekstackInlinePreviewNS" +local AUGROUP_NAME = "PeekstackInlinePreview" ---@type integer? local namespace_id = nil @@ -45,6 +46,11 @@ function M.close() state = nil end + + -- Disarm any leftover close-event autocmds. When close() is triggered by one + -- of the `once` close events, the remaining events in the group stay armed and + -- would fire on unrelated later activity. Deleting the group clears them. + pcall(vim.api.nvim_del_augroup_by_name, AUGROUP_NAME) end ---Render preview lines from a location @@ -222,7 +228,7 @@ function M.setup_close_events() local cfg = config.get() local close_events = cfg.ui.inline_preview.close_events or { "CursorMoved", "InsertEnter", "BufLeave", "WinLeave" } - local group = vim.api.nvim_create_augroup("PeekstackInlinePreview", { clear = true }) + local group = vim.api.nvim_create_augroup(AUGROUP_NAME, { clear = true }) for _, event in ipairs(close_events) do vim.api.nvim_create_autocmd(event, { diff --git a/tests/inline_preview_spec.lua b/tests/inline_preview_spec.lua index d7a542e..1f4e6e3 100644 --- a/tests/inline_preview_spec.lua +++ b/tests/inline_preview_spec.lua @@ -149,6 +149,30 @@ describe("peekstack.ui.inline_preview", function() assert.equals(1, get_namespaces_calls) end) + it("clears the close-event augroup on close to avoid leaking armed autocmds", function() + local location = { + uri = vim.uri_from_fname(temp_file), + range = { start = { line = 0, character = 0 }, ["end"] = { line = 0, character = 10 } }, + } + + inline_preview.open(location) + + -- Close-event autocmds are registered while the preview is open. + local armed = vim.api.nvim_get_autocmds({ group = "PeekstackInlinePreview" }) + assert.is_true(#armed > 0) + + inline_preview.close() + + -- After close the augroup must be gone so no `once` autocmds stay armed and + -- fire on unrelated later events. Querying a deleted group raises. + local ok, remaining = pcall(vim.api.nvim_get_autocmds, { group = "PeekstackInlinePreview" }) + if ok then + assert.equals(0, #remaining) + else + assert.is_false(ok) + end + end) + it("falls back to default close events when config is invalid", function() config.setup({ ui = { diff --git a/tests/location_spec.lua b/tests/location_spec.lua index 67cd8b9..b129e11 100644 --- a/tests/location_spec.lua +++ b/tests/location_spec.lua @@ -85,6 +85,53 @@ describe("location", function() end) end) + describe("normalize range hardening", function() + it("falls back to a zero range when range is not a table", function() + local result = location.normalize({ uri = "file:///tmp/foo.lua", range = "corrupt" }, "persist") + assert.is_not_nil(result) + assert.same({ line = 0, character = 0 }, result.range.start) + assert.same({ line = 0, character = 0 }, result.range["end"]) + end) + + it("fills in defaults for an empty range table", function() + local result = location.normalize({ uri = "file:///tmp/foo.lua", range = {} }, "persist") + assert.is_not_nil(result) + assert.same({ line = 0, character = 0 }, result.range.start) + assert.same({ line = 0, character = 0 }, result.range["end"]) + end) + + it("coerces non-numeric line and character fields to zero", function() + local result = location.normalize({ + uri = "file:///tmp/foo.lua", + range = { start = { line = "x", character = {} }, ["end"] = "bad" }, + }, "persist") + assert.is_not_nil(result) + assert.same({ line = 0, character = 0 }, result.range.start) + -- A non-table end mirrors start (a zero-width range), not crashes. + assert.same({ line = 0, character = 0 }, result.range["end"]) + end) + + it("defaults a missing end to the start position", function() + local result = location.normalize({ + uri = "file:///tmp/foo.lua", + range = { start = { line = 7, character = 3 } }, + }, "persist") + assert.is_not_nil(result) + assert.same({ line = 7, character = 3 }, result.range.start) + assert.same({ line = 7, character = 3 }, result.range["end"]) + end) + + it("preserves a well-formed range unchanged", function() + local result = location.normalize({ + uri = "file:///tmp/foo.lua", + range = { start = { line = 2, character = 1 }, ["end"] = { line = 4, character = 9 } }, + }, "persist") + assert.is_not_nil(result) + assert.same({ line = 2, character = 1 }, result.range.start) + assert.same({ line = 4, character = 9 }, result.range["end"]) + end) + end) + describe("list_from_lsp", function() it("returns empty table for nil result", function() local items = location.list_from_lsp(nil, "lsp.definition") diff --git a/tests/persist_sessions_spec.lua b/tests/persist_sessions_spec.lua index 2c929b3..fe0b083 100644 --- a/tests/persist_sessions_spec.lua +++ b/tests/persist_sessions_spec.lua @@ -865,6 +865,216 @@ describe("peekstack.persist.sessions", function() end end) + it("should skip corrupt items and still restore valid ones", function() + local original_push = stack.push + local original_reflow = stack.reflow + + write_and_wait(test_scope, { + version = 2, + sessions = { + corrupt_items = { + items = { + { + uri = "file:///tmp/valid_a.lua", + range = { start = { line = 0, character = 0 }, ["end"] = { line = 0, character = 1 } }, + provider = "test", + ts = os.time(), + }, + -- Missing uri: fails the coarse type check. + { + range = { start = { line = 1, character = 0 }, ["end"] = { line = 1, character = 1 } }, + provider = "test", + ts = os.time(), + }, + -- Non-table range: fails the coarse type check. + { + uri = "file:///tmp/corrupt.lua", + range = 42, + provider = "test", + ts = os.time(), + }, + { + uri = "file:///tmp/valid_b.lua", + range = { start = { line = 2, character = 0 }, ["end"] = { line = 2, character = 1 } }, + provider = "test", + ts = os.time(), + }, + }, + meta = { created_at = os.time(), updated_at = os.time() }, + }, + }, + }) + + local pushed = {} + local reflow_calls = 0 + + local ok, err = pcall(function() + stack.push = function(loc, _opts) + table.insert(pushed, loc.uri) + return { id = #pushed } + end + stack.reflow = function() + reflow_calls = reflow_calls + 1 + end + + local restored = nil + persist.restore("corrupt_items", { + silent = true, + on_done = function(result) + restored = result + end, + }) + + local waited = vim.wait(wait_timeout_ms, function() + return restored ~= nil + end, wait_interval_ms) + assert.is_true(waited, "Timed out waiting for restore callback") + + assert.is_true(restored) + assert.equals(2, #pushed) + assert.equals("file:///tmp/valid_a.lua", pushed[1]) + assert.equals("file:///tmp/valid_b.lua", pushed[2]) + assert.equals(1, reflow_calls) + end) + + stack.push = original_push + stack.reflow = original_reflow + + if not ok then + error(err) + end + end) + + it("should isolate a push failure to a single item when restoring", function() + local original_push = stack.push + local original_reflow = stack.reflow + + write_and_wait(test_scope, { + version = 2, + sessions = { + push_failure = { + items = { + { + uri = "file:///tmp/ok1.lua", + range = { start = { line = 0, character = 0 }, ["end"] = { line = 0, character = 1 } }, + provider = "test", + ts = os.time(), + }, + { + uri = "file:///tmp/boom.lua", + range = { start = { line = 1, character = 0 }, ["end"] = { line = 1, character = 1 } }, + provider = "test", + ts = os.time(), + }, + { + uri = "file:///tmp/ok2.lua", + range = { start = { line = 2, character = 0 }, ["end"] = { line = 2, character = 1 } }, + provider = "test", + ts = os.time(), + }, + }, + meta = { created_at = os.time(), updated_at = os.time() }, + }, + }, + }) + + local pushed = {} + local reflow_calls = 0 + + local ok, err = pcall(function() + stack.push = function(loc, _opts) + if loc.uri == "file:///tmp/boom.lua" then + error("simulated push failure") + end + table.insert(pushed, loc.uri) + return { id = #pushed } + end + stack.reflow = function() + reflow_calls = reflow_calls + 1 + end + + local restored = nil + persist.restore("push_failure", { + silent = true, + on_done = function(result) + restored = result + end, + }) + + local waited = vim.wait(wait_timeout_ms, function() + return restored ~= nil + end, wait_interval_ms) + assert.is_true(waited, "Timed out waiting for restore callback") + + -- The failing item is isolated; the surrounding valid items still restore. + assert.is_true(restored) + assert.equals(2, #pushed) + assert.equals("file:///tmp/ok1.lua", pushed[1]) + assert.equals("file:///tmp/ok2.lua", pushed[2]) + assert.equals(1, reflow_calls) + end) + + stack.push = original_push + stack.reflow = original_reflow + + if not ok then + error(err) + end + end) + + it("should report restore as unsuccessful when no item could be restored", function() + local original_push = stack.push + local original_reflow = stack.reflow + + write_and_wait(test_scope, { + version = 2, + sessions = { + all_fail = { + items = { + { + uri = "file:///tmp/unrestorable.lua", + range = { start = { line = 0, character = 0 }, ["end"] = { line = 0, character = 1 } }, + provider = "test", + ts = os.time(), + }, + }, + meta = { created_at = os.time(), updated_at = os.time() }, + }, + }, + }) + + local ok, err = pcall(function() + -- A nil return means the popup was not actually created. + stack.push = function() + return nil + end + stack.reflow = function() end + + local restored = nil + persist.restore("all_fail", { + silent = true, + on_done = function(result) + restored = result + end, + }) + + local waited = vim.wait(wait_timeout_ms, function() + return restored ~= nil + end, wait_interval_ms) + assert.is_true(waited, "Timed out waiting for restore callback") + + -- Session existed but nothing materialized, so restore reports false. + assert.is_false(restored) + end) + + stack.push = original_push + stack.reflow = original_reflow + + if not ok then + error(err) + end + end) + it("should save the root stack when stack view is active", function() local stack_view = require("peekstack.ui.stack_view")