fix(persistence): bound character file coordination

README.md

@@ -37,7 +37,9 @@ File-level atomic replacement is **not** atomicity with Minecraft inventory/play
 
 Abandonment skips remaining queued writes and schedules cleanup for acknowledged, never-issued creations. Cleanup checks the original file identity/creation and modification attributes and exact bytes under the repository lock. Changed, replaced, corrupt, symlinked, busy or unidentifiable files are retained. Partial-save failure cleans only acknowledged creations and reports uncertain targets without deleting them. Shutdown rejection preserves records rather than inventing successful cleanup. Repeated cleanup cannot delete a later file. Restart never reconstructs cleanup authority from revision zero or scans records for automatic deletion/reissue.
 
-Thirteen craft-persistence tests cover these backend boundaries, including worker timing, partial/post-rename failures, ownership changes, restart, shutdown, claim/abandon races, and mutated/cancelled observer futures. This is **not native inventory delivery**: the controller still must contain vanilla consumption, validate current view/lifecycle/content and commit actual items. Filesystem locks coordinate cooperating writers; concurrent external filesystem mutation must be excluded/coordinated. File flush/atomic rename does not establish directory power-loss durability or Minecraft/plugin-save atomicity. Empty coordination `.lock` files remain after YAML cleanup; they must not be unlinked casually because that can split locking between writers. Their file-count growth still needs a bounded coordination design before gameplay registration.
+Thirteen craft-persistence tests cover these backend boundaries, including worker timing, partial/post-rename failures, ownership changes, restart, shutdown, claim/abandon races, and mutated/cancelled observer futures. This is **not native inventory delivery**: the controller still must contain vanilla consumption, validate current view/lifecycle/content and commit actual items. Filesystem locks coordinate cooperating writers; concurrent external filesystem mutation must be excluded/coordinated. File flush/atomic rename does not establish directory power-loss durability or Minecraft/plugin-save atomicity. All create/update/cleanup mutations now share one fail-fast `characters/.records.lock` coordination file, so abandoning new characters does not accumulate one lock file per ID. Read-only atomic-file loads remain unlocked. The file stays present but its OS lock is held only for an operation, not an administrator-unlock flag. Never unlink it while writers may use it. Old per-ID lock files are left intact; no new ones are created.
+
+**Upgrade boundary:** stop all older per-ID-protocol writers before opening the same data directory with this shared-lock implementation. Mixing versions or hot-reloading while old worker writes remain queued is unsafe. YAML format and record filenames are unchanged; no deployment or existing-server data migration has been performed. Contention between cooperating repository instances fails without mutation and can be retried, rather than blocking a server tick or deleting coordination files.
 
 ## Paper sheet and recipe definition
 

src/main/java/games/dmg/fantasystick/persistence/YamlCharacterRepository.java

