Compare commits

...

9 Commits

Author SHA1 Message Date
Dai Ha cba516bda4 fleetd #296: close panes on failed spawn
CI / contract (pull_request) Successful in 1m38s
CI / build (pull_request) Successful in 2m7s
2026-09-04 12:05:26 +07:00
Dai Ha ba51e0c6cc #293: catch RuntimeException on closeTab, matching its sibling guard
CI / contract (push) Successful in 50s
CI / build (push) Successful in 2m15s
No behaviour change today. HerdrCodec wraps every encode/decode failure
and UnixSocketHerdrClient wraps every IOException, so HerdrException is
all closeTab can currently throw.

But releaseZdotdir five lines below catches RuntimeException, and the
whole point of this fix is that nothing here may mask the cleanups
below. Guarding against the expected exception type and staying bare
against any other is the same asymmetry the ticket exists to remove,
one level down. This stops a later change inside
WorkspaceControl.closeTab reopening it.
2026-09-04 11:47:42 +07:00
Dai Ha 086c59848e Merge #293: a failing tab.close no longer masks the cleanups below it 2026-09-04 11:46:00 +07:00
Dai Ha 0c10079755 #293: wrap the bare tab.close in HerdrPeerLauncher.stop()
CI / contract (pull_request) Successful in 1m41s
CI / build (pull_request) Successful in 1m59s
The pane is already closed by the time spaces.closeTab runs, so a failing
tab.close is cosmetic workspace tidying, not a real teardown failure. Left
bare, it propagated out of stop() and masked releaseZdotdir (ZDOTDIR leak)
and, worse, SessionManager.release()'s worktree removal (no self-heal,
no retry — the registry entry is already gone by then).

Wrap it in a try/catch that logs a WARN naming the tab id, matching the
"must not mask a real teardown failure above" comment already on
releaseZdotdir. isAlreadyGone is untouched — this continues past *any*
tab.close failure, not just *_not_found, since the failure is cosmetic
regardless of its cause.

Adds FakeHerdr#tabCloseFailsWith/tabCloseFailsForTab (the tab.close
counterpart to #290's paneCloseFailsForPane) plus two tests: one proving
releaseZdotdir still runs (the generated ZDOTDIR is deleted) and one
proving SessionManager.release() still removes the worktree, both with a
non-not_found tab.close failure.
2026-09-04 11:41:07 +07:00
Dai Ha ece2091b53 #280: states is now touched by two scheduler tasks, so make it concurrent
CI / contract (push) Successful in 48s
CI / build (push) Successful in 1m56s
The delayed re-check reads `states` from its own scheduled task, while
`tick` writes and prunes it. Both run on the single-threaded scheduler
Fleetd passes in today, so they are serialised — but nothing in the
class enforces that, and an unsynchronised HashMap read racing a resize
can spin a CPU forever rather than fail visibly.

