From 8426c3528f832a9dad0427faa6a1c9353d5a975a Mon Sep 17 00:00:00 2001
From: Dai Ha <dai.ha@magnolia-cms.com>
Date: Fri, 4 Sep 2026 14:20:36 +0700
Subject: [PATCH] #316: pin the fail-toward-preserve rule on the late re-check,
 found by mutation

---
 .../fleet/session/SessionManagerTest.java     | 40 +++++++++++++++++++
 1 file changed, 40 insertions(+)

diff --git a/fleetd/src/test/java/dev/ltms/fleet/session/SessionManagerTest.java b/fleetd/src/test/java/dev/ltms/fleet/session/SessionManagerTest.java
index 1b3e0e5..77e1ff2 100644
--- a/fleetd/src/test/java/dev/ltms/fleet/session/SessionManagerTest.java
+++ b/fleetd/src/test/java/dev/ltms/fleet/session/SessionManagerTest.java
@@ -91,6 +91,8 @@ class SessionManagerTest {
          *  default) falls back to the plain {@link #dirty} flag, so every existing test using this
          *  fake keeps returning one fixed answer. */
         private final List<Boolean> dirtySequence = new java.util.concurrent.CopyOnWriteArrayList<>();
+        private int failHasUncommittedOnCall = -1;
+        private RuntimeException hasUncommittedCallFailure;
         private final java.util.concurrent.atomic.AtomicInteger hasUncommittedCalls =
                 new java.util.concurrent.atomic.AtomicInteger();
 
@@ -117,6 +119,17 @@ class SessionManagerTest {
             return this;
         }
 
+        /**
+         * Throw from {@code hasUncommitted} on one specific call only, counting from 0. The
+         * whole-double {@link #failHasUncommittedWith} cannot express fleetd #316's fail-safe
+         * case, which needs the pre-stop read to succeed and only the late read to fail.
+         */
+        RecordingWorktrees failHasUncommittedOnCall(int call, RuntimeException e) {
+            this.failHasUncommittedOnCall = call;
+            this.hasUncommittedCallFailure = e;
+            return this;
+        }
+
         RecordingWorktrees failRemoveFor(String worktreePath) {
             failRemoveFor.add(worktreePath);
             return this;
@@ -151,6 +164,9 @@ class SessionManagerTest {
             if (hasUncommittedFailure != null) {
                 throw hasUncommittedFailure;
             }
+            if (call == failHasUncommittedOnCall) {
+                throw hasUncommittedCallFailure;
+            }
             if (!dirtySequence.isEmpty()) {
                 return dirtySequence.get(Math.min(call, dirtySequence.size() - 1));
             }
@@ -1254,6 +1270,30 @@ class SessionManagerTest {
                 "the fix re-reads hasUncommitted exactly once more, immediately before removal");
     }
 
+    @Test
+    void releasePreservesAWorktreeWhoseLateRecheckCannotBeRead() {
+        // fleetd #316 invariant 1, which no test pinned when the fix landed: the late re-check
+        // fails toward PRESERVING. Found by mutation — flipping dirtyImmediatelyBeforeRemoval's
+        // catch from `return true` to `return false` turned the guard into a cause of the very
+        // data loss it was added to stop, and the whole suite stayed green. The pre-stop read
+        // succeeds and says clean (call 0); the read that authorises the removal throws (call 1).
+        FakeHerdr herdr = new FakeHerdr();
+        RecordingWorktrees worktrees = new RecordingWorktrees()
+                .dirtySequence(false)
+                .failHasUncommittedOnCall(1, new WorktreeException("git status exited 128"));
+        SessionManager sessions = sessionManager(herdr, worktrees);
+        MemberSession s = sessions.acquire("ltms-local", null, "/caller/proj", null,
+                new WorktreeRequest("cb-316c", null));
+
+        sessions.release(s.paneId());
+
+        assertTrue(worktrees.removeCalls().isEmpty(),
+                "a worktree whose state cannot be read immediately before removal must be kept: "
+                        + "preserving costs disk, deleting on a guess destroys work with no other copy");
+        assertEquals(2, worktrees.hasUncommittedCallCount(),
+                "the late re-check still runs — it is the throwing call, not a skipped one");
+    }
+
     @Test
     void releaseSnapshotsWorkFoundOnlyByTheLateRecheck() {
         // #316's second half: the pre-stop dirty=false means trySnapshot never ran for this