fix: give every data file the same atomic write

Writing to a fixed sibling only moved the interleaving from the destination
to the temporary file: two saves racing there published mixed content, and
the losing move then failed with the temporary file already gone. Each write
now gets a unique temporary file, and falls back to a non-atomic replace on
the network mounts that refuse an atomic rename.

settings.yml and the conversion cache were still written in place. Both are
discarded wholesale when they do not parse, so a torn file silently costs
every player's settings or the whole accumulated cache.

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

5 files changed, 123 insertions, 16 deletions

diff --git a/platform-paper/src/main/kotlin/dev/m1sk9/lunaticChat/paper/AtomicWrite.kt b/platform-paper/src/main/kotlin/dev/m1sk9/lunaticChat/paper/AtomicWrite.kt
new file mode 100644
index 0000000..c3cd28b
--- /dev/null
+++ b/platform-paper/src/main/kotlin/dev/m1sk9/lunaticChat/paper/AtomicWrite.kt
@@ -0,0 +1,34 @@
+package dev.m1sk9.lunaticChat.paper
+
+import java.nio.file.AtomicMoveNotSupportedException
+import java.nio.file.Files
+import java.nio.file.Path
+import java.nio.file.StandardCopyOption
+import kotlin.io.path.deleteIfExists
+import kotlin.io.path.writeText
+
+/**
+ * Replaces the file at this path with [content], so nothing ever reads a half-written file.
+ *
+ * Bukkit runs onDisable before cancelling scheduler tasks, so a shutdown save and a still-pending
+ * debounced save can reach the same file at once. The temporary file gets a unique name for that
+ * reason: a fixed sibling would only move the interleaving from the destination to the temporary
+ * file, and the losing move would then fail with it already gone.
+ */
+fun Path.writeTextAtomically(content: String) {
+    val temporaryFile = Files.createTempFile(parent, fileName.toString(), ".tmp")
+    try {
+        temporaryFile.writeText(content)
+        try {
+            Files.move(temporaryFile, this, StandardCopyOption.REPLACE_EXISTING, StandardCopyOption.ATOMIC_MOVE)
+        } catch (_: AtomicMoveNotSupportedException) {
+            // Network-mounted data directories (NFS, SMB) can refuse an atomic rename. A plain
+            // replace is still better than writing the destination in place, since the content is
+            // already complete by the time anything lands on top of it.
+            Files.move(temporaryFile, this, StandardCopyOption.REPLACE_EXISTING)
+        }
+    } catch (e: Throwable) {
+        temporaryFile.deleteIfExists()
+        throw e
+    }
+}
diff --git a/platform-paper/src/main/kotlin/dev/m1sk9/lunaticChat/paper/chat/channel/ChannelStorage.kt b/platform-paper/src/main/kotlin/dev/m1sk9/lunaticChat/paper/chat/channel/ChannelStorage.kt
index 2030eb8..4fb18e8 100644
--- a/platform-paper/src/main/kotlin/dev/m1sk9/lunaticChat/paper/chat/channel/ChannelStorage.kt
+++ b/platform-paper/src/main/kotlin/dev/m1sk9/lunaticChat/paper/chat/channel/ChannelStorage.kt
@@ -4,14 +4,12 @@ import dev.m1sk9.lunaticChat.engine.chat.channel.ChannelData
 import dev.m1sk9.lunaticChat.engine.exception.ChannelStorageLoadException
 import dev.m1sk9.lunaticChat.engine.exception.ChannelStorageSaveException
 import dev.m1sk9.lunaticChat.paper.DebouncedSaver
+import dev.m1sk9.lunaticChat.paper.writeTextAtomically
 import kotlinx.serialization.json.Json
-import java.nio.file.Files
 import java.nio.file.Path
-import java.nio.file.StandardCopyOption
 import java.util.logging.Logger
 import kotlin.io.path.bufferedReader
 import kotlin.io.path.exists