`priors` and `orphanStreaks` stay plain maps: `tick` is still their only
toucher. The comment says which is which, so the next person does not
have to re-derive it.
2026-09-04 11:32:56 +07:00
Dai Ha b3f917e6f5 Merge #280: one bounded delayed re-check sweeps a ticket whose ask lapsed after GONE 2026-09-04 11:31:00 +07:00
Dai Ha d5128a1d35 #290: use the imports FakeHerdr already has
CI / build (push) Successful in 1m33s
CI / contract (push) Successful in 1m39s
2026-09-04 11:27:51 +07:00
Dai Ha 61097e5cf0 Merge #290: restore coverage for reapIdle's per-session guard 2026-09-04 11:25:45 +07:00
Dai Ha ef507bcd12 #290: restore reapIdle's per-session guard coverage via a new launcher.stop() trigger
CI / contract (pull_request) Successful in 1m29s
CI / build (pull_request) Successful in 1m56s
#283 fixed release() to catch and log a worktree-removal failure, which closed off
reapIdleCountsAllThreeSessionsWhenOnlyItsWorktreeRemovalFails as a trigger for
reapIdle's own per-session try/catch (CB-581) — that test now proves a different,
still-real thing (a swallowed removal failure doesn't shrink the reaped count),
but the try/catch itself lost its test.

Add FakeHerdr.paneCloseFailsForPane(paneId, code) so a test can make exactly one
session's launcher.stop() fail while its siblings still tear down normally
(paneCloseFailsWith already existed but fails every pane, which cannot isolate
one session in a three-session reap). Add
reapIdleSurvivesOneSessionWhoseLauncherStopFails beside the #283 test, using
launcher.stop() as the trigger the ticket names, and prove it catches removal of
reapIdle's try/catch: deleting the guard makes the test fail with the
HerdrException propagating out of reapIdle uncaught (quoted in the PR body).
2026-09-04 11:23:52 +07:00
5 changed files with 325 additions and 16 deletions
@@ -14,6 +14,7 @@ import java.util.HashSet;
import java.util.List;
import java.util.Map;
import java.util.Objects;
import java.util.concurrent.ConcurrentHashMap;
import java.util.concurrent.ScheduledExecutorService;
import java.util.concurrent.TimeUnit;
import java.util.function.BiConsumer;
@@ -46,7 +47,16 @@ public final class FleetHealthMonitor {
private final long workingSuspectAfterNanos;
private final BiConsumer<String, String> failTarget;
private final Map<String, HealthPrior> priors = new HashMap<>();
private final Map<String, HealthState> states = new HashMap<>();
/**
* The live classification per member, and the only one of this class's three maps that more
* than one scheduler task touches. {@code tick} writes it (and prunes it to the roster);
* fleetd #280's delayed {@link #recheckTerminalTarget} reads it from its own separate scheduled
* task. Both run on the single-threaded scheduler {@code Fleetd} passes in today, so they are
* serialised — but nothing in this class enforces that, and an unsynchronised {@link HashMap}
* read racing a resize can spin a CPU forever rather than fail visibly. {@code priors} and
* {@code orphanStreaks} stay plain maps because {@code tick} is still their only toucher.
*/
private final Map<String, HealthState> states = new ConcurrentHashMap<>();
/**
* CB-643: consecutive ticks on which a target looked like an orphaned delegation. The fact
* {@link MessageService#hasOrphanedDelegation} reports is a true snapshot, but it can read true
@@ -697,7 +697,20 @@ public abstract class HerdrPeerLauncher implements PeerLauncher {
if (paneId == null) {
throw new IllegalStateException("pane.split returned no pane — cannot start a peer");
}
Agent peer = startUniquelyNamed(cfg, argv, paneId).agent();
Agent peer;
try {
peer = startUniquelyNamed(cfg, argv, paneId).agent();
} catch (RuntimeException e) {
// The peer never started — don't leave the pane we just created orphaned.
// Best-effort cleanup; never let it mask the real spawn failure.
try {
stop(paneId);
} catch (RuntimeException cleanup) {
log.warn("failed to close orphaned pane {} after spawn error: {}",
paneId, cleanup.getMessage());
}
throw e;
}
log.info("{} started pane={} terminal={}", namePrefix, peer.paneId(), peer.terminalId());
return peer;
}
@@ -913,9 +926,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.
*
* <p>Resolves the tab from the pane <em>before</em> 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.
* <p>Resolves the tab from the pane <em>before</em> 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 +952,25 @@ 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 (RuntimeException e) {
// Caught as RuntimeException, not HerdrException, to match releaseZdotdir's own
// guard five lines below. Today the two are the same set — HerdrCodec wraps every
// encode/decode failure and UnixSocketHerdrClient wraps every IOException, so
// HerdrException is all closeTab can actually throw. Narrowing to it anyway would
// leave this step guarded against the expected failure and bare against any other,
// which is the exact asymmetry fleetd #293 exists to remove. No behaviour change
// today; it stops a later change inside WorkspaceControl.closeTab reopening it.
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());
@@ -973,7 +1009,8 @@ public abstract class HerdrPeerLauncher implements PeerLauncher {
* that gap: it stops waiting immediately (never burns the rest of the timeout), runs the same
* teardown the timeout path below runs, and throws with a message that says the backend exited
* rather than that the pane was slow. Any other {@link HerdrException} still propagates
* unchanged — this gate does not know how to recover from it.
* unchanged — this gate does not interpret or recover from it, but it still closes the pane
* it opened before handing the exception to its caller.
*/
private void waitUntilInjectableOrThrow(String paneId) {
long start = nowMillis.getAsLong();
@@ -987,7 +1024,15 @@ public abstract class HerdrPeerLauncher implements PeerLauncher {
if (isAlreadyGone(e)) {
failFastOnGoneBackend(paneId, e, nowMillis.getAsLong() - start);
}
throw e; // any other herdr failure is not ours to interpret — let it propagate
// This gate must not interpret an unrelated herdr error, but the caller does not
// receive paneId when spawn throws. Close the pane here before propagating e unchanged.
try {
stop(paneId);
} catch (RuntimeException cleanup) {
log.warn("failed to close orphaned pane {} after readiness-gate error: {}",
paneId, cleanup.getMessage());
}
throw e;
}
lastStatus = sample.status();
if (lastStatus.injectable() || refinedInjectable(paneId, sample)) {
@@ -7,6 +7,7 @@ import java.util.ArrayList;
import java.util.LinkedHashMap;
import java.util.List;
import java.util.Map;
import java.util.concurrent.ConcurrentHashMap;
import java.util.concurrent.CopyOnWriteArrayList;
/**
@@ -39,8 +40,12 @@ public final class FakeHerdr implements HerdrClient {
private final Map<String, List<String>> extraTabs = new LinkedHashMap<>();
private int agentNameTakenFor = 0;
private int agentPaneBusyFor = 0;
private String agentStartErrorCode = null;
private int workerTabPaneCount = 1;
private String paneCloseErrorCode = null;
private final Map<String, String> paneCloseErrorCodeFor = new ConcurrentHashMap<>();
private String tabCloseErrorCode = null;
private final Map<String, String> tabCloseErrorCodeFor = new ConcurrentHashMap<>();
private String agentSendErrorCode = null;
private boolean noPanes = false;
private volatile String agentStatus = "idle"; // steady-state agent.get status
@@ -80,18 +85,56 @@ public final class FakeHerdr implements HerdrClient {
return this;
}
/** Make every {@code agent.start} call fail with this herdr error code. */
public FakeHerdr agentStartFailsWith(String code) {
this.agentStartErrorCode = code;
return this;
}
/** Make the worker tab (w9:t2) report this many panes in {@code tab.list} (default 1). */
public FakeHerdr withWorkerTabPaneCount(int n) {
this.workerTabPaneCount = n;
return this;
}
/** Make {@code pane.close} fail with this herdr error code. */
/** Make {@code pane.close} fail with this herdr error code, for every pane. */
public FakeHerdr paneCloseFailsWith(String code) {
this.paneCloseErrorCode = code;
return this;
}
/**
* Make {@code pane.close} fail with this herdr error code, but only for the given {@code
* pane_id} — every other pane's {@code pane.close} still succeeds. Unlike {@link
* #paneCloseFailsWith}, which fails every call regardless of which pane it targets, this lets a
* test reap/release several sessions at once and make exactly one of them fail to stop, so the
* others' teardown can be asserted to proceed normally (fleetd #290).
*/
public FakeHerdr paneCloseFailsForPane(String paneId, String code) {
this.paneCloseErrorCodeFor.put(paneId, code);
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.
@@ -275,6 +318,10 @@ public final class FakeHerdr implements HerdrClient {
+ required + "`", "invalid_request", null);
}
}
if (agentStartErrorCode != null) {
throw new HerdrException("herdr error [" + agentStartErrorCode + "]: agent.start failed",
agentStartErrorCode, null);
}
long starts = calls.stream().filter(c -> c.method().equals("agent.start")).count();
if (starts <= agentPaneBusyFor) {
throw new HerdrException(
@@ -338,7 +385,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"}}""");
@@ -360,9 +417,13 @@ public final class FakeHerdr implements HerdrClient {
"foreground_processes":[]}}""");
}
case "pane.close" -> {
if (paneCloseErrorCode != null) {
throw new HerdrException("herdr error [" + paneCloseErrorCode + "]: pane.close failed",
paneCloseErrorCode, null);
Object paneIdParam = params instanceof Map<?, ?> m ? m.get("pane_id") : null;
String perPaneCode = paneIdParam == null ? null
: paneCloseErrorCodeFor.get(String.valueOf(paneIdParam));
String code = perPaneCode != null ? perPaneCode : paneCloseErrorCode;
if (code != null) {
throw new HerdrException("herdr error [" + code + "]: pane.close failed",
code, null);
}
yield mapper.readTree("{\"type\":\"ok\"}");
}
@@ -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<FleetConfig.MemberCredentials> creds = () -> new FleetConfig.MemberCredentials(
FleetConfig.MemberCredentials.POLICY_ALLOW_LIST, List.of(), List.of(), null);
Function<String, String> 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<String, Object> tabCreateParams = (Map<String, Object>) herdr.lastCall("tab.create").params();
Map<String, String> tabEnv = (Map<String, String>) 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<ILoggingEvent> 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
@@ -1100,7 +1165,8 @@ class ClaudeCodeLauncherTest {
@Test
void spawnLetsAnUnrelatedHerdrErrorPropagateUnchanged() {
// Fix 1 must only special-case a "*_not_found" answer. Any other herdr failure keeps
// propagating as-is — this gate does not know how to recover from it.
// propagating as-is — this gate does not know how to recover from it. The pane still needs
// closing because spawn throws before it can return the pane id to a caller that could stop it.
FakeHerdr herdr = new FakeHerdr();
herdr.agentStatus("unknown");
herdr.agentGetFailsWithAfter(0, "internal_error");
@@ -1117,8 +1183,27 @@ class ClaudeCodeLauncherTest {
() -> svc.spawn(new SpawnRequest(null, null, null)));
assertEquals("internal_error", ex.code());
assertEquals(0, paneCloseCount(herdr, "w9:pRoot_1"),
"an error this gate does not recognize is not this gate's teardown to run");
assertEquals(1, paneCloseCount(herdr, "w9:pRoot_1"),
"the unchanged error leaves spawn without a pane id, so this gate closes its orphaned pane");
}
@Test
void panePlacementClosesTheSplitPaneWhenAgentStartFails() {
FakeHerdr herdr = new FakeHerdr().agentStartFailsWith("internal_error");
FleetConfig.Profile cfg = new FleetConfig.Profile(
"ltms-local", "http://gx00.gw:8000", "coder", null, "FLEETD_WORKER_TOKEN",
List.of("claude"), "pane", "fleetd-workers", "w #{n}", null, null, null);
ClaudeCodeLauncher svc = new ClaudeCodeLauncher(
new AgentControl(herdr), new WorkspaceControl(herdr),
new SubscriptionGuard(Set.of("gx00.gw")), Map.of(cfg.profile(), cfg), cfg.profile(), _ -> null);
dev.ltms.fleet.herdr.HerdrException ex = assertThrows(
dev.ltms.fleet.herdr.HerdrException.class,
() -> svc.spawn(new SpawnRequest(null, null, null)));
assertEquals("internal_error", ex.code(), "agent.start failure propagates unchanged");
assertEquals(1, paneCloseCount(herdr, "w1:pSplit"),
"the pane split for a peer that never starts is closed instead of left orphaned");
}
// --- fleetd #176 fix 2: corroborated UNKNOWN refinement --------------------------------------
@@ -1069,6 +1069,79 @@ class SessionManagerTest {
assertEquals(1, paneCloseCallsFor(herdr, "w9:pRoot_3"), "the third pane is stopped");
}
/**
* fleetd #290: the #283 fix above closed the one trigger this suite used for {@code
* reapIdle}'s own per-session try/catch (CB-581) — a worktree-removal failure is now caught
* and logged inside {@code release()} itself, so it never reaches {@code reapIdle}'s guard at
* all. This test restores coverage of that guard using the trigger the ticket names: {@code
* release()} calls {@code launcher.stop(paneId)} with no try/catch around it, so a failing
* {@code pane.close} propagates straight out of {@code release()} uncaught. {@link
* FakeHerdr#paneCloseFailsForPane} (added for this ticket) makes exactly the middle session's
* stop fail, while the other two still succeed, so this proves {@code reapIdle} keeps reaping
* the rest of the roster rather than aborting the whole pass.
*/
@Test
void reapIdleSurvivesOneSessionWhoseLauncherStopFails() {
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-290a", null));
MemberSession b = sessions.acquire("ltms-local", null, "/caller/proj", null,
new WorktreeRequest("cb-290b", null));
MemberSession c = sessions.acquire("ltms-local", null, "/caller/proj", null,
new WorktreeRequest("cb-290c", null));
sessions.asPresence().markPresent(a.terminalId());
sessions.asPresence().markPresent(b.terminalId());
sessions.asPresence().markPresent(c.terminalId());
// Only the middle session's herdr pane fails to close — a and c stop normally. This is the
// 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 {
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("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,
"the middle session's launcher.stop failure is not counted as reaped, but must not "
+ "abort reaping the other two");
assertTrue(sessions.get(a.paneId()).isEmpty(), "the first session is still released");
assertTrue(sessions.get(c.paneId()).isEmpty(),
"the third session is still reached and released — proves the pass did not abort "
+ "when the middle session's release() threw");
assertTrue(sessions.get(b.paneId()).isEmpty(),
"the middle session is still deregistered — release() removes it from the registry "
+ "before launcher.stop() runs, regardless of whether stop() then throws");
assertEquals(1, paneCloseCallsFor(herdr, "w9:pRoot_1"), "the first pane is stopped");
assertEquals(1, paneCloseCallsFor(herdr, "w9:pRoot_2"),
"the middle pane's stop was attempted, even though it failed");
assertEquals(1, paneCloseCallsFor(herdr, "w9:pRoot_3"), "the third pane is stopped");
assertEquals(List.of(a.worktree(), c.worktree()), worktrees.removeCalls().stream().sorted().toList(),
"the middle session's worktree removal never runs — release() throws before reaching "
+ "it — while the other two, unaffected, still have theirs removed");
}
@Test
void unchangedRegressionCleanCompletedReleaseStillRemovesTheWorktree() {
FakeHerdr herdr = new FakeHerdr();
@@ -1083,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 <em>before</em> 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();