fleetd #525: restore the logger level, not just the appender, in SessionManagerTest

The finally block in onTurnFailedIsLoggedAtWarnWithThePriorState (and four other
tests in this file) called setLevel(Level.WARN) on the shared SessionManager
logger (one test used MemberRegistry.class) but only detached the appender in
finally, never restoring the level. Since logback Logger instances are cached
per class and shared across the whole JVM, the pinned level leaked into every
test that ran after it.

Add CapturedLog, a small AutoCloseable that captures a logger's level and
appender together and restores both on close via try-with-resources, so this
shape cannot be half-fixed again. Convert all 7 addAppender/setLevel call
sites in this file to it, including the 2 sites #522 already fixed locally
(kept per-test pinning as belt-and-braces).

Add a proving test (sharedSessionManagerLoggerLevelIsRestoredAfterOnTurnFailedPinsWarn,
@Order(2), running right after the fixed test at @Order(1)) that fails before
this fix and passes after it. Verified with a mutation: dropping the level
restore in CapturedLog.close() turns the proving test red with
'expected: <DEBUG> but was: <WARN>'; restoring is byte-identical to the
pre-mutation file (sha256 matched) and the suite goes green again.

mvn -f fleetd/pom.xml clean install: Tests run: 1699, Failures: 0, Errors: 0,
Skipped: 0. BUILD SUCCESS.

fleetd/src/main/java/dev/ltms/fleet/mcp/FleetMcp.java

@@ -527,14 +527,28 @@ public final class FleetMcp {
     }
 
     /**
-     * Pre-CB-501 identity: worker if the connection maps to a pane, otherwise the primary. Used
-     * only by the legacy constructor, where authorization is not enforced anyway.
+     * Pre-CB-501 identity: worker if the connection maps to a pane, otherwise anonymous. Used
+     * only by the legacy constructor ({@code callers == null}), where authorization is not
+     * enforced anyway — but the resolved {@link Principal} still reaches non-authz logic (e.g.
+     * {@code markSpawnedMemberPresent}, {@code recordPrimarySingleton}), so it must not be trusted
+     * with a role it did not earn.
+     *
+     * <p>fleetd #509: this used to fall back to {@link Principal#primary}, unconditionally, for
+     * every caller the connection did not resolve to a worker pane — with none of
+     * {@code CallerResolver.java:254}'s two guards ({@code isLoopback}, {@code scanComplete}).
+     * That is the exact shape #317 and #505 each closed on the enforced path; this branch was the
+     * same trap, left open on the legacy one. It now returns {@link Principal#anonymous} instead,
+     * so an unresolved legacy caller earns no authority rather than the primary's.
+     *
+     * <p>Package-private (was {@code private}) so this is unit-testable directly, the same reason
+     * {@link #denyFor} was split out — it runs inside a contextExtractor closure that only fires on
+     * a real MCP request, so nothing else could pin this behaviour.
      */
