diff --git a/fleetd/src/main/java/dev/ltms/fleet/member/HerdrPeerLauncher.java b/fleetd/src/main/java/dev/ltms/fleet/member/HerdrPeerLauncher.java
index 40e3cf6..8e9fe6c 100644
--- a/fleetd/src/main/java/dev/ltms/fleet/member/HerdrPeerLauncher.java
+++ b/fleetd/src/main/java/dev/ltms/fleet/member/HerdrPeerLauncher.java
@@ -913,9 +913,14 @@ public abstract class HerdrPeerLauncher implements PeerLauncher {
* {@link #reapOrphanWorkers() orphan-reap} and spawn-gate-timeout paths, plus any caller that
* passes a pane directly, keep working without an owning id.
*
- * Resolves the tab from the pane before closing it. An already-gone pane/tab
- * (repeated DELETE, crashed peer) is treated as success; any other failure propagates so a
- * genuinely failed teardown is not reported as done.
+ *
Resolves the tab from the pane before closing it. {@code agents.close} (the pane)
+ * is the one step whose failure means the teardown itself may not have happened: an already-gone
+ * pane (repeated DELETE, crashed peer) is treated as success, but any other failure propagates so
+ * a genuinely failed teardown is not reported as done. {@code spaces.closeTab} (fleetd #293) is
+ * different — by the time it runs the pane is already closed, so it is cosmetic workspace tidying
+ * rather than a real teardown failure, and a failure there is logged and never propagates, so it
+ * cannot mask the two cleanups below it ({@link #releaseZdotdir}, and the caller's worktree
+ * removal in {@code SessionManager.release}).
*/
@Override
public void stop(String idOrPane) {
@@ -934,7 +939,18 @@ public abstract class HerdrPeerLauncher implements PeerLauncher {
log.debug("pane.close({}) ignored — already gone: {}", paneId, e.getMessage());
}
if (loc != null && loc.tabPaneCount() == 1) {
- spaces.closeTab(loc.tabId());
+ // fleetd #293: the pane above is already closed by this point, so a failing tab.close is
+ // cosmetic workspace tidying, not a real teardown failure — it must not mask the two
+ // cleanups below it (releaseZdotdir, and the caller's worktree removal). Unlike
+ // agents.close above, this is not narrowed to "already gone": any failure here, whatever
+ // its cause, is one we continue past, so we log it at WARN (not debug) with the tab id a
+ // person can go close by hand.
+ try {
+ spaces.closeTab(loc.tabId());
+ } catch (HerdrException e) {
+ log.warn("tab.close({}) failed — the pane is already torn down, so continuing; the "
+ + "tab may need manual cleanup: {}", loc.tabId(), e.getMessage());
+ }
} else if (loc != null) {
log.debug("not closing tab {} — it holds {} panes (not a dedicated peer tab)",
loc.tabId(), loc.tabPaneCount());
diff --git a/fleetd/src/test/java/dev/ltms/fleet/herdr/FakeHerdr.java b/fleetd/src/test/java/dev/ltms/fleet/herdr/FakeHerdr.java
index 0cfe48d..a3f562b 100644
--- a/fleetd/src/test/java/dev/ltms/fleet/herdr/FakeHerdr.java
+++ b/fleetd/src/test/java/dev/ltms/fleet/herdr/FakeHerdr.java
@@ -43,6 +43,8 @@ public final class FakeHerdr implements HerdrClient {
private int workerTabPaneCount = 1;
private String paneCloseErrorCode = null;
private final Map paneCloseErrorCodeFor = new ConcurrentHashMap<>();
+ private String tabCloseErrorCode = null;
+ private final Map tabCloseErrorCodeFor = new ConcurrentHashMap<>();
private String agentSendErrorCode = null;
private boolean noPanes = false;
private volatile String agentStatus = "idle"; // steady-state agent.get status
@@ -106,6 +108,26 @@ public final class FakeHerdr implements HerdrClient {
return this;
}
+ /** Make {@code tab.close} fail with this herdr error code, for every tab. */
+ public FakeHerdr tabCloseFailsWith(String code) {
+ this.tabCloseErrorCode = code;
+ return this;
+ }
+
+ /**
+ * Make {@code tab.close} fail with this herdr error code, but only for the given {@code
+ * tab_id} — every other tab's {@code tab.close} still succeeds. The {@code tab.close}
+ * counterpart to {@link #paneCloseFailsForPane} (fleetd #290): lets a test make exactly one
+ * session's tab teardown fail while proving the rest of {@code stop()} — {@code
+ * releaseZdotdir}, and the caller's worktree removal — still runs (fleetd #293). Named "ForTab"
+ * rather than "ForPane" (unlike its sibling) because {@code tab.close} keys on {@code tab_id},
+ * not a pane id.
+ */
+ public FakeHerdr tabCloseFailsForTab(String tabId, String code) {
+ this.tabCloseErrorCodeFor.put(tabId, code);
+ return this;
+ }
+
/**
* Make {@code pane.list} report no panes at all — models a second herdr daemon (CB-185) that
* simply does not host the pane a {@link PaneLocator} is searching for.
@@ -352,7 +374,17 @@ public final class FakeHerdr implements HerdrClient {
.formatted(workerTabPaneCount,
seeded.isEmpty() ? "" : "," + String.join(",", seeded)));
}
- case "tab.close" -> mapper.readTree("{\"type\":\"ok\"}");
+ case "tab.close" -> {
+ Object tabIdParam = params instanceof Map m ? m.get("tab_id") : null;
+ String perTabCode = tabIdParam == null ? null
+ : tabCloseErrorCodeFor.get(String.valueOf(tabIdParam));
+ String code = perTabCode != null ? perTabCode : tabCloseErrorCode;
+ if (code != null) {
+ throw new HerdrException("herdr error [" + code + "]: tab.close failed",
+ code, null);
+ }
+ yield mapper.readTree("{\"type\":\"ok\"}");
+ }
case "pane.get" -> mapper.readTree("""
{"type":"pane_info","pane":{"pane_id":"w9:pW","workspace_id":"w9",
"tab_id":"w9:t2","agent_status":"idle"}}""");
diff --git a/fleetd/src/test/java/dev/ltms/fleet/member/ClaudeCodeLauncherTest.java b/fleetd/src/test/java/dev/ltms/fleet/member/ClaudeCodeLauncherTest.java
index 258c45e..2352841 100644
--- a/fleetd/src/test/java/dev/ltms/fleet/member/ClaudeCodeLauncherTest.java
+++ b/fleetd/src/test/java/dev/ltms/fleet/member/ClaudeCodeLauncherTest.java
@@ -925,6 +925,71 @@ class ClaudeCodeLauncherTest {
assertTrue(herdr.called("pane.close"), "stop via handle.id() must close the pane");
}
+ /** A tab-placement launcher with {@code memberCredentials policy=allow-list} under a zsh shell — the
+ * combination that makes {@link HerdrPeerLauncher#spawn} generate a real ZDOTDIR, so {@code
+ * releaseZdotdir}'s effect (the directory's deletion) is observable from a test. */
+ private ClaudeCodeLauncher serviceWithAllowList(FakeHerdr herdr) {
+ FleetConfig.Profile cfg = new FleetConfig.Profile(
+ "ltms-local", "http://gx00.gw:8000", "coder", null, "FLEETD_WORKER_TOKEN",
+ List.of("ccs", "ltms-local"), "tab", "fleetd-workers",
+ "worker: {profile} #{n}", null, null, null);
+ Supplier creds = () -> new FleetConfig.MemberCredentials(
+ FleetConfig.MemberCredentials.POLICY_ALLOW_LIST, List.of(), List.of(), null);
+ Function env = name -> "SHELL".equals(name) ? "/bin/zsh" : null;
+ return new ClaudeCodeLauncher(new AgentControl(herdr), new WorkspaceControl(herdr),
+ new SubscriptionGuard(Set.of("gx00.gw")), Map.of(cfg.profile(), cfg), cfg.profile(),
+ env, 0, System::currentTimeMillis, () -> { }, null, creds);
+ }
+
+ /**
+ * fleetd #293: {@code stop()} used to run {@code spaces.closeTab} bare — any non-{@code
+ * *_not_found} herdr error propagated straight out of {@code stop()}, skipping {@code
+ * releaseZdotdir} entirely (the pane was already closed by that point, so the tab-close failure
+ * is cosmetic, not a real teardown failure). Proves both halves of the fix: {@code stop()} no
+ * longer throws for this failure, and {@code releaseZdotdir} still runs — observed here by the
+ * generated ZDOTDIR actually being deleted, since {@code releaseZdotdir}'s last line is {@code
+ * EnvAllowListScrub.deleteRecursively(dir)}.
+ */
+ @Test
+ @SuppressWarnings("unchecked")
+ void stopStillReleasesZdotdirWhenCloseTabFailsWithANonNotFoundCode() {
+ FakeHerdr herdr = new FakeHerdr();
+ ClaudeCodeLauncher svc = serviceWithAllowList(herdr);
+ PeerHandle handle = svc.spawn(new SpawnRequest(null, null, null));
+ Map tabCreateParams = (Map) herdr.lastCall("tab.create").params();
+ Map tabEnv = (Map) tabCreateParams.get("env");
+ String zdotdir = tabEnv.get("ZDOTDIR");
+ assertNotNull(zdotdir, "policy=allow-list under a zsh shell must have generated a ZDOTDIR: " + tabEnv);
+ Path dir = Path.of(zdotdir);
+ assertTrue(Files.isDirectory(dir), "the generated ZDOTDIR must exist before stop(): " + dir);
+ herdr.tabCloseFailsForTab("w9:t2", "internal_error");
+
+ Logger logger = (Logger) LoggerFactory.getLogger(HerdrPeerLauncher.class);
+ ListAppender appender = new ListAppender<>();
+ appender.start();
+ logger.addAppender(appender);
+ try {
+ assertDoesNotThrow(() -> svc.stop(handle.id()),
+ "fleetd #293: a failing tab.close is cosmetic — it must not propagate out of stop()");
+ } finally {
+ logger.detachAppender(appender);
+ }
+
+ assertTrue(herdr.called("tab.close"), "tab.close was still attempted");
+ assertFalse(Files.exists(dir),
+ "releaseZdotdir must still run and delete the generated ZDOTDIR despite the tab.close "
+ + "failure: " + dir);
+ String warn = appender.list.stream()
+ .filter(e -> e.getLevel().equals(Level.WARN))
+ .map(ILoggingEvent::getFormattedMessage)
+ .filter(m -> m.contains("tab.close") && m.contains("w9:t2"))
+ .findFirst()
+ .orElse(null);
+ assertNotNull(warn, "the failing tab.close must be logged at WARN naming the tab id — a "
+ + "silently swallowed failure with no message is not an improvement. Log lines: "
+ + appender.list.stream().map(ILoggingEvent::getFormattedMessage).toList());
+ }
+
// --- CB-519: host-unique id, decoupled from the pane coordinate ------------------------------
@Test
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 080be4c..e353519 100644
--- a/fleetd/src/test/java/dev/ltms/fleet/session/SessionManagerTest.java
+++ b/fleetd/src/test/java/dev/ltms/fleet/session/SessionManagerTest.java
@@ -1156,6 +1156,41 @@ class SessionManagerTest {
"COMPLETED release of a clean worktree still removes it");
}
+ /**
+ * fleetd #293: {@code HerdrPeerLauncher.stop()} used to run {@code spaces.closeTab} bare — a
+ * failing {@code tab.close} (any code other than {@code *_not_found}) propagated straight out
+ * of {@code stop()}. {@code SessionManager.release} calls {@code launcher.stop(paneId)} with
+ * no try/catch (fleetd #283 wrapped the WORKTREE-removal step further down, not this one), so
+ * the throw happened before that worktree-removal step ever ran — and by then {@code
+ * registry.remove(paneId)} had already run, so a second {@code stop} is a no-op: the worktree
+ * leaked with no retry path. The pane itself is already closed by the time {@code tab.close}
+ * runs, so its failure is cosmetic workspace tidying, not a real teardown failure. The fix
+ * wraps {@code closeTab} inside {@code stop()} so it no longer throws for this reason; this
+ * test proves both halves at once: {@code release()} does not throw, and it still removes the
+ * worktree.
+ */
+ @Test
+ void releaseStillRemovesTheWorktreeWhenCloseTabFails() {
+ 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-293a", null));
+ // FakeHerdr's pane.get always answers with tab_id "w9:t2" for a tab-placement spawn.
+ herdr.tabCloseFailsForTab("w9:t2", "internal_error");
+
+ assertDoesNotThrow(() -> sessions.release(s.paneId()),
+ "a failing tab.close is cosmetic (the pane is already closed by then) — it must not "
+ + "propagate out of release()");
+
+ assertTrue(herdr.called("tab.close"), "tab.close was still attempted");
+ assertEquals(List.of(s.worktree()), worktrees.removeCalls(),
+ "release() must still remove the worktree even though tab.close failed — this is "
+ + "the leak fleetd #293 reports: before the fix, release() never reached this "
+ + "step at all");
+ assertTrue(sessions.get(s.paneId()).isEmpty(), "the session is still deregistered");
+ }
+
@Test
void unchangedRegressionDirtyCompletedReleaseStillPreservesTheWorktree() {
FakeHerdr herdr = new FakeHerdr();