-import kotlin.io.path.writeText
 
 /**
  * Manages the storage of channel data on disk.
@@ -68,14 +66,7 @@ class ChannelStorage(
     fun saveToDisk(data: ChannelData) {
         try {
             val jsonContent = json.encodeToString(ChannelData.serializer(), data)
-
-            // Written to a sibling and moved into place. Bukkit runs onDisable before cancelling
-            // scheduler tasks, so the shutdown save and a still-pending debounced save can reach
-            // this at the same time; two truncating writes to the same path would interleave and
-            // leave channels.json unparseable.
-            val temporaryFile = channelsFile.resolveSibling("${channelsFile.fileName}.tmp")
-            temporaryFile.writeText(jsonContent)
-            Files.move(temporaryFile, channelsFile, StandardCopyOption.REPLACE_EXISTING, StandardCopyOption.ATOMIC_MOVE)
+            channelsFile.writeTextAtomically(jsonContent)
             logger.fine("Successfully saved channels from ${channelsFile.fileName}.")
         } catch (e: Exception) {
             throw ChannelStorageSaveException(
diff --git a/platform-paper/src/main/kotlin/dev/m1sk9/lunaticChat/paper/converter/ConversionCache.kt b/platform-paper/src/main/kotlin/dev/m1sk9/lunaticChat/paper/converter/ConversionCache.kt
index fb96ba1..c550f09 100644
--- a/platform-paper/src/main/kotlin/dev/m1sk9/lunaticChat/paper/converter/ConversionCache.kt
+++ b/platform-paper/src/main/kotlin/dev/m1sk9/lunaticChat/paper/converter/ConversionCache.kt
@@ -1,6 +1,7 @@
 package dev.m1sk9.lunaticChat.paper.converter
 
 import dev.m1sk9.lunaticChat.engine.converter.CacheData
+import dev.m1sk9.lunaticChat.paper.writeTextAtomically
 import kotlinx.serialization.json.Json
 import java.nio.file.Path
 import java.util.concurrent.ConcurrentHashMap
@@ -8,7 +9,6 @@ import java.util.concurrent.atomic.AtomicBoolean
 import java.util.logging.Logger
 import kotlin.io.path.bufferedReader
 import kotlin.io.path.exists
-import kotlin.io.path.writeText
 
 class ConversionCache(
     private val cacheFile: Path,
@@ -55,7 +55,7 @@ class ConversionCache(
     private fun initializeEmptyCache() {
         val emptyData = CacheData(version = CACHE_VERSION, entries = emptyMap())
         val jsonBuffer = Json.encodeToString(CacheData.serializer(), emptyData)
-        cacheFile.writeText(jsonBuffer)
+        cacheFile.writeTextAtomically(jsonBuffer)
     }
 
     /**
@@ -89,6 +89,9 @@ class ConversionCache(
      *
      * Called from the periodic task and at shutdown. Skipping a clean cache matters because the
      * task fires on a fixed interval whether or not anyone chatted.
+     *
+     * A failed write leaves the previous file untouched: a cache that does not parse is discarded
+     * wholesale on the next boot, so a torn file costs every entry accumulated so far.
      */
     fun saveToDisk() {
         if (!dirty.getAndSet(false)) return
@@ -100,7 +103,7 @@ class ConversionCache(
                     entries = conversionMemoryCache.toMap(),
                 )
             val jsonBuffer = Json.encodeToString(CacheData.serializer(), data)
-            cacheFile.writeText(jsonBuffer)
+            cacheFile.writeTextAtomically(jsonBuffer)
             logger.info("Saved ${conversionMemoryCache.size} cache entries to disk.")
         } catch (e: Exception) {
             dirty.set(true)
diff --git a/platform-paper/src/main/kotlin/dev/m1sk9/lunaticChat/paper/settings/YamlPlayerSettingsStorage.kt b/platform-paper/src/main/kotlin/dev/m1sk9/lunaticChat/paper/settings/YamlPlayerSettingsStorage.kt
index f4d8a6f..6e28f6f 100644
--- a/platform-paper/src/main/kotlin/dev/m1sk9/lunaticChat/paper/settings/YamlPlayerSettingsStorage.kt
+++ b/platform-paper/src/main/kotlin/dev/m1sk9/lunaticChat/paper/settings/YamlPlayerSettingsStorage.kt
@@ -3,11 +3,11 @@ package dev.m1sk9.lunaticChat.paper.settings
 import com.charleskorn.kaml.Yaml
 import dev.m1sk9.lunaticChat.engine.settings.PlayerSettingsData
 import dev.m1sk9.lunaticChat.paper.DebouncedSaver
+import dev.m1sk9.lunaticChat.paper.writeTextAtomically
 import java.nio.file.Path
 import java.util.logging.Logger
 import kotlin.io.path.bufferedReader
 import kotlin.io.path.exists
-import kotlin.io.path.writeText
 
 /**
  * Handles YAML file I/O operations for player settings.
@@ -50,12 +50,15 @@ class YamlPlayerSettingsStorage(
      * Saves player settings to the YAML file synchronously.
      * This should only be called from async context or during shutdown.
      *
+     * A failed write leaves the previous file untouched: loading falls back to empty settings when
+     * the YAML does not parse, so a torn file would silently discard every player's settings.
+     *
      * @param data The settings data to save
      */
     fun saveToDisk(data: PlayerSettingsData) {
         try {
             val yamlContent = yaml.encodeToString(PlayerSettingsData.serializer(), data)
-            settingsFile.writeText(yamlContent)
+            settingsFile.writeTextAtomically(yamlContent)
             logger.fine("Saved player settings to disk")
         } catch (e: Exception) {
             logger.severe("Failed to save settings: ${e.message}")
diff --git a/platform-paper/src/test/kotlin/dev/m1sk9/lunaticChat/paper/AtomicWriteTest.kt b/platform-paper/src/test/kotlin/dev/m1sk9/lunaticChat/paper/AtomicWriteTest.kt
new file mode 100644
index 0000000..ea0c35f
--- /dev/null
+++ b/platform-paper/src/test/kotlin/dev/m1sk9/lunaticChat/paper/AtomicWriteTest.kt
@@ -0,0 +1,76 @@
+package dev.m1sk9.lunaticChat.paper
+
+import java.nio.file.Files
+import java.nio.file.Path
+import java.util.concurrent.ConcurrentLinkedQueue
+import java.util.concurrent.CyclicBarrier
+import kotlin.io.path.exists
+import kotlin.io.path.listDirectoryEntries
+import kotlin.io.path.readText
+import kotlin.io.path.writeText
+import kotlin.test.Test
+import kotlin.test.assertContains
+import kotlin.test.assertEquals
+import kotlin.test.assertTrue
+
+class AtomicWriteTest {
+    private fun withTemporaryDirectory(block: (Path) -> Unit) {
+        val directory = Files.createTempDirectory("atomic-write-test")
+        try {
+            block(directory)
+        } finally {
+            directory.toFile().deleteRecursively()
+        }
+    }
+
+    @Test
+    fun `writes the content and leaves no temporary file behind`() =
+        withTemporaryDirectory { directory ->
+            val target = directory.resolve("channels.json")
+
+            target.writeTextAtomically("""{"channels":[]}""")
+
+            assertEquals("""{"channels":[]}""", target.readText())
+            assertEquals(listOf(target), directory.listDirectoryEntries())
+        }
+
+    @Test
+    fun `replaces existing content`() =
+        withTemporaryDirectory { directory ->
+            val target = directory.resolve("settings.yml")
+            target.writeText("version: 1")
+
+            target.writeTextAtomically("version: 2")
+
+            assertEquals("version: 2", target.readText())
+        }
+
+    @Test
+    fun `concurrent writers each publish a whole file rather than colliding`() =
+        withTemporaryDirectory { directory ->
+            val target = directory.resolve("channels.json")
+            val writerCount = 8
+            val contents = (1..writerCount).map { "content-$it".repeat(4_000) }
+            val failures = ConcurrentLinkedQueue<Throwable>()
+            val barrier = CyclicBarrier(writerCount)
+
+            // A shutdown save and a still-pending debounced save can reach the same file at once.
+            // With a shared temporary path they interleave there instead, and the losing move fails
+            // with the temporary file already gone.
+            val writers =
+                contents.map { content ->
+                    Thread {
+                        barrier.await()
+                        runCatching { target.writeTextAtomically(content) }
+                            .onFailure { failures.add(it) }
+                    }
+                }
+            writers.forEach { it.start() }
+            writers.forEach { it.join() }
+
+            assertTrue(failures.isEmpty(), "writes failed: ${failures.map { it.toString() }}")
+            assertContains(contents, target.readText())
+            assertEquals(listOf(target), directory.listDirectoryEntries())
+            assertTrue(target.exists())
+        }
+}