-    private static Principal legacyPrincipal(ConnectionIdentity identity, String addr, int port) {
+    static Principal legacyPrincipal(ConnectionIdentity identity, String addr, int port) {
         ConnectionIdentity.Caller c = identity.resolve(addr, port);
         return c.terminal() != null
                 ? Principal.worker(c.terminal(), c.pid())
-                : Principal.primary(c.pid());
+                : Principal.anonymous();
     }
 
     /** The caller reconstructed from the transport context. */

fleetd/src/main/java/dev/ltms/fleet/session/SessionManager.java

@@ -302,9 +302,10 @@ public final class SessionManager implements TurnListener {
      * with no copy and no error. Do NOT fuse these back together; the cost of an orphaned worktree
      * is a logged path an operator can reclaim, the cost of a deleted one is unrecoverable work.
      */
-    private void release(String paneId, ReleaseCause cause) {
+    private MemberSession release(String paneId, ReleaseCause cause) {
         MemberSession removed = registry.remove(paneId);
         releaseRemoved(paneId, removed, handles.remove(paneId), cause);
+        return removed;
     }
 
     /**
@@ -1064,16 +1065,39 @@ public final class SessionManager implements TurnListener {
      * drain (see above), and a straggler must not buy the drain more time than the flag it lost the
      * race against would have. In the ordinary case the sweep finds nothing and costs one empty
      * {@link #roster()} call.
+     *
+     * <p>fleetd #512: a drain that releases every session cleanly used to log nothing at all — the
+     * only log calls in this method and {@link #drainSnapshot} sit on abnormal paths, so "nothing
+     * logged" was indistinguishable from "died on the first session". The {@code log.info} at the
+     * end below is a positive assertion that the drain actually finished, on the normal path,
+     * every time — including the all-zero case, which is a common and legitimate outcome (no
+     * members were live) and must still produce the line. Both {@link #drainSnapshot} passes (the
+     * main snapshot and the straggler sweep) are folded into the one line: a caller reading two
+     * lines could not tell a two-pass drain from two separate drains.
      */
     void drainAll(long timeoutNanos) {
         long deadline = System.nanoTime() + timeoutNanos;
         draining.set(true);
-        drainSnapshot(roster(), deadline);
+        DrainTally tally = drainSnapshot(roster(), deadline);
         List<MemberSession> stragglers = roster();
         if (!stragglers.isEmpty()) {
             log.warn("drain sweep found {} session(s) registered after the drain snapshot was "
                     + "taken (raced past the shutdown guard); draining them too", stragglers.size());
-            drainSnapshot(stragglers, deadline);
+            tally = tally.plus(drainSnapshot(stragglers, deadline));
+        }
+        log.info("drain complete: released={} abandoned={} (still BUSY at the shutdown deadline)",
+                tally.released(), tally.abandoned());
+    }
+
+    /**
+     * Running count for one {@link #drainAll} invocation, folded across both {@link #drainSnapshot}
+     * passes (fleetd #512). {@code abandoned} counts sessions that were still {@code BUSY} at the
+     * moment they were released — i.e. the whole-drain deadline passed before they left {@code BUSY}
+     * on their own (see {@link #drainSnapshot}) — a subset of {@code released}, not additional to it.
+     */
+    private record DrainTally(int released, int abandoned) {
+        private DrainTally plus(DrainTally other) {
+            return new DrainTally(released + other.released, abandoned + other.abandoned);
         }
     }
 
@@ -1081,8 +1105,12 @@ public final class SessionManager implements TurnListener {
      * Drain exactly the sessions in {@code snapshot}, waiting out a {@code BUSY} one against the
      * shared whole-drain {@code deadline} before releasing it. Shared by {@link #drainAll}'s main
      * pass and its post-loop straggler sweep (fleetd #308) so both honor the same one budget.
+     * Returns how many sessions this pass released, and how many of those were still {@code BUSY}
+     * (abandoned mid-turn) at the moment of release.
      */
-    private void drainSnapshot(List<MemberSession> snapshot, long deadline) {
+    private DrainTally drainSnapshot(List<MemberSession> snapshot, long deadline) {
+        int released = 0;
+        int abandoned = 0;
         for (MemberSession s : snapshot) {
             try {
                 if (s.state() == MemberSession.State.BUSY) {
@@ -1100,11 +1128,16 @@ public final class SessionManager implements TurnListener {
                         }
                     }
                 }
-                release(s.paneId(), ReleaseCause.SHUTDOWN);
+                MemberSession removed = release(s.paneId(), ReleaseCause.SHUTDOWN);
+                released++;
+                if (removed != null && removed.state() == MemberSession.State.BUSY) {
+                    abandoned++;
+                }
             } catch (RuntimeException e) {
                 log.warn("drain failed for pane={}; continuing with remaining sessions", s.paneId(), e);
             }
         }
+        return new DrainTally(released, abandoned);
     }
 
     /**

fleetd/src/test/java/dev/ltms/fleet/herdr/PaneLocatorTest.java

@@ -180,6 +180,32 @@ class PaneLocatorTest {
         assertTrue(outcome.complete(), "a positive match elsewhere in the scan is definitive");
     }
 
+    // --- fleetd #509: the completeness fold across clients must not collapse to "last wins" ----
+
+    @Test
+    void anEarlierClientsErrorSurvivesALaterClientsCleanNegative() {
+        // terminalForPid folds each client's Lookup.complete() with
+        //     complete = complete && outcome.complete();
+        // (PaneLocator.java:117). With a SINGLE client, a fold that keeps only the last outcome
+        // (dropping the "complete &&" prefix) agrees with the real fold — which is why 14 of the
+        // 15 pre-existing tests never catch that mutation: none of them vary the number of clients.
+        // Here the LEAD client errors on exactly the pane that would have owned the pid (so its
+        // scan is incomplete AND finds no match), and the MEMBER client cleanly reports no panes
+        // at all (a complete, negative scan). The real fold ANDs the two into false. A fold that
+        // just keeps the last client's outcome would read this as a clean true — the earlier
+        // error is erased, and CallerResolver.java:254 would read scanComplete() as true and
+        // promote an unverified caller to the primary.
+        HerdrClient lead = new FakeHerdr().processInfoFailsForPane("w2:p7", "transient");
+        HerdrClient member = new FakeHerdr().withNoPanes();
+        PaneLocator two = new PaneLocator(lead, member);
+
+        PaneLocator.Lookup outcome = two.terminalForPid(FakeHerdr.WORKER_PID);
+
+        assertNull(outcome.terminal(), "the pane that could have owned the pid was never checked");
+        assertFalse(outcome.complete(),
+                "an earlier client's error must survive a later client's clean negative");
+    }
+
     /** Minimal single-pane {@link HerdrClient} fake, purpose-built for the ancestry tests above. */
     private static final class OnePaneHerdr implements HerdrClient {
         private final ObjectMapper mapper = new ObjectMapper();

fleetd/src/test/java/dev/ltms/fleet/mcp/FleetMcpAuthzTest.java

@@ -190,6 +190,27 @@ class FleetMcpAuthzTest {
                 "no CallerResolver supplied ⇒ authorization not enforced (legacy behaviour)");
     }
 
+    /**
+     * fleetd #509: {@code legacyPrincipal} (used only when {@code callers == null}, i.e. the
+     * legacy constructor above) used to fall back to {@link Principal#primary} for ANY caller the
+     * connection did not resolve to a worker pane — no {@code isLoopback} check, no
+     * {@code scanComplete} check, unlike the enforced path's {@code CallerResolver.java:254}. A
+     * non-loopback caller (an off-host client) is exactly the case that must never earn the
+     * primary's authority, and authorization being disabled in legacy mode does not make that
+     * safe: the resolved {@link Principal} still reaches non-authz logic such as
+     * {@code markSpawnedMemberPresent} and {@code recordPrimarySingleton}.
+     */
+    @Test
+    void legacyPrincipalIsAnonymousNotPrimaryForAnUnresolvedCaller() {
+        ConnectionIdentity identity = new ConnectionIdentity(new PaneLocator(herdr), _ -> 999_999);
+        // A non-loopback address never even reaches the pane scan — resolve() short-circuits it
+        // to Caller(null, -1, true), the same "no terminal" shape a genuine primary's connection
+        // produces. legacyPrincipal must not conflate the two.
+        Principal p = FleetMcp.legacyPrincipal(identity, "8.8.8.8", 1234);
+        assertEquals(Principal.anonymous(), p,
+                "an unresolved legacy caller must earn no authority, not the primary's");
+    }
+
     // --- fleetd #439: who may see fleet_list's coordinator row ----------------------------------
 
     /**

fleetd/src/test/java/dev/ltms/fleet/session/SessionManagerTest.java

@@ -25,7 +25,12 @@ import dev.ltms.fleet.peer.SpawnRequest;
 import dev.ltms.fleet.placement.BackendQuarantine;
 import dev.ltms.fleet.placement.PlacementDecision;
 import dev.ltms.fleet.placement.PlacementPolicies;
+import org.junit.jupiter.api.AfterAll;
+import org.junit.jupiter.api.BeforeAll;
+import org.junit.jupiter.api.MethodOrderer;
+import org.junit.jupiter.api.Order;
 import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.TestMethodOrder;
 import org.junit.jupiter.api.io.TempDir;
 import org.slf4j.LoggerFactory;
 
@@ -50,9 +55,46 @@ import static org.junit.jupiter.api.Assertions.*;
  * CB-301 / CB-303 acceptance tests for the authoritative session registry, one-shot lifecycle FSM,
  * and configurable lifecycle limits (idle TTL, context cap, drain).
  * No live herdr — everything runs against the same {@link FakeHerdr} the rest of the project uses.
+ *
+ * <p>fleetd #525: only {@link #onTurnFailedIsLoggedAtWarnWithThePriorState} (explicitly
+ * {@link Order#value() @Order(1)}) and the proving test right after it
+ * ({@link #sharedSessionManagerLoggerLevelIsRestoredAfterOnTurnFailedPinsWarn}, {@code @Order(2)})
+ * care about method order — every other test here has no {@code @Order} and so runs after both of
+ * these (JUnit 5's {@link MethodOrderer.OrderAnnotation} gives an unannotated method the lowest
+ * priority), in whatever relative order it already ran in.
  */
+@TestMethodOrder(MethodOrderer.OrderAnnotation.class)
 class SessionManagerTest {
 
+    /**
+     * fleetd #525: the level {@link SessionManager}'s logger had when this class started, captured
+     * before any test here — including the leak this ticket fixes — can touch it. {@code
+     * pinSessionManagerLoggerToAKnownBaseline} then forces a distinctive, known value (DEBUG) so
+     * {@link #sharedSessionManagerLoggerLevelIsRestoredAfterOnTurnFailedPinsWarn} can tell "the
+     * level came back to what it was" apart from "the level happens to already be WARN because
+     * some earlier test class in this JVM fork (surefire reuses forks by default) left it there" —
+     * a real risk, since {@code ch.qos.logback.classic.Logger} instances are cached per class and
+     * shared across the whole JVM, and this exact logger is also touched by
+     * {@code WorktreeSessionManagerTest#releasePreservesDirtyWorktreeAndLogsWarn}, which has the
+     * same unfixed leak (reported, not fixed — out of this ticket's scope).
+     */
+    private static Level sessionManagerLevelBeforeThisClass;
+
+    @BeforeAll
+    static void pinSessionManagerLoggerToAKnownBaseline() {
+        ch.qos.logback.classic.Logger sessionLog =
+                (ch.qos.logback.classic.Logger) LoggerFactory.getLogger(SessionManager.class);
+        sessionManagerLevelBeforeThisClass = sessionLog.getLevel();
+        sessionLog.setLevel(Level.DEBUG);
+    }
+
+    @AfterAll
+    static void restoreSessionManagerLoggerLevel() {
+        ch.qos.logback.classic.Logger sessionLog =
+                (ch.qos.logback.classic.Logger) LoggerFactory.getLogger(SessionManager.class);
+        sessionLog.setLevel(sessionManagerLevelBeforeThisClass);
+    }
+
     private SessionManager sessionManager(FakeHerdr herdr) {
         FleetConfig.Profile cfg = new FleetConfig.Profile(
                 "ltms-local", "http://gx00.gw:8000", "coder", null, "FLEETD_WORKER_TOKEN",
@@ -81,6 +123,55 @@ class SessionManagerTest {
         return new SessionManager(workers, worktrees, clock);
     }
 
+    /**
+     * fleetd #525: captures a logger's output and, on {@link #close}, restores <em>both</em> the
+     * appender and the level to what they were before. A bare {@code addAppender}/{@code
+     * setLevel} pair whose {@code finally} only detaches the appender leaves the level pinned —
+     * {@code ch.qos.logback.classic.Logger} instances are cached per class and shared across the
+     * whole JVM, so a level set by one test in this class is still in effect for every test that
+     * runs after it, in this class or any other. try-with-resources makes "restored the appender
+     * but not the level" impossible to write, because there is only one thing to close.
+     */
+    private static final class CapturedLog implements AutoCloseable {
+        private final ch.qos.logback.classic.Logger logger;
+        private final Level originalLevel;
+        private final ListAppender<ILoggingEvent> appender;
+
+        private CapturedLog(Class<?> loggerClass, Level pinnedLevel) {
+            LoggerContext ctx = (LoggerContext) LoggerFactory.getILoggerFactory();
+            this.logger = (ch.qos.logback.classic.Logger) LoggerFactory.getLogger(loggerClass);
+            this.originalLevel = logger.getLevel();
+            this.appender = new ListAppender<>();
+            appender.setContext(ctx);
+            appender.start();
+            logger.addAppender(appender);
+            if (pinnedLevel != null) {
+                logger.setLevel(pinnedLevel);
+            }
+        }
+
+        /** Capture {@code loggerClass}'s output, pinning its level to {@code pinnedLevel} for the
+         *  duration of the try-with-resources block. */
+        static CapturedLog at(Class<?> loggerClass, Level pinnedLevel) {
+            return new CapturedLog(loggerClass, pinnedLevel);
+        }
+
+        /** Capture {@code loggerClass}'s output without changing its level. */
+        static CapturedLog of(Class<?> loggerClass) {
+            return new CapturedLog(loggerClass, null);
+        }
+
+        List<ILoggingEvent> events() {
+            return appender.list;
+        }
+
+        @Override
+        public void close() {
+            logger.detachAppender(appender);
+            logger.setLevel(originalLevel);
+        }
+    }
+
     /**
      * CB-581: a {@link Worktrees} test double whose {@code hasUncommitted} and {@code remove} can
      * be told to throw, so {@link SessionManager#release} can be exercised against exactly the
@@ -466,24 +557,15 @@ class SessionManagerTest {
 
     @Test
     void backendErrorForUnknownTargetIsWarnedAndDoesNotCreateASession() {
-        LoggerContext ctx = (LoggerContext) LoggerFactory.getILoggerFactory();
-        ch.qos.logback.classic.Logger sessionLog =
-                (ch.qos.logback.classic.Logger) LoggerFactory.getLogger(SessionManager.class);
-        ListAppender<ILoggingEvent> appender = new ListAppender<>();
-        appender.setContext(ctx);
-        appender.start();
-        sessionLog.addAppender(appender);
-        try {
+        try (CapturedLog log = CapturedLog.of(SessionManager.class)) {
             SessionManager sessions = sessionManager(new FakeHerdr());
 
             assertFalse(sessions.onBackendError("term_missing", "backend exited"));
 
             assertTrue(sessions.roster().isEmpty(), "unknown target must not create a session");
-            assertTrue(appender.list.stream().anyMatch(e -> e.getLevel().equals(Level.WARN)
+            assertTrue(log.events().stream().anyMatch(e -> e.getLevel().equals(Level.WARN)
                             && e.getFormattedMessage().contains("term_missing")),
                     "unknown target is logged at WARN");
-        } finally {
-            sessionLog.detachAppender(appender);
         }
     }
 
@@ -516,19 +598,12 @@ class SessionManagerTest {
     }
 
     @Test
+    @Order(1)
     void onTurnFailedIsLoggedAtWarnWithThePriorState() {
         // CB-564: this transition used to be a bare DEBUG "session marked failed" — a symptom with no
         // cause. A member that can no longer be delegated to must be at least WARN, and should name
         // what stage it failed at (here: BUSY, i.e. a turn was in flight and never resolved).
-        LoggerContext ctx = (LoggerContext) LoggerFactory.getILoggerFactory();
-        ch.qos.logback.classic.Logger sessionLog =
-                (ch.qos.logback.classic.Logger) LoggerFactory.getLogger(SessionManager.class);
-        ListAppender<ILoggingEvent> appender = new ListAppender<>();
-        appender.setContext(ctx);
-        appender.start();
-        sessionLog.addAppender(appender);
-        sessionLog.setLevel(Level.WARN);
-        try {
+        try (CapturedLog log = CapturedLog.at(SessionManager.class, Level.WARN)) {
             FakeHerdr herdr = new FakeHerdr();
             SessionManager sessions = sessionManager(herdr);
             MemberSession session = sessions.acquire("ltms-local", null, "/caller", "term_primary");
@@ -538,18 +613,37 @@ class SessionManagerTest {
 
             sessions.onTurnFailed(terminal);
 
-            String warn = appender.list.stream()
+            String warn = log.events().stream()
                     .filter(e -> e.getLevel().equals(Level.WARN))
                     .map(ILoggingEvent::getFormattedMessage)
                     .findFirst()
                     .orElse("no turn-failed WARN logged");
             assertTrue(warn.contains(terminal), "the log names the member: " + warn);
             assertTrue(warn.contains("BUSY"), "the log names the stage it failed at: " + warn);
-        } finally {
-            sessionLog.detachAppender(appender);
         }
     }
 
+    /**
+     * fleetd #525: proves the leak in {@link #onTurnFailedIsLoggedAtWarnWithThePriorState} above
+     * (which runs immediately before this, via {@code @Order}) is closed. That test pins the
+     * shared {@link SessionManager} logger to WARN through a {@link CapturedLog}; if {@link
+     * CapturedLog#close} only detached the appender — the original bug, before this ticket's fix —
+     * the level would still read WARN here instead of the {@code DEBUG} baseline this class's
+     * {@code @BeforeAll} set. Runs at {@code @Order(2)}, guaranteed after {@code @Order(1)} and
+     * before every other (unannotated) test in this class.
+     */
+    @Test
+    @Order(2)
+    void sharedSessionManagerLoggerLevelIsRestoredAfterOnTurnFailedPinsWarn() {
+        ch.qos.logback.classic.Logger sessionLog =
+                (ch.qos.logback.classic.Logger) LoggerFactory.getLogger(SessionManager.class);
+        assertEquals(Level.DEBUG, sessionLog.getLevel(),
+                "onTurnFailedIsLoggedAtWarnWithThePriorState pins the shared SessionManager logger "
+                        + "to WARN; its cleanup must restore the level it captured (DEBUG, set by "
+                        + "this class's @BeforeAll) rather than leaving WARN pinned for every test "
+                        + "that runs after it");
+    }
+
     /**
      * fleetd #226: a contended slot is refused through the real {@link SessionManager#acquire}
      * path before the real launcher can hand an architect charter to a process.
@@ -576,28 +670,18 @@ class SessionManagerTest {
         SessionManager sessions = sessionManager(herdr);
         MemberRegistry members = architectRegistry();
         sessions.setMemberLifecycle(bindFailureAfterReservation(members));
-        LoggerContext ctx = (LoggerContext) LoggerFactory.getILoggerFactory();
-        ch.qos.logback.classic.Logger registryLog = (ch.qos.logback.classic.Logger)
-                LoggerFactory.getLogger(MemberRegistry.class);
-        ListAppender<ILoggingEvent> appender = new ListAppender<>();
-        appender.setContext(ctx);
-        appender.start();
-        registryLog.addAppender(appender);
-        registryLog.setLevel(Level.WARN);
-        try {
+        try (CapturedLog log = CapturedLog.at(MemberRegistry.class, Level.WARN)) {
             MemberSession session = sessions.acquire("ltms-local", MemberRole.ARCHITECT, null,
                     "/caller", "term_primary", null);
 
             assertEquals(MemberRole.DEV, session.role(), "a failed reservation bind must use the fallback");
-            String warn = appender.list.stream()
+            String warn = log.events().stream()
                     .filter(e -> e.getLevel().equals(Level.WARN))
                     .map(ILoggingEvent::getFormattedMessage)
                     .findFirst()
                     .orElse("no slot-exhaustion WARN logged");
             assertTrue(warn.contains("ltms-local"), "the WARN names the profile: " + warn);
             assertTrue(warn.contains(session.terminalId()), "the WARN names the terminal: " + warn);
-        } finally {
-            registryLog.detachAppender(appender);
         }
     }
 
@@ -902,6 +986,82 @@ class SessionManagerTest {
                 .count();
     }
 
+    /**
+     * fleetd #512: a drain that releases every session cleanly used to log nothing at all — the
+     * two log calls in {@code drainAll}/{@code drainSnapshot} both sit on abnormal paths, so
+     * "clean drain" and "died on the first session" were indistinguishable. This asserts the new
+     * {@code log.info} line fires on the ordinary, nothing-went-wrong path, and that its numbers
+     * are the real counts (two released, zero abandoned) rather than just a non-empty string.
+     */
+    @Test
+    void drainAllLogsACompletionLineWithTheRealCountsOnACleanDrain() {
+        FakeHerdr herdr = new FakeHerdr();
+        SessionManager sessions = sessionManager(herdr);
+        MemberSession first = sessions.acquire("ltms-local", "/one", "/caller", "ownerOne");
+        MemberSession second = sessions.acquire("ltms-local", "/two", "/caller", "ownerTwo");
+        sessions.asPresence().markPresent(first.terminalId());
+        sessions.asPresence().markPresent(second.terminalId());
+        // Both stay READY — neither is delivered a turn, so neither is BUSY and the drain below
+        // has nothing abnormal to hit.
+
+        // Pin INFO explicitly: fleetd #525 made CapturedLog itself restore the level it pins, but
+        // this pin stays anyway as belt-and-braces — a later change to the sweep must not be able
+        // to make this INFO assertion vacuous again by leaving some other test's WARN pin in place.
+        try (CapturedLog log = CapturedLog.at(SessionManager.class, Level.INFO)) {
+            sessions.drainAll(TimeUnit.MILLISECONDS.toNanos(100));
+
+            assertTrue(sessions.roster().isEmpty(), "precondition: the drain actually ran");
+            String info = log.events().stream()
+                    .filter(e -> e.getLevel().equals(Level.INFO))
+                    .map(ILoggingEvent::getFormattedMessage)
+                    .filter(m -> m.contains("drain complete"))
+                    .findFirst()
+                    .orElse("no drain-complete INFO logged");
+            assertTrue(info.contains("released=2"),
+                    "both released sessions must be counted: " + info);
+            assertTrue(info.contains("abandoned=0"),
+                    "neither session was BUSY, so nothing was abandoned mid-turn: " + info);
+        }
+    }
+
+    /**
+     * fleetd #512: the same completion line must also report a non-zero abandoned count when a
+     * session is still {@code BUSY} once the whole-drain deadline passes — the case the ticket
+     * calls out as the one a script needs to be able to see. Reuses the same BUSY/READY mix as
+     * {@link #drainAllReleasesBusyAndReadySessionsAndWaitsForBusy}, which already forces the busy
+     * session to spin until the real-time deadline expires (its state never leaves BUSY on its
+     * own), and adds the log assertion that test does not make.
+     */
+    @Test
+    void drainAllLogsANonZeroAbandonedCountForASessionStillBusyAtTheDeadline() {
+        FakeHerdr herdr = new FakeHerdr();
+        SessionManager sessions = sessionManager(herdr);
+        MemberSession ready = sessions.acquire("ltms-local", "/ready", "/caller", "ownerR");
+        MemberSession busy = sessions.acquire("ltms-local", "/busy", "/caller", "ownerB");
+        sessions.asPresence().markPresent(ready.terminalId());
+        sessions.asPresence().markPresent(busy.terminalId());
+        sessions.onDelivered(busy.terminalId(), TestTurnTokens.inert(busy.terminalId()));
+        // busy never leaves BUSY — no completion is delivered — so the drain below must spin the
+        // full timeout and then release it anyway, counting it abandoned.
+
+        // Pin INFO explicitly — see the comment in drainAllLogsACompletionLineWithTheRealCountsOnACleanDrain.
+        try (CapturedLog log = CapturedLog.at(SessionManager.class, Level.INFO)) {
+            sessions.drainAll(TimeUnit.MILLISECONDS.toNanos(100));
+
+            assertTrue(sessions.roster().isEmpty(), "precondition: the drain actually ran");
+            String info = log.events().stream()
+                    .filter(e -> e.getLevel().equals(Level.INFO))
+                    .map(ILoggingEvent::getFormattedMessage)
+                    .filter(m -> m.contains("drain complete"))
+                    .findFirst()
+                    .orElse("no drain-complete INFO logged");
+            assertTrue(info.contains("released=2"),
+                    "both the ready and the busy session are released: " + info);
+            assertTrue(info.contains("abandoned=1"),
+                    "the busy session hit the deadline still BUSY and must be counted: " + info);
+        }
+    }
+
     // --- fleetd #308: a spawn accepted while the shutdown drain is running must not orphan ---
 
     @Test
@@ -1202,21 +1362,13 @@ class SessionManagerTest {
                 new WorktreeRequest("cb-581a", null));
         worktrees.failHasUncommittedWith(new WorktreeException("git status exited 128"));
 
-        LoggerContext ctx = (LoggerContext) LoggerFactory.getILoggerFactory();
-        ch.qos.logback.classic.Logger sessionLog =
-                (ch.qos.logback.classic.Logger) LoggerFactory.getLogger(SessionManager.class);
-        ListAppender<ILoggingEvent> appender = new ListAppender<>();
-        appender.setContext(ctx);
-        appender.start();
-        sessionLog.addAppender(appender);
-        sessionLog.setLevel(Level.WARN);
-        try {
+        try (CapturedLog log = CapturedLog.at(SessionManager.class, Level.WARN)) {
             assertDoesNotThrow(() -> sessions.release(s.paneId()),
                     "a throwing dirty check must not abort the release");
 
             assertTrue(worktrees.removeCalls().isEmpty(),
                     "the worktree is preserved when its dirty state cannot be determined");
-            String warn = appender.list.stream()
+            String warn = log.events().stream()
                     .filter(e -> e.getLevel().equals(Level.WARN))
                     .map(ILoggingEvent::getFormattedMessage)
                     .filter(m -> m.contains(s.worktree()))
@@ -1224,8 +1376,6 @@ class SessionManagerTest {
                     .orElse("no warn logged naming the worktree");
             assertTrue(warn.contains(s.paneId()), "the WARN names the pane: " + warn);
             assertTrue(warn.contains(s.terminalId()), "the WARN names the terminal: " + warn);
-        } finally {
-            sessionLog.detachAppender(appender);
         }
     }
 
@@ -1392,20 +1542,12 @@ class SessionManagerTest {
         // this itself, so it no longer propagates out of release() at all.
         worktrees.failRemoveFor(b.worktree());
 
-        LoggerContext ctx = (LoggerContext) LoggerFactory.getILoggerFactory();
-        ch.qos.logback.classic.Logger sessionLog =
-                (ch.qos.logback.classic.Logger) LoggerFactory.getLogger(SessionManager.class);
-        ListAppender<ILoggingEvent> appender = new ListAppender<>();
-        appender.setContext(ctx);
-        appender.start();
-        sessionLog.addAppender(appender);
-        sessionLog.setLevel(Level.WARN);
         int reaped;
-        try {
+        try (CapturedLog log = CapturedLog.at(SessionManager.class, Level.WARN)) {
             clock[0] = 100;
             reaped = sessions.reapIdle(10);
 
-            String warn = appender.list.stream()
+            String warn = log.events().stream()
                     .filter(e -> e.getLevel().equals(Level.WARN))
                     .map(ILoggingEvent::getFormattedMessage)
                     .filter(m -> m.contains(b.paneId()))
@@ -1413,8 +1555,6 @@ class SessionManagerTest {
                     .orElse("no worktree-removal-failure WARN logged");
             assertTrue(warn.contains(b.terminalId()), "the WARN names the failed session's terminal: " + warn);
             assertTrue(warn.contains(b.worktree()), "the WARN names the failed session's worktree: " + warn);
-        } finally {
-            sessionLog.detachAppender(appender);
         }
 
         assertEquals(3, reaped,
@@ -1460,28 +1600,18 @@ class SessionManagerTest {
         // trigger reapIdle's own guard is for, now that #283 closed the worktree-removal trigger.
         herdr.paneCloseFailsForPane("w9:pRoot_2", "internal_error");
 
-        LoggerContext ctx = (LoggerContext) LoggerFactory.getILoggerFactory();
-        ch.qos.logback.classic.Logger sessionLog =
-                (ch.qos.logback.classic.Logger) LoggerFactory.getLogger(SessionManager.class);
-        ListAppender<ILoggingEvent> appender = new ListAppender<>();
-        appender.setContext(ctx);
-        appender.start();
-        sessionLog.addAppender(appender);
-        sessionLog.setLevel(Level.WARN);
         int reaped;
-        try {
+        try (CapturedLog log = CapturedLog.at(SessionManager.class, Level.WARN)) {
             clock[0] = 100;
             reaped = sessions.reapIdle(10);
 
-            String warn = appender.list.stream()
+            String warn = log.events().stream()
                     .filter(e -> e.getLevel().equals(Level.WARN))
                     .map(ILoggingEvent::getFormattedMessage)
                     .filter(m -> m.contains("reap failed") && m.contains(b.paneId()))
                     .findFirst()
                     .orElse("no reap-failed WARN logged for the failing session");
             assertTrue(warn.contains(b.terminalId()), "the WARN names the failed session's terminal: " + warn);
-        } finally {
-            sessionLog.detachAppender(appender);
         }
 
         assertEquals(2, reaped,

scripts/redeploy-fleetd.sh

@@ -452,6 +452,35 @@ classify_amqp_connection_errors() {
   REDEPLOY_UNEXPLAINED_ERRORS=$((REDEPLOY_UNEXPLAINED_ERRORS + pending_inbox + pending_lead_mailbox))
 }
 
+# fleetd #517: extracted so the suite can call this decision directly, the same way #510 extracted
+# wait_for_daemon_exit so its ordering became checkable. Before this, the only test of the drain-gate
+# abort message was a grep of this script's own source for the wording — so mutating the `if` below
+# to `if false` (making the branch unreachable) left every test green, because the wording was still
+# sitting in the file. Pure: only decides which message applies and prints it, no side effects, so a
+# test can call it directly with an in-memory staged path instead of driving the real drain-gate flow
+# (which needs a live $OLD_PID and an interactive prompt neither test can supply).
+#
+# The four cases:
+#   build ran,  staged jar present -> names the staged jar and how to finish or discard it
+#   build ran,  staged jar absent  -> "nothing changed" (nothing was staged this run either)
+#   --no-build, staged jar present -> ALSO "nothing changed", deliberately: --no-build itself builds
+#     and stages nothing (see require_no_build_jar above), so a staged jar found here is a leftover
+#     from an earlier, unrelated run. THIS run truly changed nothing, and the next DO_BUILD=1 run
+#     wipes that leftover before it builds (`rm -f "$JAR_STAGED"` in the build section above) — so
+#     there is nothing here for the operator to lose track of.
+#   --no-build, staged jar absent  -> "nothing changed"
+drain_gate_refusal() {
+  local do_build="$1" staged_path="$2"
+  if [ "$do_build" = 1 ] && [ -f "$staged_path" ]; then
+    printf 'aborted — the running daemon was NOT touched, but the freshly built jar is sitting at
+        %s, not yet swapped into %s. Rerun WITHOUT --no-build to finish the restart —
+        the freshly built jar is no longer at the live path that --no-build requires — or
+        remove %s by hand if you want to discard this build.' "$staged_path" "$JAR" "$staged_path"
+  else
+    printf 'aborted — nothing changed'
+  fi
+}
+
 # CB-600: sourceable for testing. When this file is SOURCED (not executed) it stops here — nothing
 # below runs — so a test harness can `source` it to call check_log_path_matches_plist (or the
 # other pure helpers above) against a throwaway plist fixture without ever reaching the mutating
@@ -610,15 +639,10 @@ if [ -n "$OLD_PID" ] && [ "$ASSUME_YES" = 0 ]; then
   echo
   read -r -p "   Fleet drained? type yes to restart: " reply
   if [ "$reply" != "yes" ]; then
-    # fleetd #493: "nothing changed" would be a lie once a build has run — the freshly built jar
-    # already moved to $JAR_STAGED (stage_built_jar, above), so the live path has one fewer file
-    # than before this run started, even though the running daemon itself was never touched.
-    if [ "$DO_BUILD" = 1 ] && [ -f "$JAR_STAGED" ]; then
-      die "aborted — the running daemon was NOT touched, but the freshly built jar is sitting at
-          $JAR_STAGED, not yet swapped into $JAR. Rerun (with or without --no-build) to finish the
-          restart, or remove $JAR_STAGED by hand if you want to discard this build."
-    fi
-    die "aborted — nothing changed"
+    # fleetd #493 / #517: "nothing changed" would be a lie once a build has run and staged a jar —
+    # see drain_gate_refusal above for the full decision and why each of its four cases reads the
+    # way it does.
+    die "$(drain_gate_refusal "$DO_BUILD" "$JAR_STAGED")"
   fi
 fi
 

scripts/test-redeploy-fleetd.sh

@@ -206,6 +206,46 @@ test_assert_single_daemon_rejects_two_pids() {
   printf '%s' "$output" | grep -qF '4343' || fail "refusal message does not list the pids it found"
 }
 
+# fleetd #511 — jar_id()'s no-argument default was unpinned by any test: nothing proved it reports
+# $JAR (the live path) rather than $JAR_STAGED. Both halves matter, so this pins both: the bare call
+# must hash the live jar, and an explicit path argument must hash THAT file, not fall back to $JAR.
+# Two files with different content, so a default pointed at the wrong one reports the wrong hash
+# rather than accidentally matching.
+test_jar_id_defaults_to_live_and_reports_explicit_path() {
+  local dir saved_jar="$JAR" saved_staged="$JAR_STAGED"
+  local live_hash staged_hash default_result explicit_result
+  dir="$TMP/jar-id"; mkdir -p "$dir"
+  JAR="$dir/fleetd.jar"; JAR_STAGED="$dir/fleetd-new.jar"
+  printf 'live jar bytes' > "$JAR"
+  printf 'staged jar bytes, not the same content' > "$JAR_STAGED"
+  live_hash="$(shasum -a 256 "$JAR" | cut -c1-12)"
+  staged_hash="$(shasum -a 256 "$JAR_STAGED" | cut -c1-12)"
+  default_result="$(jar_id)"
+  explicit_result="$(jar_id "$JAR_STAGED")"
+  JAR="$saved_jar"; JAR_STAGED="$saved_staged"
+  [ "$live_hash" != "$staged_hash" ] || fail "test fixture error: live and staged jars hashed the same"
+  assert_equals "$live_hash" "$default_result" "jar_id with no arguments must report the hash of \$JAR"
+  assert_equals "$staged_hash" "$explicit_result" "jar_id \"\$JAR_STAGED\" must report the hash of the staged jar, not fall back to \$JAR"
+}
+
+# fleetd #517 — jar_id()'s "absent" branch was unpinned by any test: the existing test above (#511)
+# proves both halves of the present-file contract but never exercises the missing-file path. This
+# word matters more than a string usually would: "absent" is the #413 signal that a `mvn clean`
+# deleted the running daemon's jar out from under it, and the `redeploy-fleetd` skill points
+# operators at `--check` for exactly this. Covers both the no-argument default and an explicit path,
+# since the mutation (`absent` -> `present`) sits on the single shared `|| echo` and would flip both.
+test_jar_id_reports_absent_for_missing_file() {
+  local saved_jar="$JAR" dir default_result explicit_result
+  dir="$TMP/jar-id-absent"; mkdir -p "$dir"
+  JAR="$dir/does-not-exist.jar"
+  [ ! -f "$JAR" ] || fail "test fixture error: \$JAR unexpectedly exists at $JAR"
+  default_result="$(jar_id)"
+  explicit_result="$(jar_id "$dir/also-does-not-exist.jar")"
+  JAR="$saved_jar"
+  assert_equals "absent" "$default_result" "jar_id with no arguments must report absent when \$JAR does not exist"
+  assert_equals "absent" "$explicit_result" "jar_id with an explicit missing path must report absent"
+}
+
 # fleetd #493 — never build into the path a running process holds. stage_built_jar/swap_staged_jar
 # are exercised directly against real files on disk (not stubs), because the whole point is file
 # behavior (does the content move, does the source disappear, does a failure leave both sides
@@ -344,6 +384,74 @@ test_swap_ordered_after_wait_and_before_start() {
     || fail "swap_staged_jar (line $swap_line) is not before the start section (line $start_line)"
 }
 
+# fleetd #511: the drain-gate abort message (fired when a build has staged a jar but the operator
+# declines the drain confirmation) used to tell the operator to "Rerun (with or without --no-build)"
+# to finish the restart. That is wrong — by the time this message can fire, stage_built_jar has
+# already moved the jar off $JAR, so a rerun WITH --no-build hits require_no_build_jar's own refusal
+# ("no jar at $JAR — run without --no-build"). Like test_swap_ordered_after_wait_and_before_start
+# above, this code path is never reached by sourcing (the SOURCED guard stops before the main flow),
+# so the only way to pin its exact wording is to read the source.
+test_drain_gate_abort_message_says_no_no_build() {
+  local src="$ROOT/scripts/redeploy-fleetd.sh" msg
+  msg="$(grep -A3 -F 'aborted — the running daemon was NOT touched, but the freshly built jar is sitting at' "$src")"
+  [ -n "$msg" ] || fail "could not find the drain-gate staged-jar abort message in redeploy-fleetd.sh"
+  if printf '%s' "$msg" | grep -qF 'with or without --no-build'; then
+    fail "abort message still claims a rerun WITH --no-build can finish the restart"
+  fi
+  printf '%s' "$msg" | grep -qF 'WITHOUT --no-build' \
+    || fail "abort message does not tell the operator to rerun without --no-build"
+  printf '%s' "$msg" | grep -qF 'no longer at the live path' \
+    || fail "abort message does not say why --no-build cannot finish the restart"
+}
+
+# fleetd #517 — the drain-gate abort branch itself. Before this, the only test of this message was
+# a source-text grep (test_drain_gate_abort_message_says_no_no_build, below): it greps this script's
+# own file for the wording, which stays in the file even if the `if` guarding it is mutated to
+# `if false` and the branch can never run. These four tests call drain_gate_refusal directly instead,
+# so they fail if the branch is unreachable OR if its wording regresses — the grep test is KEPT
+# alongside these, not replaced, because it catches a different regression (a re-wording that still
+# reaches the right branch would not change which case fires here, but would still be worth pinning).
+test_drain_gate_refusal_build_ran_staged_present() {
+  local dir staged result
+  dir="$TMP/drain-refusal-build-staged"; mkdir -p "$dir"
+  staged="$dir/fleetd-new.jar"
+  printf 'staged jar bytes' > "$staged"
+  result="$(drain_gate_refusal 1 "$staged")"
+  printf '%s' "$result" | grep -qF "$staged" \
+    || fail "build-ran+staged-present refusal does not name the staged jar path"
+  printf '%s' "$result" | grep -qF 'Rerun WITHOUT --no-build' \
+    || fail "build-ran+staged-present refusal does not tell the operator how to finish the restart"
+  if printf '%s' "$result" | grep -qF 'nothing changed'; then
+    fail "build-ran+staged-present refusal must not claim nothing changed — the jar already moved"
+  fi
+}
+
+test_drain_gate_refusal_build_ran_staged_absent() {
+  local dir result
+  dir="$TMP/drain-refusal-build-no-staged"; mkdir -p "$dir"
+  result="$(drain_gate_refusal 1 "$dir/fleetd-new.jar")"
+  assert_equals "aborted — nothing changed" "$result" "build-ran+staged-absent refusal wording"
+}
+
+# --no-build itself never builds or stages anything (require_no_build_jar, above), so a staged jar
+# found here is a leftover from an earlier, unrelated run — THIS run truly changed nothing. See the
+# comment above drain_gate_refusal in redeploy-fleetd.sh for the full reasoning.
+test_drain_gate_refusal_no_build_staged_present() {
+  local dir staged result
+  dir="$TMP/drain-refusal-no-build-staged"; mkdir -p "$dir"
+  staged="$dir/fleetd-new.jar"
+  printf 'leftover staged jar bytes' > "$staged"
+  result="$(drain_gate_refusal 0 "$staged")"
+  assert_equals "aborted — nothing changed" "$result" "no-build+staged-present refusal must deliberately say nothing changed"
+}
+
+test_drain_gate_refusal_no_build_staged_absent() {
+  local dir result
+  dir="$TMP/drain-refusal-no-build-no-staged"; mkdir -p "$dir"
+  result="$(drain_gate_refusal 0 "$dir/fleetd-new.jar")"
+  assert_equals "aborted — nothing changed" "$result" "no-build+staged-absent refusal wording"
+}
+
 test_no_errors() {
   cat > "$TMP/no-errors.log" <<'LOG'
 2026-09-05 12:00:00 INFO fleetd listening
@@ -555,6 +663,8 @@ test_require_drivable_supervisor_accepts_known_kinds
 test_count_daemon_pids
 test_assert_single_daemon_accepts_one_pid
 test_assert_single_daemon_rejects_two_pids
+test_jar_id_defaults_to_live_and_reports_explicit_path
+test_jar_id_reports_absent_for_missing_file
 test_stage_built_jar_moves_off_live_path
 test_stage_built_jar_dies_when_build_produced_nothing
 test_swap_staged_jar_moves_staged_onto_live
@@ -565,6 +675,11 @@ test_require_no_build_jar_accepts_present_jar
 test_wait_for_daemon_exit_returns_true_once_pid_clears
 test_wait_for_daemon_exit_times_out_if_pid_never_clears
 test_swap_ordered_after_wait_and_before_start
+test_drain_gate_abort_message_says_no_no_build
+test_drain_gate_refusal_build_ran_staged_present
+test_drain_gate_refusal_build_ran_staged_absent
+test_drain_gate_refusal_no_build_staged_present
+test_drain_gate_refusal_no_build_staged_absent
 test_no_errors
 test_recovery_patterns_match_source
 test_attributed_recovered_connection_error