@@ -58,7 +58,7 @@ public final class YamlCharacterRepository {
     Creation createTracked(CharacterRecord character) throws IOException {
         if (character.revision().signum() != 0) throw new IOException("New character revision must be zero");
         Creation[] created = new Creation[1];
-        withLock(character.id(), () -> {
+        withLock(() -> {
             if (Files.exists(file(character.id()), LinkOption.NOFOLLOW_LINKS)) {
                 throw new IOException("Character already exists: " + character.id());
             }
@@ -71,7 +71,7 @@ public final class YamlCharacterRepository {
         if (!character.revision().equals(expectedRevision.add(BigInteger.ONE))) {
             throw new IOException("Character update must advance exactly one revision");
         }
-        withLock(character.id(), () -> {
+        withLock(() -> {
             CharacterRecord current = load(character.id()).orElseThrow(() -> new IOException("Character not found"));
             if (!current.revision().equals(expectedRevision)) throw new IOException("Stale character revision");
             write(character);
@@ -81,7 +81,7 @@ public final class YamlCharacterRepository {
     /** Only the in-memory creator of a never-issued batch may request this cleanup. */
     boolean discardUnissued(Creation created) throws IOException {
         boolean[] removed = {false};
-        withLock(created.record.id(), () -> {
+        withLock(() -> {
             var path = file(created.record.id());
             if (Files.notExists(path, LinkOption.NOFOLLOW_LINKS)) {
                 removed[0] = true;
@@ -138,9 +138,10 @@ public final class YamlCharacterRepository {
         }
     }
 
-    private void withLock(UUID id, Operation operation) throws IOException {
+    /** Bounded coordination for all mutations; never mix with older per-ID writers in this directory. */
+    private void withLock(Operation operation) throws IOException {
         Files.createDirectories(directory);
-        Path lockPath = directory.resolve(id + ".lock");
+        Path lockPath = directory.resolve(".records.lock");
         try (var channel = FileChannel.open(lockPath, StandardOpenOption.CREATE, StandardOpenOption.WRITE,
                 LinkOption.NOFOLLOW_LINKS);
              var lock = channel.tryLock()) {

src/test/java/games/dmg/fantasystick/persistence/CraftPersistenceTest.java

@@ -267,17 +267,18 @@ class CraftPersistenceTest {
                 CharacterStorage.CraftCleanup cleanup;
                 if (scenario == 3) {
                     try (var channel = java.nio.channels.FileChannel.open(
-                            data.resolve("characters").resolve(protectedRecord.id() + ".lock"),
+                            data.resolve("characters").resolve(".records.lock"),
                             java.nio.file.StandardOpenOption.WRITE);
                          var lock = channel.lock()) {
                         assertTrue(lock.isValid());
                         cleanup = batch.abandon().join();
                     }
                 } else cleanup = batch.abandon().join();
-                assertEquals(Set.of(protectedRecord.id()), cleanup.retained());
-                assertEquals(Set.of(removable.id()), cleanup.removed());
+                assertEquals(scenario == 3 ? Set.of(protectedRecord.id(), removable.id())
+                    : Set.of(protectedRecord.id()), cleanup.retained());
+                assertEquals(scenario == 3 ? Set.of() : Set.of(removable.id()), cleanup.removed());
                 assertTrue(java.nio.file.Files.exists(file, java.nio.file.LinkOption.NOFOLLOW_LINKS));
-                assertTrue(storage.find(removable.id()).join().isEmpty());
+                assertEquals(scenario != 3, storage.find(removable.id()).join().isEmpty());
                 if (scenario == 2) {
                     assertTrue(java.nio.file.Files.isSymbolicLink(file));
                     assertTrue(java.nio.file.Files.isRegularFile(data.resolve("protected-target.yml")));

src/test/java/games/dmg/fantasystick/persistence/YamlCharacterRepositoryTest.java

@@ -43,6 +43,67 @@ class YamlCharacterRepositoryTest {
             original.loadout(), original.cooldowns(), original.dead(), original.deathId(), original.diedAt());
     }
 
+    @Test
+    void repeatedCreationAndAbandonmentDoNotAccumulatePerCharacterCoordinationFiles() throws Exception {
+        var repository = new YamlCharacterRepository(data);
+        for (int index = 0; index < 96; index++) {
+            var created = repository.createTracked(sample(UUID.randomUUID()));
+            assertTrue(repository.discardUnissued(created));
+        }
+        try (var paths = Files.list(data.resolve("characters"))) {
+            var remaining = paths.toList();
+            assertTrue(remaining.size() <= 1, "Abandoned crafts must not leave one coordination file per generated ID");
+            assertTrue(remaining.stream().allMatch(path -> path.getFileName().toString().endsWith(".lock")));
+        }
+        var legitimate = sample(UUID.randomUUID());
+        repository.create(legitimate);
+        assertEquals(legitimate, repository.load(legitimate.id()).orElseThrow());
+    }
+
+    @Test
+    void sharedCoordinationRefusesEveryMutationWhileBusyAndPermitsRetryAfterRelease() throws Exception {
+        var repository = new YamlCharacterRepository(data);
+        var original = sample(UUID.randomUUID());
+        var unissued = repository.createTracked(sample(UUID.randomUUID()));
+        var candidate = sample(UUID.randomUUID());
+        repository.create(original);
+        var update = rename(original, "Retry after contention", BigInteger.ONE);
+        try (var channel = java.nio.channels.FileChannel.open(data.resolve("characters/.records.lock"),
+                java.nio.file.StandardOpenOption.WRITE);
+             var lock = channel.lock()) {
+            assertTrue(lock.isValid());
+            assertThrows(IOException.class, () -> new YamlCharacterRepository(data).create(candidate));
+            assertThrows(IOException.class, () -> repository.replace(update, BigInteger.ZERO));
+            assertThrows(IOException.class, () -> repository.discardUnissued(unissued));
+            assertEquals(original, repository.load(original.id()).orElseThrow());
+            assertEquals(unissued.record, repository.load(unissued.record.id()).orElseThrow());
+            assertTrue(repository.load(candidate.id()).isEmpty());
+        }
+        repository.create(candidate);
+        repository.replace(update, BigInteger.ZERO);
+        assertTrue(repository.discardUnissued(unissued));
+        assertEquals(update, repository.load(original.id()).orElseThrow());
+    }
+
+    @Test
+    void leavesLegacyCoordinationFilesIntactAndRefusesASymlinkedSharedLock() throws Exception {
+        var repository = new YamlCharacterRepository(data);
+        var original = sample(UUID.randomUUID());
+        repository.create(original);
+        var legacy = data.resolve("characters").resolve(original.id() + ".lock");
+        Files.writeString(legacy, "Legacy coordination fixture");
+        repository.replace(rename(original, "Updated", BigInteger.ONE), BigInteger.ZERO);
+        assertEquals("Legacy coordination fixture", Files.readString(legacy));
+        var shared = data.resolve("characters/.records.lock");
+        Files.delete(shared); // Test setup only: no writer is holding this fixture lock.
+        var outside = data.resolve("outside-lock");
+        Files.writeString(outside, "Protected outside fixture");
+        Files.createSymbolicLink(shared, outside);
+        assertThrows(IOException.class, () -> repository.create(sample(UUID.randomUUID())));
+        assertEquals("Protected outside fixture", Files.readString(outside));
+        assertTrue(Files.isSymbolicLink(shared));
+    }
+
     @Test
     void updatesExactlyTheExpectedRevisionAndNeverOverwritesWithAStaleCopy() throws Exception {
         var repository = new YamlCharacterRepository(data);