From cd42383de789d28946e3c2495f1727a057daa251 Mon Sep 17 00:00:00 2001 From: Masaaki Hirotsu Date: Sat, 3 Oct 2026 07:00:37 +0900 Subject: [PATCH 1/3] test(user_events): run session event tests in a throwaway repository The save/restore event tests wrote to the same repository store as persist_sessions_spec, which runs in a parallel process, so an async save here could overwrite fixtures there with a stale snapshot. Each test now runs from its own temporary git repository and removes its store file. --- tests/user_events_spec.lua | 33 ++++++++++++++++++++++++++++----- 1 file changed, 28 insertions(+), 5 deletions(-) diff --git a/tests/user_events_spec.lua b/tests/user_events_spec.lua index 16c86e9..ba8ac1d 100644 --- a/tests/user_events_spec.lua +++ b/tests/user_events_spec.lua @@ -6,6 +6,25 @@ describe("peekstack.core.user_events", function() -- Track received events local received_events = {} + ---Run `fn` from a throwaway git repository so session writes do not touch + ---the store shared with other specs running in parallel processes. + ---@param fn fun() + local function in_temp_repo(fn) + local fs = require("peekstack.util.fs") + local original_cwd = vim.fn.getcwd() + local dir = vim.fn.tempname() + vim.fn.mkdir(dir .. "/.git", "p") + vim.cmd.cd(dir) + local store_path = fs.scope_path("repo") + local ok, err = pcall(fn) + vim.cmd.cd(original_cwd) + pcall(vim.fn.delete, store_path) + pcall(vim.fn.delete, dir, "rf") + if not ok then + error(err, 0) + end + end + ---@param name string ---@return boolean local function wait_for_event(name) @@ -185,10 +204,12 @@ describe("peekstack.core.user_events", function() local persist = require("peekstack.persist") config.setup({ persist = { enabled = true } }) - persist.save_current("test_session") + in_temp_repo(function() + persist.save_current("test_session") - local ok = wait_for_event("PeekstackSave") - assert.is_true(ok, "Timed out waiting for PeekstackSave event") + local ok = wait_for_event("PeekstackSave") + assert.is_true(ok, "Timed out waiting for PeekstackSave event") + end) local found = false for _, ev in ipairs(received_events) do @@ -205,9 +226,11 @@ describe("peekstack.core.user_events", function() local persist = require("peekstack.persist") config.setup({ persist = { enabled = true } }) - persist.restore("test_session") + in_temp_repo(function() + persist.restore("test_session") - vim.wait(100) + vim.wait(100) + end) local _found = false for _, ev in ipairs(received_events) do From 894ebebd4769c8034b714b64ced203d56037dc8e Mon Sep 17 00:00:00 2001 From: Masaaki Hirotsu Date: Sat, 3 Oct 2026 07:00:48 +0900 Subject: [PATCH 2/3] fix(persist): bind each session operation to the store it was requested in The store resolved its file from the current directory on every read and write, so starting an async save in repository A and running :cd B before it finished wrote A's sessions over B's store and dropped B's sessions. - Resolve the store file once when a save, restore, list, delete or rename is requested and carry it through the update queue, read, write and cache refresh; the store module now takes a file path instead of a scope. - Key the session cache by store file so a listing after a directory change no longer returns the previous repository's sessions. - Cover save, delete, rename and queued updates across a directory change with real store files in two temporary repositories. --- README.md | 2 + doc/peekstack.txt | 4 +- lua/peekstack/persist/cache.lua | 22 ++-- lua/peekstack/persist/orchestrator.lua | 94 +++++++++------- lua/peekstack/persist/service.lua | 24 ++-- lua/peekstack/persist/store.lua | 21 ++-- tests/persist_sessions_spec.lua | 13 ++- tests/persist_store_path_spec.lua | 145 +++++++++++++++++++++++++ tests/persist_store_spec.lua | 55 +++++----- 9 files changed, 272 insertions(+), 108 deletions(-) create mode 100644 tests/persist_store_path_spec.lua diff --git a/README.md b/README.md index a86f573..3e4dfeb 100644 --- a/README.md +++ b/README.md @@ -354,6 +354,8 @@ for a name instead of using the default. > [!WARNING] > Persistence uses repository storage when the current working directory is inside a git repository. > Outside a git repository, sessions fall back to cwd-based storage. +> The storage is resolved when a save, delete, or rename is requested, so changing directory while +> it is still being written does not move it to another repository. > [!IMPORTANT] > Sessions are written as plain JSON under `vim.fn.stdpath("state") .. "/peekstack/"`. Each entry diff --git a/doc/peekstack.txt b/doc/peekstack.txt index 084368b..375884f 100644 --- a/doc/peekstack.txt +++ b/doc/peekstack.txt @@ -452,7 +452,9 @@ Persistence is disabled by default. Enable with `persist.enabled = true`. Persistence uses repository storage when the current working directory is inside a git repository. Outside a git repository, it falls back to -cwd-based storage. +cwd-based storage. The storage is resolved when a save, delete, or rename +is requested, so changing directory while it is still being written does +not move it to another repository. `persist.session.default_name` is used when a name is omitted, unless `persist.session.prompt_if_missing` is true. diff --git a/lua/peekstack/persist/cache.lua b/lua/peekstack/persist/cache.lua index 31e438a..aa2ca97 100644 --- a/lua/peekstack/persist/cache.lua +++ b/lua/peekstack/persist/cache.lua @@ -1,30 +1,32 @@ local M = {} ----@type table +---Sessions keyed by the store file they were read from, so a cached snapshot +---of one repository is never served for another after a directory change. +---@type table> local cached_sessions = {} -local cache_loaded = false +---@param path string ---@param data PeekstackStoreData ---@return PeekstackStoreData -function M.update(data) - cached_sessions = data.sessions or {} - cache_loaded = true +function M.update(path, data) + cached_sessions[path] = data.sessions or {} return data end +---@param path string ---@return table -function M.get() - return cached_sessions +function M.get(path) + return cached_sessions[path] or {} end +---@param path string ---@return boolean -function M.is_loaded() - return cache_loaded +function M.is_loaded(path) + return cached_sessions[path] ~= nil end function M.reset() cached_sessions = {} - cache_loaded = false end return M diff --git a/lua/peekstack/persist/orchestrator.lua b/lua/peekstack/persist/orchestrator.lua index e7704a6..f820cca 100644 --- a/lua/peekstack/persist/orchestrator.lua +++ b/lua/peekstack/persist/orchestrator.lua @@ -1,4 +1,5 @@ local config = require("peekstack.config") +local fs = require("peekstack.util.fs") local cache = require("peekstack.persist.cache") local migrate = require("peekstack.persist.migrate") local store = require("peekstack.persist.store") @@ -8,9 +9,13 @@ local M = {} local SCOPE = "repo" +---Resolve the store file for the current working directory. +---Every operation resolves it once when it is accepted and carries it through +---its read, write and cache update, so a `:cd` while the operation is in +---flight cannot redirect it to another repository's store. ---@return string -function M.scope() - return SCOPE +function M.store_path() + return fs.scope_path(SCOPE) end ---@param data PeekstackStoreData @@ -36,9 +41,10 @@ end ---Does NOT touch the cache; callers that want to refresh it should use ---`refresh_cache_async` instead. This keeps save/delete/rename flows from ---updating the cache before a successful write. +---@param path string ---@param on_done fun(data: PeekstackStoreData) -function M.read_async(on_done) - store.read(SCOPE, { +function M.read_async(path, on_done) + store.read(path, { on_done = function(read_data) on_done(M.ensure_data(read_data)) end, @@ -46,38 +52,42 @@ function M.read_async(on_done) end ---Synchronously read and migrate store data without touching the cache. +---@param path string ---@return PeekstackStoreData -function M.read_sync() - return M.ensure_data(store.read_sync(SCOPE)) +function M.read_sync(path) + return M.ensure_data(store.read_sync(path)) end ---Asynchronously read store data and refresh the cache from disk. ---Used by read-only flows (restore, list_sessions) that should reflect the ---latest persisted state in memory. +---@param path string ---@param on_done fun(data: PeekstackStoreData) -function M.refresh_cache_async(on_done) - M.read_async(function(data) - cache.update(data) +function M.refresh_cache_async(path, on_done) + M.read_async(path, function(data) + cache.update(path, data) on_done(data) end) end ---Synchronously read store data and refresh the cache from disk. +---@param path string ---@return PeekstackStoreData -function M.refresh_cache_sync() - local data = M.read_sync() - cache.update(data) +function M.refresh_cache_sync(path) + local data = M.read_sync(path) + cache.update(path, data) return data end ---Asynchronously write data; on success refresh cache before calling `on_done`. +---@param path string ---@param data PeekstackStoreData ---@param on_done? fun(success: boolean) -function M.write_async(data, on_done) - store.write(SCOPE, data, { +function M.write_async(path, data, on_done) + store.write(path, data, { on_done = function(success) if success then - cache.update(data) + cache.update(path, data) end if on_done then on_done(success) @@ -86,7 +96,20 @@ function M.write_async(data, on_done) }) end +---Synchronously write data; on success refresh cache. +---@param path string +---@param data PeekstackStoreData +---@return boolean +function M.write_sync(path, data) + local success = store.write_sync(path, data) + if success then + cache.update(path, data) + end + return success +end + ---@class PeekstackPersistUpdate +---@field path string ---@field mutate fun(data: PeekstackStoreData): boolean ---@field on_done? fun(success: boolean) @@ -113,7 +136,7 @@ local function run_next_update() run_next_update() end - M.read_async(function(data) + M.read_async(update.path, function(data) local ok, keep = pcall(update.mutate, data) if not ok then notify.warn("Failed to update session data: " .. tostring(keep)) @@ -124,20 +147,21 @@ local function run_next_update() finish(false) return end - M.write_async(data, finish) + M.write_async(update.path, data, finish) end) end ----Asynchronously read, mutate and write back store data. +---Asynchronously read, mutate and write back store data at `path`. ---Updates are serialized: each one reads the file only after the previous ---write finished, so overlapping save/delete/rename calls cannot clobber each ---other with a stale snapshot (read-modify-write lost update). ---`mutate` returns true to persist the change or false to abort without ---writing; `on_done` receives whether the data was written successfully. +---@param path string ---@param mutate fun(data: PeekstackStoreData): boolean ---@param on_done? fun(success: boolean) -function M.update_async(mutate, on_done) - update_queue[#update_queue + 1] = { mutate = mutate, on_done = on_done } +function M.update_async(path, mutate, on_done) + update_queue[#update_queue + 1] = { path = path, mutate = mutate, on_done = on_done } if not update_running then run_next_update() end @@ -146,35 +170,25 @@ end ---Upper bound for draining in-flight async updates before a sync update. local UPDATE_SYNC_DRAIN_MS = 1000 ----Synchronously read, mutate and write back store data. +---Synchronously read, mutate and write back store data at `path`. ---Drains queued async updates first (bounded by UPDATE_SYNC_DRAIN_MS) so a ---sync save issued while an async save is mid-flight does not race it; the ---sync write itself then runs atomically from Lua's point of view. +---@param path string ---@param mutate fun(data: PeekstackStoreData): boolean ---@return boolean success whether the data was written -function M.update_sync(mutate) +function M.update_sync(path, mutate) if update_running then vim.wait(UPDATE_SYNC_DRAIN_MS, function() return not update_running end, 10) end - local data = M.read_sync() + local data = M.read_sync(path) if not mutate(data) then return false end - return M.write_sync(data) -end - ----Synchronously write data; on success refresh cache. ----@param data PeekstackStoreData ----@return boolean -function M.write_sync(data) - local success = store.write_sync(SCOPE, data) - if success then - cache.update(data) - end - return success + return M.write_sync(path, data) end ---Reset the in-memory session cache and drop queued updates. @@ -184,14 +198,16 @@ function M.reset_cache() update_running = false end +---@param path string ---@return boolean -function M.cache_loaded() - return cache.is_loaded() +function M.cache_loaded(path) + return cache.is_loaded(path) end +---@param path string ---@return table -function M.cache_sessions() - return cache.get() +function M.cache_sessions(path) + return cache.get(path) end return M diff --git a/lua/peekstack/persist/service.lua b/lua/peekstack/persist/service.lua index 1f2d134..6f4e79c 100644 --- a/lua/peekstack/persist/service.lua +++ b/lua/peekstack/persist/service.lua @@ -82,7 +82,9 @@ end ---Save the current stack to persistent storage with optional name. ---@param name? string ----@param opts? { root_winid?: integer, silent?: boolean, sync?: boolean, on_done?: fun(success: boolean) } +---`store_path` pins the store file; it defaults to the current repository's +---store, resolved when the save is requested. +---@param opts? { root_winid?: integer, store_path?: string, 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 @@ -100,9 +102,10 @@ function M.save_current(name, opts) local resolved_name = sessions.resolve_name(name) local items = sessions.collect_items(opts and opts.root_winid or nil) + local path = opts and opts.store_path or orchestrator.store_path() if sync then - local success = orchestrator.update_sync(function(data) + local success = orchestrator.update_sync(path, function(data) sessions.upsert(data, resolved_name, items) return true end) @@ -111,7 +114,7 @@ function M.save_current(name, opts) return end - orchestrator.update_async(function(data) + orchestrator.update_async(path, function(data) sessions.upsert(data, resolved_name, items) return true end, function(success) @@ -141,7 +144,7 @@ function M.restore(name, opts) -- Freeze the target stack now: the read below is async and the user may -- move to another window before the session is restored. local root_winid = sessions.resolve_root_winid(opts and opts.root_winid or nil) - orchestrator.refresh_cache_async(function(data) + orchestrator.refresh_cache_async(orchestrator.store_path(), function(data) local session = data.sessions[resolved_name] if not session or not session.items or #session.items == 0 then @@ -204,15 +207,16 @@ function M.list_sessions(opts) return {} end + local path = orchestrator.store_path() if on_done then - orchestrator.refresh_cache_async(function(data) + orchestrator.refresh_cache_async(path, function(data) on_done(data.sessions or {}) end) - elseif not orchestrator.cache_loaded() then - orchestrator.refresh_cache_sync() + elseif not orchestrator.cache_loaded(path) then + orchestrator.refresh_cache_sync(path) end - return orchestrator.cache_sessions() + return orchestrator.cache_sessions(path) end ---Delete a named session. @@ -223,7 +227,7 @@ function M.delete_session(name) end local found = false - orchestrator.update_async(function(data) + orchestrator.update_async(orchestrator.store_path(), function(data) found = sessions.delete(data, name) if not found then notify.warn("Session not found: " .. name) @@ -258,7 +262,7 @@ function M.rename_session(from, to) end local renamed = false - orchestrator.update_async(function(data) + orchestrator.update_async(orchestrator.store_path(), function(data) local result = sessions.rename(data, from, to) renamed = result.ok if result.err == "missing" then diff --git a/lua/peekstack/persist/store.lua b/lua/peekstack/persist/store.lua index bfd479d..581b0ac 100644 --- a/lua/peekstack/persist/store.lua +++ b/lua/peekstack/persist/store.lua @@ -1,4 +1,3 @@ -local fs = require("peekstack.util.fs") local codec = require("peekstack.persist.codec") local notify = require("peekstack.util.notify") @@ -30,15 +29,14 @@ local function ensure_parent_dir(path) return true end ----@param scope string +---@param path string store file path, resolved by the caller ---@param opts { on_done: fun(data: PeekstackStoreData) } -function M.read(scope, opts) +function M.read(path, opts) local on_done = opts and opts.on_done or nil if not on_done then return end - local path = fs.scope_path(scope) vim.uv.fs_stat(path, function(stat_err, stat) if stat_err or not stat or stat.size == 0 then vim.schedule(function() @@ -71,10 +69,9 @@ function M.read(scope, opts) end) end ----@param scope string +---@param path string store file path, resolved by the caller ---@return PeekstackStoreData -function M.read_sync(scope) - local path = fs.scope_path(scope) +function M.read_sync(path) local stat = vim.uv.fs_stat(path) if not stat or stat.size == 0 then return codec.empty_data() @@ -94,10 +91,10 @@ function M.read_sync(scope) return codec.decode(path, data) end ----@param scope string +---@param path string store file path, resolved by the caller ---@param data PeekstackStoreData ---@param opts? { on_done?: fun(success: boolean) } -function M.write(scope, data, opts) +function M.write(path, data, opts) local on_done = opts and opts.on_done or nil local function finish(success) if on_done then @@ -107,7 +104,6 @@ function M.write(scope, data, opts) end end - local path = fs.scope_path(scope) local encoded = codec.encode(data) if not encoded then finish(false) @@ -154,11 +150,10 @@ function M.write(scope, data, opts) end) end ----@param scope string +---@param path string store file path, resolved by the caller ---@param data PeekstackStoreData ---@return boolean -function M.write_sync(scope, data) - local path = fs.scope_path(scope) +function M.write_sync(path, data) local encoded = codec.encode(data) if not encoded then return false diff --git a/tests/persist_sessions_spec.lua b/tests/persist_sessions_spec.lua index 43e4e51..6f247cc 100644 --- a/tests/persist_sessions_spec.lua +++ b/tests/persist_sessions_spec.lua @@ -4,6 +4,7 @@ describe("peekstack.persist.sessions", function() local stack = require("peekstack.core.stack") local store = require("peekstack.persist.store") local migrate = require("peekstack.persist.migrate") + local fs = require("peekstack.util.fs") -- Use "repo" as the fixed scope for persistence local test_scope = "repo" @@ -72,7 +73,7 @@ describe("peekstack.persist.sessions", function() ---@param scope string ---@param data PeekstackStoreData local function write_and_wait(scope, data) - local success = store.write_sync(scope, data) + local success = store.write_sync(fs.scope_path(scope), data) assert.is_true(success, "Store write failed") end @@ -81,7 +82,7 @@ describe("peekstack.persist.sessions", function() local function read_and_wait(scope) local done = false local result = nil - store.read(scope, { + store.read(fs.scope_path(scope), { on_done = function(data) result = data done = true @@ -105,7 +106,7 @@ describe("peekstack.persist.sessions", function() end if not in_flight then in_flight = true - store.read(scope, { + store.read(fs.scope_path(scope), { on_done = function(data) if predicate(data) then satisfied = true @@ -1253,7 +1254,7 @@ describe("peekstack.persist.sessions", function() local original_write_sync = store.write_sync local ok, err = pcall(function() - store.write_sync = function(_scope, _data) + store.write_sync = function(_path, _data) return false end @@ -1283,7 +1284,7 @@ describe("peekstack.persist.sessions", function() local original_write = store.write local ok, err = pcall(function() - store.write = function(_scope, _data, opts) + store.write = function(_path, _data, opts) if opts and opts.on_done then vim.schedule(function() opts.on_done(false) @@ -1334,7 +1335,7 @@ describe("peekstack.persist.sessions", function() local original_write = store.write local write_done = false local ok, err = pcall(function() - store.write = function(_scope, _data, opts) + store.write = function(_path, _data, opts) if opts and opts.on_done then vim.schedule(function() opts.on_done(false) diff --git a/tests/persist_store_path_spec.lua b/tests/persist_store_path_spec.lua new file mode 100644 index 0000000..8468158 --- /dev/null +++ b/tests/persist_store_path_spec.lua @@ -0,0 +1,145 @@ +describe("peekstack.persist store path", function() + local persist = require("peekstack.persist") + local config = require("peekstack.config") + local fs = require("peekstack.util.fs") + local store = require("peekstack.persist.store") + local stack = require("peekstack.core.stack") + + local wait_timeout_ms = 1000 + local wait_interval_ms = 10 + + local original_cwd = nil + local work_dir = nil + ---@type { dir: string, path: string } + local repo_a = nil + ---@type { dir: string, path: string } + local repo_b = nil + + ---@param name string + ---@return { dir: string, path: string } + local function make_repo(name) + local dir = work_dir .. "/" .. name + vim.fn.mkdir(dir .. "/.git", "p") + vim.cmd.cd(dir) + return { dir = dir, path = fs.scope_path("repo") } + end + + ---@param name string + ---@return PeekstackSession + local function session(name) + return { + items = { { uri = "file:///tmp/" .. name .. ".lua", range = {}, title = name } }, + meta = { created_at = 1, updated_at = 1 }, + } + end + + ---@param repo { path: string } + ---@param names string[] + local function seed(repo, names) + local sessions = {} + for _, name in ipairs(names) do + sessions[name] = session(name) + end + assert.is_true(store.write_sync(repo.path, { version = 2, sessions = sessions })) + end + + ---@param repo { path: string } + ---@return string[] + local function session_names(repo) + local names = vim.tbl_keys(store.read_sync(repo.path).sessions or {}) + table.sort(names) + return names + end + + ---@param repo { path: string } + ---@param expected string[] + local function wait_for_names(repo, expected) + local ok = vim.wait(wait_timeout_ms, function() + return vim.deep_equal(session_names(repo), expected) + end, wait_interval_ms) + assert.is_true(ok, "Timed out waiting for sessions: " .. table.concat(expected, ",")) + end + + before_each(function() + config.setup({ persist = { enabled = true } }) + persist._reset_cache() + stack._reset() + original_cwd = vim.fn.getcwd() + work_dir = vim.fn.tempname() + repo_a = make_repo("a") + repo_b = make_repo("b") + seed(repo_a, { "a_existing" }) + seed(repo_b, { "b_existing" }) + vim.cmd.cd(repo_a.dir) + end) + + after_each(function() + vim.cmd.cd(original_cwd) + pcall(vim.fn.delete, repo_a.path) + pcall(vim.fn.delete, repo_b.path) + pcall(vim.fn.delete, work_dir, "rf") + persist._reset_cache() + stack._reset() + end) + + it("writes an async save to the repository it was requested in", function() + local saved = nil + persist.save_current("a_new", { + silent = true, + on_done = function(success) + saved = success + end, + }) + vim.cmd.cd(repo_b.dir) + + local ok = vim.wait(wait_timeout_ms, function() + return saved ~= nil + end, wait_interval_ms) + assert.is_true(ok, "Timed out waiting for save") + assert.is_true(saved) + assert.same({ "a_existing", "a_new" }, session_names(repo_a)) + assert.same({ "b_existing" }, session_names(repo_b)) + end) + + it("deletes from the repository it was requested in", function() + seed(repo_a, { "a_existing", "a_doomed" }) + seed(repo_b, { "a_doomed", "b_existing" }) + + persist.delete_session("a_doomed") + vim.cmd.cd(repo_b.dir) + + wait_for_names(repo_a, { "a_existing" }) + assert.same({ "a_doomed", "b_existing" }, session_names(repo_b)) + end) + + it("renames in the repository it was requested in", function() + persist.rename_session("a_existing", "a_renamed") + vim.cmd.cd(repo_b.dir) + + wait_for_names(repo_a, { "a_renamed" }) + assert.same({ "b_existing" }, session_names(repo_b)) + end) + + it("keeps queued updates bound to their own repository", function() + persist.save_current("a_queued", { silent = true }) + vim.cmd.cd(repo_b.dir) + persist.save_current("b_queued", { silent = true }) + + wait_for_names(repo_a, { "a_existing", "a_queued" }) + wait_for_names(repo_b, { "b_existing", "b_queued" }) + end) + + it("lists the current repository's sessions after a directory change", function() + assert.is_not_nil(persist.list_sessions({ silent = true }).a_existing) + + vim.cmd.cd(repo_b.dir) + local listed = persist.list_sessions({ silent = true }) + assert.is_nil(listed.a_existing) + assert.is_not_nil(listed.b_existing) + + vim.cmd.cd(repo_a.dir) + listed = persist.list_sessions({ silent = true }) + assert.is_not_nil(listed.a_existing) + assert.is_nil(listed.b_existing) + end) +end) diff --git a/tests/persist_store_spec.lua b/tests/persist_store_spec.lua index 20ce245..55503ae 100644 --- a/tests/persist_store_spec.lua +++ b/tests/persist_store_spec.lua @@ -2,16 +2,16 @@ describe("peekstack.persist.store", function() local store = require("peekstack.persist.store") local fs = require("peekstack.util.fs") - local test_scope = "global" + local test_path = fs.scope_path("global") local wait_timeout_ms = 500 local wait_interval_ms = 10 - ---@param scope string + ---@param path string ---@param data PeekstackStoreData - local function write_and_wait(scope, data) + local function write_and_wait(path, data) local done = false local success = false - store.write(scope, data, { + store.write(path, data, { on_done = function(ok) done = true success = ok @@ -24,12 +24,12 @@ describe("peekstack.persist.store", function() assert.is_true(success, "Store write failed") end - ---@param scope string + ---@param path string ---@return PeekstackStoreData - local function read_and_wait(scope) + local function read_and_wait(path) local done = false local result = nil - store.read(scope, { + store.read(path, { on_done = function(data) result = data done = true @@ -75,20 +75,19 @@ describe("peekstack.persist.store", function() end before_each(function() - write_and_wait(test_scope, { version = 2, sessions = {} }) + write_and_wait(test_path, { version = 2, sessions = {} }) end) after_each(function() - write_and_wait(test_scope, { version = 2, sessions = {} }) + write_and_wait(test_path, { version = 2, sessions = {} }) end) it("returns empty data when file is missing", function() - local path = fs.scope_path(test_scope) - delete_and_wait(path) + delete_and_wait(test_path) local called = false local result = nil - store.read(test_scope, { + store.read(test_path, { on_done = function(data) called = true result = data @@ -114,15 +113,14 @@ describe("peekstack.persist.store", function() }, }, } - write_and_wait(test_scope, data) + write_and_wait(test_path, data) - local result = read_and_wait(test_scope) + local result = read_and_wait(test_path) assert.same(data, result) end) it("returns empty data and warns for invalid JSON", function() - local path = fs.scope_path(test_scope) - write_raw_and_wait(path, "{ invalid json") + write_raw_and_wait(test_path, "{ invalid json") local warnings = {} local original_notify = vim.notify @@ -132,7 +130,7 @@ describe("peekstack.persist.store", function() end end - local result = read_and_wait(test_scope) + local result = read_and_wait(test_path) vim.notify = original_notify assert.same({ version = 2, sessions = {} }, result) @@ -150,15 +148,14 @@ describe("peekstack.persist.store", function() }, }, } - write_and_wait(test_scope, data) + write_and_wait(test_path, data) - local result = store.read_sync(test_scope) + local result = store.read_sync(test_path) assert.same(data, result) end) it("read_sync returns empty data and warns for invalid JSON", function() - local path = fs.scope_path(test_scope) - write_raw_and_wait(path, "{ invalid json") + write_raw_and_wait(test_path, "{ invalid json") local warnings = {} local original_notify = vim.notify @@ -168,7 +165,7 @@ describe("peekstack.persist.store", function() end end - local result = store.read_sync(test_scope) + local result = store.read_sync(test_path) vim.notify = original_notify assert.same({ version = 2, sessions = {} }, result) @@ -187,12 +184,12 @@ describe("peekstack.persist.store", function() }, } - local ok = store.write_sync(test_scope, data) + local ok = store.write_sync(test_path, data) assert.is_true(ok) - assert.same(data, store.read_sync(test_scope)) + assert.same(data, store.read_sync(test_path)) end) - it("allows overlapping async writes to the same scope", function() + it("allows overlapping async writes to the same file", function() local first = { version = 2, sessions = { @@ -214,13 +211,13 @@ describe("peekstack.persist.store", function() local done = 0 local successes = {} - store.write(test_scope, first, { + store.write(test_path, first, { on_done = function(ok) done = done + 1 successes[#successes + 1] = ok end, }) - store.write(test_scope, second, { + store.write(test_path, second, { on_done = function(ok) done = done + 1 successes[#successes + 1] = ok @@ -234,13 +231,13 @@ describe("peekstack.persist.store", function() assert.is_true(successes[1]) assert.is_true(successes[2]) - local result = read_and_wait(test_scope) + local result = read_and_wait(test_path) assert.equals(2, result.version) assert.is_true(result.sessions.first ~= nil or result.sessions.second ~= nil) end) it("write_sync returns false when payload cannot be encoded", function() - local ok = store.write_sync(test_scope, { + local ok = store.write_sync(test_path, { version = 2, sessions = {}, invalid = function() end, From eace3748bbdc460a72dd691fc8adb8ab6e5443c2 Mon Sep 17 00:00:00 2001 From: Masaaki Hirotsu Date: Sat, 3 Oct 2026 07:00:48 +0900 Subject: [PATCH 3/3] fix(persist): save the tracked stack on leave instead of the current window save_on_leave() fell back to the current window once the debounced save had run, so moving to an empty split and quitting overwrote the auto session with zero items while the original stack still held popups. - Track the stack to save (root window and store file) separately from the debounce state, and save that stack on leave; skip the leave save when no stack changed during the session. - Resolve the store file when a change is observed, flush a pending save before a change for another stack or store replaces it, and bind each timer callback to the target it debounced. - Skip saving once the tracked root window is closed instead of saving the current window's unrelated stack, and document the auto session target. --- README.md | 5 ++ doc/peekstack.txt | 6 ++ lua/peekstack/persist/auto.lua | 121 ++++++++++++++++++++++------- tests/persist_auto_spec.lua | 138 ++++++++++++++++++++++++++++++++- 4 files changed, 242 insertions(+), 28 deletions(-) diff --git a/README.md b/README.md index 3e4dfeb..95b034b 100644 --- a/README.md +++ b/README.md @@ -373,6 +373,11 @@ When `persist.auto.enabled = true`, peekstack can automatically restore and save - **Save** on `PeekstackPush` / `PeekstackClose` / `PeekstackRestorePopup` with a debounce - **Save on leave** on `VimLeavePre` if `save_on_leave = true` +The auto session holds a single stack: the one that changed most recently, saved to the repository +that was current when it changed. Leaving Neovim saves that stack even if the cursor is in another +window, skips the save if no stack changed during the session, and keeps the stored session as-is +once that stack's window has been closed. + Auto persist only runs inside a git repository and always uses the repository session storage. Make sure `persist.enabled = true` as well. diff --git a/doc/peekstack.txt b/doc/peekstack.txt index 375884f..15ed8ce 100644 --- a/doc/peekstack.txt +++ b/doc/peekstack.txt @@ -468,6 +468,12 @@ Auto persistence (`persist.auto`) requires `persist.enabled = true`: debounce_ms debounce time for auto save save_on_leave save on VimLeavePre +The auto session holds a single stack: the one that changed most +recently, saved to the repository that was current when it changed. +Leaving Neovim saves that stack even if the cursor is in another window, +skips the save if no stack changed during the session, and keeps the +stored session as-is once that stack's window has been closed. + Auto persistence runs only inside a git repository and always uses the repository session storage. diff --git a/lua/peekstack/persist/auto.lua b/lua/peekstack/persist/auto.lua index 96ba3c2..91762a6 100644 --- a/lua/peekstack/persist/auto.lua +++ b/lua/peekstack/persist/auto.lua @@ -1,15 +1,29 @@ local config = require("peekstack.config") local fs = require("peekstack.util.fs") +local orchestrator = require("peekstack.persist.orchestrator") local persist = require("peekstack.persist") local stack = require("peekstack.core.stack") local timer_util = require("peekstack.util.timer") local M = {} +---A stack together with the store file it is saved to. Both are fixed when +---the change is observed, so neither a `:cd` nor a window switch before the +---save runs can redirect it. +---@class PeekstackAutoSaveTarget +---@field root_winid integer +---@field store_path string + ---@type uv.uv_timer_t? local save_timer = nil ----@type integer? -local pending_root_winid = nil +---Target whose debounced save has not run yet. +---@type PeekstackAutoSaveTarget? +local pending_target = nil +---The auto session mirrors a single stack: the one changed most recently. +---Kept apart from `pending_target` so the leave-time save still knows which +---stack to write after the debounced save has already run. +---@type PeekstackAutoSaveTarget? +local tracked_target = nil ---@type string? local last_restored_repo = nil @@ -31,15 +45,6 @@ local function resolve_session_name() return "auto" end ----@param winid? integer ----@return integer? -local function normalize_root_winid(winid) - if winid and type(winid) == "number" and vim.api.nvim_win_is_valid(winid) then - return winid - end - return nil -end - ---@return integer local function resolve_root_winid() local winid = vim.api.nvim_get_current_win() @@ -53,24 +58,62 @@ local function resolve_root_winid() return winid end +---Capture the stack to save and the store it belongs to. +---An explicit `root_winid` that no longer exists yields nil instead of falling +---back to the current window, which holds an unrelated stack. ---@param root_winid? integer +---@return PeekstackAutoSaveTarget? +local function capture_target(root_winid) + if not fs.repo_root() then + return nil + end + if root_winid == nil then + root_winid = resolve_root_winid() + elseif type(root_winid) ~= "number" or not vim.api.nvim_win_is_valid(root_winid) then + return nil + end + return { + root_winid = stack.get_root_winid(root_winid), + store_path = orchestrator.store_path(), + } +end + +---@param a PeekstackAutoSaveTarget +---@param b PeekstackAutoSaveTarget +---@return boolean +local function same_target(a, b) + return a.root_winid == b.root_winid and a.store_path == b.store_path +end + +---@param target PeekstackAutoSaveTarget ---@param opts? { sync?: boolean } ---@return boolean -local function save_session(root_winid, opts) +local function save_session(target, opts) if not is_enabled() then return false end - if not fs.repo_root() then + -- Once the root window is gone its stack is too; saving now would collect + -- the current window's stack and overwrite the session with it. + if not vim.api.nvim_win_is_valid(target.root_winid) then return false end persist.save_current(resolve_session_name(), { - root_winid = normalize_root_winid(root_winid), + root_winid = target.root_winid, + store_path = target.store_path, silent = true, sync = opts and opts.sync or false, }) return true end +local function flush_pending() + local target = pending_target + pending_target = nil + if target then + save_session(target) + end +end + ---@return boolean function M.maybe_restore() if not is_enabled() then @@ -115,16 +158,23 @@ function M.schedule_save(opts) return false end - if not fs.repo_root() then + local target = capture_target(opts and opts.root_winid or nil) + if not target then return false end - local debounce_ms = tonumber(cfg.persist.auto.debounce_ms) or 1000 - local root_winid = normalize_root_winid(opts and opts.root_winid or nil) - if root_winid then - pending_root_winid = root_winid + -- A debounced save for another stack or store must not be swallowed by + -- this one: write it out now instead of letting the new target replace it. + if pending_target and not same_target(pending_target, target) then + if save_timer then + save_timer:stop() + end + flush_pending() end + pending_target = target + tracked_target = target + local debounce_ms = tonumber(cfg.persist.auto.debounce_ms) or 1000 if save_timer then save_timer:stop() else @@ -134,16 +184,23 @@ function M.schedule_save(opts) save_timer:start(debounce_ms, 0, function() save_timer:stop() - local target_winid = pending_root_winid - pending_root_winid = nil + -- Bind to the target this timer debounced: a schedule_save() that runs + -- before the scheduled callback replaces pending_target, and that newer + -- target must wait for its own timer. + local fired = pending_target vim.schedule(function() - save_session(target_winid) + if fired and pending_target == fired then + flush_pending() + end end) end) return true end +---Without an explicit `root_winid` this saves the stack auto save last +---tracked, not the current window's, which may be an unrelated split. +---Nothing is written when no stack changed during the session. ---@param opts? { root_winid?: integer } ---@return boolean function M.save_on_leave(opts) @@ -156,21 +213,30 @@ function M.save_on_leave(opts) return false end - local root_winid = normalize_root_winid(opts and opts.root_winid or nil) or pending_root_winid - pending_root_winid = nil + local target + if opts and opts.root_winid then + target = capture_target(opts.root_winid) + else + target = pending_target or tracked_target + end + pending_target = nil if save_timer then save_timer:stop() end - return save_session(root_winid, { sync = true }) + if not target then + return false + end + return save_session(target, { sync = true }) end function M.setup() timer_util.close(save_timer) timer_util.get_store().persist_auto = nil save_timer = nil - pending_root_winid = nil + pending_target = nil + tracked_target = nil local group = vim.api.nvim_create_augroup("PeekstackPersistAuto", { clear = true }) @@ -213,7 +279,8 @@ end ---Reset internal state (for testing). function M._reset() last_restored_repo = nil - pending_root_winid = nil + pending_target = nil + tracked_target = nil timer_util.close(save_timer) timer_util.get_store().persist_auto = nil save_timer = nil diff --git a/tests/persist_auto_spec.lua b/tests/persist_auto_spec.lua index 14c9254..449aa62 100644 --- a/tests/persist_auto_spec.lua +++ b/tests/persist_auto_spec.lua @@ -8,6 +8,32 @@ describe("peekstack.persist.auto", function() local original_repo_root = nil local original_restore = nil local original_save = nil + local repo = "/tmp/peekstack-test-repo" + + ---@return { name: string, opts: table }[] + local function capture_saves() + local calls = {} + original_save = persist.save_current + persist.save_current = function(name, opts) + calls[#calls + 1] = { name = name, opts = opts } + end + return calls + end + + ---@param calls table[] + ---@param count integer + local function wait_for_saves(calls, count) + local ok = vim.wait(200, function() + return #calls == count + end, 10) + assert.is_true(ok, "Timed out waiting for " .. count .. " save(s)") + end + + ---@param repo_dir string + ---@return string + local function store_path_for(repo_dir) + return vim.fn.stdpath("state") .. "/peekstack/repo_" .. fs.slug(repo_dir) .. ".json" + end before_each(function() stack._reset() @@ -27,9 +53,10 @@ describe("peekstack.persist.auto", function() }, }) + repo = "/tmp/peekstack-test-repo" original_repo_root = fs.repo_root fs.repo_root = function() - return "/tmp/peekstack-test-repo" + return repo end end) @@ -48,6 +75,8 @@ describe("peekstack.persist.auto", function() original_save = nil auto._reset() stack._reset() + vim.cmd("silent! tabonly") + vim.cmd("silent! only") end) it("restores when repo exists and stack is empty", function() @@ -119,4 +148,111 @@ describe("peekstack.persist.auto", function() assert.is_true(saved) assert.equals(1, calls) end) + + it("saves to the store resolved when the save was scheduled", function() + local calls = capture_saves() + + auto.schedule_save({ root_winid = vim.api.nvim_get_current_win() }) + repo = "/tmp/peekstack-test-other-repo" + + wait_for_saves(calls, 1) + assert.equals(store_path_for("/tmp/peekstack-test-repo"), calls[1].opts.store_path) + end) + + it("flushes a pending save before scheduling one for another store", function() + local calls = capture_saves() + local winid = vim.api.nvim_get_current_win() + + auto.schedule_save({ root_winid = winid }) + repo = "/tmp/peekstack-test-other-repo" + auto.schedule_save({ root_winid = winid }) + + wait_for_saves(calls, 2) + assert.equals(store_path_for("/tmp/peekstack-test-repo"), calls[1].opts.store_path) + assert.equals(store_path_for("/tmp/peekstack-test-other-repo"), calls[2].opts.store_path) + end) + + it("flushes a pending save before scheduling one for another stack", function() + local calls = capture_saves() + local first = vim.api.nvim_get_current_win() + vim.cmd("vsplit") + local second = vim.api.nvim_get_current_win() + + auto.schedule_save({ root_winid = first }) + auto.schedule_save({ root_winid = second }) + + wait_for_saves(calls, 2) + assert.equals(first, calls[1].opts.root_winid) + assert.equals(second, calls[2].opts.root_winid) + end) + + it("saves the stack of the window that changed after a tab switch", function() + local calls = capture_saves() + local winid = vim.api.nvim_get_current_win() + + auto.schedule_save({ root_winid = winid }) + vim.cmd("tabnew") + + wait_for_saves(calls, 1) + assert.equals(winid, calls[1].opts.root_winid) + end) + + it("saves the tracked stack on leave from another split", function() + local calls = capture_saves() + local winid = vim.api.nvim_get_current_win() + + auto.schedule_save({ root_winid = winid }) + wait_for_saves(calls, 1) + + vim.cmd("vsplit") + assert.are_not.equal(winid, vim.api.nvim_get_current_win()) + repo = "/tmp/peekstack-test-other-repo" + + assert.is_true(auto.save_on_leave()) + assert.equals(2, #calls) + assert.equals(winid, calls[2].opts.root_winid) + assert.equals(store_path_for("/tmp/peekstack-test-repo"), calls[2].opts.store_path) + assert.is_true(calls[2].opts.sync) + end) + + it("saves a pending stack synchronously on leave", function() + local calls = capture_saves() + local winid = vim.api.nvim_get_current_win() + + auto.schedule_save({ root_winid = winid }) + vim.cmd("vsplit") + + assert.is_true(auto.save_on_leave()) + assert.equals(1, #calls) + assert.equals(winid, calls[1].opts.root_winid) + assert.is_true(calls[1].opts.sync) + + vim.wait(60, function() + return false + end, 10) + assert.equals(1, #calls) + end) + + it("does not save on leave when no stack changed", function() + local calls = capture_saves() + assert.is_false(auto.save_on_leave()) + assert.equals(0, #calls) + end) + + it("does not save a stack whose root window was closed", function() + local calls = capture_saves() + vim.cmd("vsplit") + local closing = vim.api.nvim_get_current_win() + + auto.schedule_save({ root_winid = closing }) + vim.api.nvim_win_close(closing, true) + + vim.wait(60, function() + return false + end, 10) + assert.equals(0, #calls) + assert.is_false(auto.save_on_leave()) + assert.equals(0, #calls) + assert.is_false(auto.schedule_save({ root_winid = closing })) + end) end)