fix: take the channel snapshot where the caches are mutated

Passing ::snapshot to the debounced write meant the three caches were read
from the writer thread while the server thread mutated them. They are
separate maps, so a snapshot could catch a channel already removed from
channelsCache while its membersCache entry still existed - persisting an
orphaned member list - or the reverse, persisting a channel with no members
and therefore no owner. Neither crashes anything, since the readers filter
on channelsCache, but they are wrong state written to channels.json and
carried across restarts.

The snapshot is taken on the mutating thread again and handed over through
a volatile field, so the write still reads the newest state when it
eventually runs rather than the state at queue time. That keeps both
properties: consistent halves, and a batched write that reflects every
change made during the delay.

This does not give back the per-change cost the earlier commit was avoiding,
because copying three maps is not what made saving expensive - the debounce
already coalesces the file write, which is.

Also floors the conversion cache interval at one second. It is now the only
writer besides shutdown, so a non-positive value would both be rejected by
runAtFixedRate and leave the cache unsaved until the server stopped.

Co-Authored-By: Claude <noreply@anthropic.com>

3 files changed, 53 insertions, 3 deletions

diff --git a/platform-paper/src/main/kotlin/dev/m1sk9/lunaticChat/paper/ServiceInitializer.kt b/platform-paper/src/main/kotlin/dev/m1sk9/lunaticChat/paper/ServiceInitializer.kt
index 9729f55..522a1a6 100644
--- a/platform-paper/src/main/kotlin/dev/m1sk9/lunaticChat/paper/ServiceInitializer.kt
+++ b/platform-paper/src/main/kotlin/dev/m1sk9/lunaticChat/paper/ServiceInitializer.kt
@@ -418,10 +418,13 @@ class ServiceInitializer(
     fun schedulePeriodicTasks(services: ServiceContainer) {
         val conversionCache = services.conversionCache
         if (conversionCache != null) {
+            // The periodic task is the only writer besides shutdown, so a non-positive interval
+            // would both reject the schedule and leave the cache unsaved until the server stops.
             val intervalSeconds =
                 configuration.features.japaneseConversion
                     .cacheSaveIntervalSeconds
                     .toLong()
+                    .coerceAtLeast(1)
             plugin.server.asyncScheduler.runAtFixedRate(
                 plugin,
                 { conversionCache.saveToDisk() },
diff --git a/platform-paper/src/main/kotlin/dev/m1sk9/lunaticChat/paper/chat/channel/ChannelManager.kt b/platform-paper/src/main/kotlin/dev/m1sk9/lunaticChat/paper/chat/channel/ChannelManager.kt
index 7b41f75..663984c 100644
--- a/platform-paper/src/main/kotlin/dev/m1sk9/lunaticChat/paper/chat/channel/ChannelManager.kt
+++ b/platform-paper/src/main/kotlin/dev/m1sk9/lunaticChat/paper/chat/channel/ChannelManager.kt
@@ -29,6 +29,12 @@ class ChannelManager(
     private val membersCache = ConcurrentHashMap<String, CopyOnWriteArrayList<ChannelMember>>()
     private val activeChannels = ConcurrentHashMap<UUID, String>()
 
+    // Handed to the debounced write, which reads it when it finally runs rather than when it was
+    // queued - so a batched write persists every change made during the delay, not just the one
+    // that started it.
+    @Volatile
+    private var latestSnapshot: ChannelData = ChannelData()
+
     /**
      * Initializes the ChannelManager by loading data from storage.
      */
@@ -434,13 +440,18 @@ class ChannelManager(
      * Saves the current state of channels and members to storage asynchronously.
      */
     private fun saveToStorage() {
-        storage.queueAsyncSave(::snapshot)
+        latestSnapshot = snapshot()
+        storage.queueAsyncSave { latestSnapshot }
         logger.fine("${channelsCache.size} channels queued for saving to storage.")
     }
 
     /**
-     * A point-in-time copy of everything persisted. Built inside the write rather than at each
-     * call site, so a burst of changes copies the caches once.
+     * A point-in-time copy of everything persisted.
+     *
+     * Taken on the mutating thread, because the three caches are separate: read from the write
+     * thread instead, a snapshot could catch a channel already removed from [channelsCache] while
+     * its [membersCache] entry still existed, and persist the halves inconsistently. Copying the
+     * caches is cheap; it is the file write that the debounce is there to coalesce.
      */
     private fun snapshot(): ChannelData =
         ChannelData(
diff --git a/platform-paper/src/test/kotlin/dev/m1sk9/lunaticChat/paper/chat/channel/ChannelManagerTest.kt b/platform-paper/src/test/kotlin/dev/m1sk9/lunaticChat/paper/chat/channel/ChannelManagerTest.kt
index d5ae373..04bdfd8 100644
--- a/platform-paper/src/test/kotlin/dev/m1sk9/lunaticChat/paper/chat/channel/ChannelManagerTest.kt
+++ b/platform-paper/src/test/kotlin/dev/m1sk9/lunaticChat/paper/chat/channel/ChannelManagerTest.kt
@@ -16,6 +16,7 @@ import dev.m1sk9.lunaticChat.paper.TestUtils.createTestUUID
 import dev.m1sk9.lunaticChat.paper.config.key.ChannelChatFeatureConfig
 import io.mockk.every
 import io.mockk.mockk
+import io.mockk.slot
 import io.mockk.verify
 import kotlin.test.Test
 import kotlin.test.assertEquals
@@ -624,4 +625,39 @@ class ChannelManagerTest {
 
         verify { storage.saveToDisk(any()) }
     }
+
+    @Test
+    fun `a queued write sees changes made after it was queued`() {
+        val (manager, storage, _) = createManager()
+        val queued = slot<() -> ChannelData>()
+        every { storage.queueAsyncSave(capture(queued)) } returns Unit
+
+        val ownerId = createTestUUID(1)
+        manager.createChannel(createTestChannel(id = "first-ch", name = "First", ownerId = ownerId))
+        manager.createChannel(createTestChannel(id = "second-ch", name = "Second", ownerId = createTestUUID(2)))
+
+        // The debounced write runs later; it must persist the state as of then, not as of the
+        // change that started the timer.
+        val persisted = queued.captured()
+        assertTrue(persisted.channels.containsKey("first-ch"))
+        assertTrue(persisted.channels.containsKey("second-ch"))
+    }
+
+    @Test
+    fun `a queued write never persists a channel without its members`() {
+        val (manager, storage, _) = createManager()
+        val queued = slot<() -> ChannelData>()
+        every { storage.queueAsyncSave(capture(queued)) } returns Unit
+
+        val ownerId = createTestUUID(1)
+        manager.createChannel(createTestChannel(id = "keep-ch", name = "Keep", ownerId = ownerId))
+        manager.createChannel(createTestChannel(id = "drop-ch", name = "Drop", ownerId = ownerId))
+        manager.deleteChannel("drop-ch", ownerId)
+
+        // channelsCache, membersCache and activeChannels are three separate maps. The snapshot has
+        // to be taken where they are mutated, or it can catch them mid-update and persist halves.
+        val persisted = queued.captured()
+        assertEquals(persisted.channels.keys, persisted.members.keys)
+        assertTrue(persisted.activeChannels.values.all { it in persisted.channels.keys })
+    }
 }