refactor(persist): preserve cache update timing on write failures

Split the orchestrator API into read_*/refresh_cache_* variants so the
cache is only mutated when callers explicitly want to reflect on-disk
state (restore, list_sessions) or after a write succeeds. Save, delete,
and rename now read without touching the cache, matching the original
service.lua semantics where a failed write must not clobber the cached
view.

Author: mhiro2

lua/peekstack/persist/orchestrator.lua

@@ -19,13 +19,6 @@ function M.ensure_data(data)
   return migrate.ensure(data)
 end
 
----Migrate read data and refresh the in-memory cache.
----@param read_data PeekstackStoreData
----@return PeekstackStoreData
-function M.update_cache(read_data)
-  return cache.update(M.ensure_data(read_data))
-end
-
 ---Check if persistence is enabled, optionally notifying when disabled.
 ---@param silent? boolean
 ---@return boolean
@@ -39,20 +32,42 @@ function M.ensure_enabled(silent)
   return true
 end
 
----Asynchronously read store data, refresh cache, and pass migrated data to `on_done`.
+---Asynchronously read store data and pass migrated data to `on_done`.
+---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 on_done fun(data: PeekstackStoreData)
 function M.read_async(on_done)
   store.read(SCOPE, {
     on_done = function(read_data)
-      on_done(M.update_cache(read_data))
+      on_done(M.ensure_data(read_data))
     end,
   })
 end
 
----Synchronously read store data and refresh cache.
+---Synchronously read and migrate store data without touching the cache.
 ---@return PeekstackStoreData
 function M.read_sync()
-  return M.update_cache(store.read_sync(SCOPE))
+  return M.ensure_data(store.read_sync(SCOPE))
+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 on_done fun(data: PeekstackStoreData)
+function M.refresh_cache_async(on_done)
+  M.read_async(function(data)
+    cache.update(data)
+    on_done(data)
+  end)
+end
+
+---Synchronously read store data and refresh the cache from disk.
+---@return PeekstackStoreData
+function M.refresh_cache_sync()
+  local data = M.read_sync()
+  cache.update(data)
+  return data
 end
 
 ---Asynchronously write data; on success refresh cache before calling `on_done`.

lua/peekstack/persist/service.lua

@@ -50,7 +50,7 @@ function M.save_current(name, opts)
   local items = sessions.collect_items(opts and opts.root_winid or nil)
 
   if sync then
-    local data = sessions.upsert(orchestrator.ensure_data(orchestrator.read_sync()), resolved_name, items)
+    local data = sessions.upsert(orchestrator.read_sync(), resolved_name, items)
     local success = orchestrator.write_sync(data)
     notify_save_result(success, resolved_name, items, silent)
     finish(success)
@@ -84,7 +84,7 @@ function M.restore(name, opts)
   end
 
   local resolved_name = sessions.resolve_name(name)
-  orchestrator.read_async(function(data)
+  orchestrator.refresh_cache_async(function(data)
     local session = data.sessions[resolved_name]
 
     if not session or not session.items or #session.items == 0 then
@@ -158,11 +158,11 @@ function M.list_sessions(opts)
   end
 
   if on_done then
-    orchestrator.read_async(function(data)
+    orchestrator.refresh_cache_async(function(data)
       on_done(data.sessions or {})
     end)
   elseif not orchestrator.cache_loaded() then
-    orchestrator.read_sync()
+    orchestrator.refresh_cache_sync()
   end
 
   return orchestrator.cache_sessions()

tests/persist_sessions_spec.lua

@@ -899,4 +899,118 @@ describe("peekstack.persist.sessions", function()
 
     stack_view.toggle()
   end)
+
+  it("should not update cache when sync save write fails", function()
+    push_popup("write_fail_sync", { title = "Sync fail" })
+
+    local original_write_sync = store.write_sync
+    local ok, err = pcall(function()
+      store.write_sync = function(_scope, _data)
+        return false
+      end
+
+      local saved = nil
+      persist.save_current("write_fail_sync_session", {
+        silent = true,
+        sync = true,
+        on_done = function(success)
+          saved = success
+        end,
+      })
+
+      assert.is_false(saved)
+      local sessions = persist.list_sessions({ silent = true })
+      assert.is_nil(sessions["write_fail_sync_session"])
+    end)
+
+    store.write_sync = original_write_sync
+
+    if not ok then
+      error(err)
+    end
+  end)
+
+  it("should not update cache when async save write fails", function()
+    push_popup("write_fail_async", { title = "Async fail" })
+
+    local original_write = store.write
+    local ok, err = pcall(function()
+      store.write = function(_scope, _data, opts)
+        if opts and opts.on_done then
+          vim.schedule(function()
+            opts.on_done(false)
+          end)
+        end
+      end
+
+      local saved = nil
+      persist.save_current("write_fail_async_session", {
+        silent = true,
+        on_done = function(success)
+          saved = success
+        end,
+      })
+
+      local waited = vim.wait(wait_timeout_ms, function()
+        return saved ~= nil
+      end, wait_interval_ms)
+      assert.is_true(waited, "Timed out waiting for save callback")
+      assert.is_false(saved)
+
+      local listed = nil
+      persist.list_sessions({
+        silent = true,
+        on_done = function(sessions)
+          listed = sessions
+        end,
+      })
+      local list_waited = vim.wait(wait_timeout_ms, function()
+        return listed ~= nil
+      end, wait_interval_ms)
+      assert.is_true(list_waited, "Timed out waiting for list callback")
+      assert.is_nil(listed["write_fail_async_session"])
+    end)
+
+    store.write = original_write
+
+    if not ok then
+      error(err)
+    end
+  end)
+
+  it("should not update cache when delete write fails", function()
+    push_popup("delete_write_fail", { title = "To delete" })
+    persist.save_current("delete_write_fail_session", { silent = true, sync = true })
+    wait_for_session("delete_write_fail_session", true)
+
+    local original_write = store.write
+    local write_done = false
+    local ok, err = pcall(function()
+      store.write = function(_scope, _data, opts)
+        if opts and opts.on_done then
+          vim.schedule(function()
+            opts.on_done(false)
+            write_done = true
+          end)
+        end
+      end
+
+      persist.delete_session("delete_write_fail_session")
+
+      local waited = vim.wait(wait_timeout_ms, function()
+        return write_done
+      end, wait_interval_ms)
+      assert.is_true(waited, "Timed out waiting for delete write attempt")
+
+      -- Cache should still hold the session because the write failed.
+      local cached = persist.list_sessions({ silent = true })
+      assert.is_not_nil(cached["delete_write_fail_session"])
+    end)
+
+    store.write = original_write
+
+    if not ok then
+      error(err)
+    end
+  end)
 end)