Merge CB-581: a throw in release() no longer orphans the pane or aborts reapIdle

Verified by the lead: own build of the branch merged onto main — 717 tests,
BUILD SUCCESS, exit 0.

release() ran an unprotected sequence. hasUncommitted shells out to git status
and throws on a non-zero exit, which skipped both notifyReleased and
launcher.stop. The session was already out of the registry, so nothing retried
it: a live pane kept burning a fleet slot while absent from the roster, and any
send blocked on it was never resolved. reapIdle called release bare inside a
loop, so one such session aborted the whole pass and skipped every session
after it.

Now the dirty-check block is guarded and fails toward preserving the worktree —
'we could not tell' must not be treated as 'it is clean', because deleting on a
guess destroys work with no other copy. notifyReleased runs in a finally and
launcher.stop runs unconditionally, so the pane always stops. reapIdle catches
per session, matching the shape drainAll already used.

Checked while reviewing: notifyReleased cannot throw out of the finally — it
already guards each listener and only logs. The pane stop is genuinely
unconditional.

bridged/src/main/java/dev/ltms/bridged/session/SessionManager.java

@@ -197,27 +197,44 @@ public final class SessionManager implements TurnListener {
         MemberSession removed = registry.remove(paneId);
         boolean preserveWorktree = cause == ReleaseCause.SHUTDOWN;
         if (removed != null) {
-            memberLifecycle.released(removed.terminalId());
-            log.debug("releasing session pane={} terminal={} state={} cause={}",
-                    removed.paneId(), removed.terminalId(), removed.state(), cause);
-            if (preserveWorktree && removed.worktree() != null) {
-                logPreservedForShutdown(removed);
-            } else if (removed.worktree() != null && worktrees.hasUncommitted(removed.worktree())) {
-                // CB-576: a release that would otherwise remove the worktree finds it holding
-                // uncommitted work the bridge cannot see. A worker that ends a turn without
-                // committing (normally because it stopped to ask a question or refused the turn)
-                // has its only copy of that work in the worktree. Remove would --force-delete it,
-                // so preserve the directory and tell an operator where to find it.
+            try {
+                memberLifecycle.released(removed.terminalId());
+                log.debug("releasing session pane={} terminal={} state={} cause={}",
+                        removed.paneId(), removed.terminalId(), removed.state(), cause);
+                if (preserveWorktree && removed.worktree() != null) {
+                    logPreservedForShutdown(removed);
+                } else if (removed.worktree() != null && worktrees.hasUncommitted(removed.worktree())) {
+                    // CB-576: a release that would otherwise remove the worktree finds it holding
+                    // uncommitted work the bridge cannot see. A worker that ends a turn without
+                    // committing (normally because it stopped to ask a question or refused the turn)
+                    // has its only copy of that work in the worktree. Remove would --force-delete it,
+                    // so preserve the directory and tell an operator where to find it.
+                    preserveWorktree = true;
+                    log.warn("release {} preserves dirty worktree {} for pane={} terminal={}: "
+                            + "the worktree holds uncommitted changes that --force remove would destroy",
+                            cause, removed.worktree(), removed.paneId(), removed.terminalId());
+                }
+            } catch (RuntimeException e) {
+                // CB-581: hasUncommitted shells out to `git status` and can throw on a non-zero
+                // exit. We can no longer tell whether the worktree holds uncommitted work, so fail
+                // toward the safe answer and preserve it — deleting on a guess can destroy work
+                // that has no other copy (CB-576), while keeping it on a false alarm only costs
+                // disk. The exception must not propagate: the pane still has to stop below.
                 preserveWorktree = true;
-                log.warn("release {} preserves dirty worktree {} for pane={} terminal={}: "
-                        + "the worktree holds uncommitted changes that --force remove would destroy",
-                        cause, removed.worktree(), removed.paneId(), removed.terminalId());
+                log.warn("release {} could not tell whether worktree {} for pane={} terminal={} has "
+                        + "uncommitted changes; preserving it rather than risk destroying unsaved work: {}",
+                        cause, removed.worktree(), removed.paneId(), removed.terminalId(), e.toString());
+            } finally {
+                // CB-516/CB-581: a send still waiting on this worker can never be answered now, no
+                // matter what happened above. Tell the listener BEFORE the pane is torn down, so a
+                // blocked caller fails fast with a real reason instead of sitting on a rendezvous
+                // nothing will ever resolve.
+                notifyReleased(removed.terminalId());
             }
-            // CB-516: a send still waiting on this worker can never be answered now. Tell the
-            // listener BEFORE the pane is torn down, so a blocked caller fails fast with a real
-            // reason instead of sitting on a rendezvous nothing will ever resolve.
-            notifyReleased(removed.terminalId());
         }
+        // CB-581: the pane must always stop, even if the dirty check above threw. A session removed
+        // from the registry with no pane stop is an orphaned pane — a live terminal burning a fleet
+        // slot that no longer appears in the roster and can never be reclaimed.
         launcher.stop(paneId);
         if (removed != null && !preserveWorktree && removed.worktree() != null) {
             worktrees.remove(worktrees.repoRoot(removed.cwd()), removed.worktree());
@@ -539,8 +556,15 @@ public final class SessionManager implements TurnListener {
                 log.debug("reaping idle session terminal={} pane={}: idle {}s exceeds the {}s ttl",
                         s.terminalId(), s.paneId(), TimeUnit.NANOSECONDS.toSeconds(idleNanos),
                         TimeUnit.NANOSECONDS.toSeconds(idleTtlNanos));
-                release(s.paneId());
-                reaped++;
+                // CB-581: one session that fails to release must not abort the whole reaping pass —
+                // match drainAll's per-session try/catch so the rest of the roster still gets reaped.
+                try {
+                    release(s.paneId());
+                    reaped++;
+                } catch (RuntimeException e) {
+                    log.warn("reap failed for pane={} terminal={} worktree={}; continuing with "
+                            + "remaining sessions", s.paneId(), s.terminalId(), s.worktree(), e);
+                }
             }
         }
         return reaped;

bridged/src/test/java/dev/ltms/bridged/session/SessionManagerTest.java

@@ -46,6 +46,82 @@ class SessionManagerTest {
         return sessionManager(herdr, clock, 0);
     }
 
+    private SessionManager sessionManager(FakeHerdr herdr, Worktrees worktrees) {
+        return sessionManager(herdr, worktrees, System::nanoTime);
+    }
+
+    private SessionManager sessionManager(FakeHerdr herdr, Worktrees worktrees, LongSupplier clock) {
+        BridgedConfig.Profile cfg = new BridgedConfig.Profile(
+                "ltms-local", "http://gx00.gw:8000", "coder", null, "BRIDGED_WORKER_TOKEN",
+                List.of("ccs", "ltms-local"), "tab", "bridged-workers",
+                "worker: {profile} #{n}", null, null, null);
+        ClaudeCodeLauncher workers = new ClaudeCodeLauncher(new AgentControl(herdr), new WorkspaceControl(herdr),
+                new SubscriptionGuard(Set.of("gx00.gw")), Map.of(cfg.profile(), cfg), cfg.profile(), _ -> null);
+        return new SessionManager(workers, worktrees, clock);
+    }
+
+    /**
+     * 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
+     * failure {@code GitWorktrees} produces when {@code git status}/{@code git worktree remove}
+     * exits non-zero.
+     */
+    private static final class RecordingWorktrees implements Worktrees {
+        private final List<String> removeCalls = new java.util.ArrayList<>();
+        private final java.util.Set<String> failRemoveFor = new java.util.HashSet<>();
+        private volatile boolean dirty = false;
+        private volatile RuntimeException hasUncommittedFailure;
+
+        RecordingWorktrees dirty(boolean dirty) {
+            this.dirty = dirty;
+            return this;
+        }
+
+        RecordingWorktrees failHasUncommittedWith(RuntimeException e) {
+            this.hasUncommittedFailure = e;
+            return this;
+        }
+
+        RecordingWorktrees failRemoveFor(String worktreePath) {
+            failRemoveFor.add(worktreePath);
+            return this;
+        }
+
+        @Override
+        public String add(String repoRoot, String branch, String baseRef) {
+            return "/wt/" + branch.replace('/', '_');
+        }
+
+        @Override
+        public void remove(String repoRoot, String worktreePath) {
+            if (failRemoveFor.contains(worktreePath)) {
+                throw new WorktreeException("simulated remove failure for " + worktreePath);
+            }
+            removeCalls.add(worktreePath);
+        }
+
+        @Override
+        public boolean hasUncommitted(String worktreePath) {
+            if (hasUncommittedFailure != null) {
+                throw hasUncommittedFailure;
+            }
+            return dirty;
+        }
+
+        @Override
+        public void overlayParity(String repoRoot, String worktreePath, List<String> overlay) {
+        }
+
+        @Override
+        public String repoRoot(String cwd) {
+            return "/repo";
+        }
+
+        List<String> removeCalls() {
+            return List.copyOf(removeCalls);
+        }
+    }
+
     private SessionManager sessionManager(FakeHerdr herdr, LongSupplier clock, int contextCap) {
         return sessionManager(herdr, clock, contextCap, false);
     }
@@ -525,4 +601,172 @@ class SessionManagerTest {
                 "a listener failure must never prevent the teardown it is reacting to");
         assertTrue(sessions.get(s.paneId()).isEmpty(), "and the session is still deregistered");
     }
+
+    // --- CB-581: a throw inside release() must not orphan the pane or abort reapIdle -----------
+
+    @Test
+    void releasePreservesWorktreeWhenDirtyCheckThrows() {
+        FakeHerdr herdr = new FakeHerdr();
+        RecordingWorktrees worktrees = new RecordingWorktrees();
+        SessionManager sessions = sessionManager(herdr, worktrees);
+        MemberSession s = sessions.acquire("ltms-local", null, "/caller/proj", null,
+                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 {
+            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()
+                    .filter(e -> e.getLevel().equals(Level.WARN))
+                    .map(ILoggingEvent::getFormattedMessage)
+                    .filter(m -> m.contains(s.worktree()))
+                    .findFirst()
+                    .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);
+        }
+    }
+
+    @Test
+    void releaseStillStopsThePaneWhenDirtyCheckThrows() {
+        FakeHerdr herdr = new FakeHerdr();
+        RecordingWorktrees worktrees = new RecordingWorktrees();
+        SessionManager sessions = sessionManager(herdr, worktrees);
+        MemberSession s = sessions.acquire("ltms-local", null, "/caller/proj", null,
+                new WorktreeRequest("cb-581b", null));
+        worktrees.failHasUncommittedWith(new WorktreeException("git status exited 128"));
+
+        sessions.release(s.paneId());
+
+        assertEquals(1, paneCloseCallsFor(herdr, "w9:pRoot_1"),
+                "the pane is stopped exactly once even though the dirty check threw");
+    }
+
+    @Test
+    void releaseStillNotifiesTheListenerWhenDirtyCheckThrows() {
+        FakeHerdr herdr = new FakeHerdr();
+        RecordingWorktrees worktrees = new RecordingWorktrees();
+        SessionManager sessions = sessionManager(herdr, worktrees);
+        java.util.List<String> released = new java.util.concurrent.CopyOnWriteArrayList<>();
+        sessions.onRelease(released::add);
+        MemberSession s = sessions.acquire("ltms-local", null, "/caller/proj", null,
+                new WorktreeRequest("cb-581c", null));
+        worktrees.failHasUncommittedWith(new WorktreeException("git status exited 128"));
+
+        sessions.release(s.paneId());
+
+        assertEquals(java.util.List.of(s.terminalId()), released,
+                "a blocked caller must still be told the terminal was released, even though the "
+                        + "dirty check threw");
+    }
+
+    @Test
+    void reapIdleSurvivesOneSessionThatFailsToRelease() {
+        long[] clock = {0};
+        FakeHerdr herdr = new FakeHerdr();
+        RecordingWorktrees worktrees = new RecordingWorktrees();
+        SessionManager sessions = sessionManager(herdr, worktrees, () -> clock[0]);
+        MemberSession a = sessions.acquire("ltms-local", null, "/caller/proj", null,
+                new WorktreeRequest("cb-581d", null));
+        MemberSession b = sessions.acquire("ltms-local", null, "/caller/proj", null,
+                new WorktreeRequest("cb-581e", null));
+        MemberSession c = sessions.acquire("ltms-local", null, "/caller/proj", null,
+                new WorktreeRequest("cb-581f", null));
+        sessions.asPresence().markPresent(a.terminalId());
+        sessions.asPresence().markPresent(b.terminalId());
+        sessions.asPresence().markPresent(c.terminalId());
+        // The middle session's worktree removal fails — release() propagates that, so this is the
+        // one call reapIdle's per-session guard must survive without skipping the rest of the pass.
+        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 {
+            clock[0] = 100;
+            reaped = sessions.reapIdle(10);
+
+            String warn = appender.list.stream()
+                    .filter(e -> e.getLevel().equals(Level.WARN))
+                    .map(ILoggingEvent::getFormattedMessage)
+                    .filter(m -> m.contains(b.paneId()))
+                    .findFirst()
+                    .orElse("no reap-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(2, reaped, "the middle session's failure is logged, not counted as reaped");
+        assertTrue(sessions.get(a.paneId()).isEmpty(), "the first session is still released");
+        assertTrue(sessions.get(c.paneId()).isEmpty(), "the third session is still released");
+        assertTrue(sessions.get(b.paneId()).isEmpty(),
+                "the middle session is still deregistered even though its worktree removal threw");
+        assertEquals(1, paneCloseCallsFor(herdr, "w9:pRoot_1"), "the first pane is stopped");
+        assertEquals(1, paneCloseCallsFor(herdr, "w9:pRoot_2"),
+                "the middle pane is still stopped even though its worktree removal failed");
+        assertEquals(1, paneCloseCallsFor(herdr, "w9:pRoot_3"), "the third pane is stopped");
+    }
+
+    @Test
+    void unchangedRegressionCleanCompletedReleaseStillRemovesTheWorktree() {
+        FakeHerdr herdr = new FakeHerdr();
+        RecordingWorktrees worktrees = new RecordingWorktrees();
+        SessionManager sessions = sessionManager(herdr, worktrees);
+        MemberSession s = sessions.acquire("ltms-local", null, "/caller/proj", null,
+                new WorktreeRequest("cb-581g", null));
+
+        sessions.release(s.paneId());
+
+        assertEquals(List.of(s.worktree()), worktrees.removeCalls(),
+                "COMPLETED release of a clean worktree still removes it");
+    }
+
+    @Test
+    void unchangedRegressionDirtyCompletedReleaseStillPreservesTheWorktree() {
+        FakeHerdr herdr = new FakeHerdr();
+        RecordingWorktrees worktrees = new RecordingWorktrees().dirty(true);
+        SessionManager sessions = sessionManager(herdr, worktrees);
+        MemberSession s = sessions.acquire("ltms-local", null, "/caller/proj", null,
+                new WorktreeRequest("cb-581h", null));
+
+        sessions.release(s.paneId());
+
+        assertTrue(worktrees.removeCalls().isEmpty(),
+                "COMPLETED release of a dirty worktree still preserves it");
+    }
+
+    @Test
+    void unchangedRegressionShutdownDrainStillPreservesTheWorktree() {
+        FakeHerdr herdr = new FakeHerdr();
+        RecordingWorktrees worktrees = new RecordingWorktrees();
+        SessionManager sessions = sessionManager(herdr, worktrees);
+        MemberSession s = sessions.acquire("ltms-local", null, "/caller/proj", null,
+                new WorktreeRequest("cb-581i", null));
+        sessions.asPresence().markPresent(s.terminalId());
+
+        sessions.drainAll(TimeUnit.MILLISECONDS.toNanos(100));
+
+        assertTrue(worktrees.removeCalls().isEmpty(), "SHUTDOWN drain still preserves the worktree");
+    }
 }