fleetd #296: close panes on failed spawn

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.

fleetd/src/main/java/dev/ltms/fleet/health/FleetHealthMonitor.java

@@ -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;
@@ -28,6 +29,14 @@ public final class FleetHealthMonitor {
     static final int MAX_FAIL_TARGET_ATTEMPTS = 3;
     // CB-641: Match the injector's 60s readiness gate so health allows a full first boot.
     static final long READINESS_GRACE_NANOS = TimeUnit.SECONDS.toNanos(60);
+    /**
+     * fleetd #280: how long after a terminal transition to wait before the one bounded re-check
+     * fires. Must exceed the worst-case reverse-rendezvous {@code fleet_ask} window (55-115s, see
+     * {@code FleetMcp.ASK_DEFAULT_TIMEOUT_MS} / {@code FleetApp.MAX_ASK_TIMEOUT_MS}) so that, if the
+     * target was genuinely {@code ASKING} when {@code state} was first observed, its own ask has had
+     * time to lapse (clearing {@code Task#question} back to {@code null}) before this fires.
+     */
+    static final long ASK_LAPSE_RECHECK_DELAY_SECONDS = 120;
 
     private final AgentControl agents;
     private final Supplier<List<MemberSession>> roster;
@@ -38,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
@@ -171,6 +189,7 @@ public final class FleetHealthMonitor {
         // member stayed terminal.
         if (terminal(next)) {
             failTerminalTarget(target, next);
+            scheduleTerminalRecheck(target, next);
         }
     }
 
@@ -191,6 +210,48 @@ public final class FleetHealthMonitor {
                 target, state, MAX_FAIL_TARGET_ATTEMPTS, last);
     }
 
+    /**
+     * fleetd #280: schedule the one bounded, delayed follow-up for a terminal transition — never a
+     * per-tick retry (CB-580 rejected that shape; {@link #reportTransition} still fires
+     * {@link #failTerminalTarget} exactly once per transition, unconditionally on the tick loop).
+     * This is a single one-shot task, scheduled once per transition into GONE/NEVER_READY, so a
+     * member stuck terminal for the rest of its life gets exactly one extra attempt, not one per
+     * tick. See {@link #recheckTerminalTarget} for why the extra attempt is safe.
+     */
+    private void scheduleTerminalRecheck(String target, HealthState state) {
+        if (scheduler.isShutdown()) return;
+        try {
+            scheduler.schedule(() -> recheckTerminalTarget(target, state),
+                    ASK_LAPSE_RECHECK_DELAY_SECONDS, TimeUnit.SECONDS);
+        } catch (RuntimeException e) {
+            log.warn("fleet health: could not schedule terminal re-check for member={} state={}",
+                    target, state, e);
+        }
+    }
+
+    /**
+     * fleetd #280: the delayed re-check {@link #scheduleTerminalRecheck} scheduled for one terminal
+     * transition. By now, a {@code fleet_ask} that was still open when {@code state} was first
+     * observed has had time to lapse on its own (see {@link #ASK_LAPSE_RECHECK_DELAY_SECONDS}),
+     * clearing {@code Task#question} back to {@code null} — which is exactly what
+     * {@link MessageService#abandon(String, String, boolean)}'s {@code sweepAsking=false} filter
+     * needs to finally match it. Calling {@link #failTerminalTarget} again is safe only because
+     * {@code sweepAsking} stays {@code false}: a task genuinely still {@code ASKING} is skipped
+     * exactly as it was on the very first attempt — this never fails a ticket whose ask has not yet
+     * lapsed.
+     *
+     * <p><strong>Guarded on "target is still classified {@code state}."</strong> Without this guard,
+     * a member that recovered (or was released and dropped from the roster) between the transition
+     * and this re-check would still take a blind {@code failTarget} call — reaching into whatever
+     * brand-new, unrelated turn it has since picked up and failing it too. {@link #states} already
+     * carries the live classification (updated every tick, pruned to the current roster on release),
+     * so a stale or recovered target simply reads as a mismatch here and this is a no-op.
+     */
+    void recheckTerminalTarget(String target, HealthState state) {
+        if (states.get(target) != state) return;
+        failTerminalTarget(target, state);
+    }
+
     private static boolean terminal(HealthState state) {
         return state == HealthState.GONE || state == HealthState.NEVER_READY;
     }

fleetd/src/main/java/dev/ltms/fleet/member/HerdrPeerLauncher.java

@@ -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;
     }
@@ -947,7 +960,14 @@ public abstract class HerdrPeerLauncher implements PeerLauncher {
             // person can go close by hand.
             try {
                 spaces.closeTab(loc.tabId());
-            } catch (HerdrException e) {
+            } 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());
             }
@@ -989,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();
@@ -1003,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)) {

fleetd/src/test/java/dev/ltms/fleet/health/FleetHealthMonitorTest.java

@@ -24,7 +24,9 @@ import org.slf4j.LoggerFactory;
 import java.util.List;
 import java.util.Map;
 import java.util.Set;
+import java.util.concurrent.CompletableFuture;
 import java.util.concurrent.Executors;
+import java.util.concurrent.ScheduledFuture;
 import java.util.concurrent.ScheduledThreadPoolExecutor;
 import java.util.concurrent.TimeUnit;
 import java.util.concurrent.atomic.AtomicLong;
@@ -434,4 +436,120 @@ class FleetHealthMonitorTest {
             logger.detachAppender(appender);
         }
     }
+
+    // --- fleetd #280: a GONE/NEVER_READY guess whose fleet_ask lapses AFTER the first sweep must
+    // still be swept, without ever reaching into a target that has since recovered or left the
+    // roster. See FleetHealthMonitor.recheckTerminalTarget's javadoc for the full reachability chain.
+
+    @Test void terminalTransitionSchedulesExactlyOneDelayedRecheck() {
+        ScheduledThreadPoolExecutor scheduler = new ScheduledThreadPoolExecutor(1);
+        FleetHealthMonitor monitor = monitor(new FakeHerdr(),
+                List.of(member("term_a", MemberSession.State.BUSY, 0, 0)), scheduler, () -> 1, 600,
+                (_, _) -> { });
+
+        monitor.reportTransition("term_a", HealthState.GONE);
+
+        // Driven via reportTransition directly (not tick()), so the queue holds only the recheck.
+        assertEquals(1, scheduler.getQueue().size());
+        ScheduledFuture<?> scheduled = (ScheduledFuture<?>) scheduler.getQueue().peek();
+        assertTrue(scheduled.getDelay(TimeUnit.SECONDS) > 100,
+                "the delay must clear the worst-case fleet_ask lapse window (up to 115s)");
+
+        // An unchanged tick must not queue a second one (CB-580's fire-once rule extends to this).
+        monitor.reportTransition("term_a", HealthState.GONE);
+        assertEquals(1, scheduler.getQueue().size());
+        monitor.stop();
+    }
+
+    @Test void recheckIsANoOpOnceTheTargetHasRecovered() {
+        RecordingFailTarget failTarget = new RecordingFailTarget();
+        FleetHealthMonitor monitor = monitorWith(failTarget);
+        monitor.reportTransition("term_a", HealthState.GONE);
+        assertEquals(1, failTarget.calls.size());
+
+        monitor.reportTransition("term_a", HealthState.IDLE); // recovered before the recheck fired
+        monitor.recheckTerminalTarget("term_a", HealthState.GONE);
+
+        assertEquals(1, failTarget.calls.size(), "a recovered target must not be reached into again");
+        monitor.stop();
+    }
+
+    @Test void recheckIsANoOpForATargetItNeverObserved() {
+        // Mirrors "left the roster": tick() prunes states.keySet() to the current roster on release
+        // (see FleetHealthMonitor.tick), so a target this monitor never recorded is the same case.
+        RecordingFailTarget failTarget = new RecordingFailTarget();
+        FleetHealthMonitor monitor = monitorWith(failTarget);
+
+        monitor.recheckTerminalTarget("term_never_seen", HealthState.GONE);
+
+        assertEquals(0, failTarget.calls.size(), "an untracked/released target must not be reached into");
+        monitor.stop();
+    }
+
+    /**
+     * The scenario from the ticket, end to end, driven through the real {@link MessageService}: a
+     * target's ask is still genuinely open when health first observes GONE (sweep must skip it,
+     * exactly as {@code abandonDoesNotFailAnAsyncTicketWaitingForAnAnswer} pins), the ask then lapses
+     * on its own, an unchanged tick still must not refire, and only the delayed recheck sweeps the
+     * now-lapsed ticket to FAILED.
+     */
+    @Test void delayedRecheckSweepsATicketWhoseAskLapsedAfterGoneWasFirstObserved() throws Exception {
+        FakeHerdr herdr = new FakeHerdr().withAgent("worker", "term_a", "pane-term_a", "tab_a")
+                .readText("$ prompt");
+        AgentControl agents = new AgentControl(herdr);
+        Rendezvous rendezvous = new Rendezvous();
+        Injector injector = new Injector(agents);
+        InMemoryReplyInbox inbox = new InMemoryReplyInbox();
+        inbox.own("term_a");
+        MessageService messages = new MessageService(agents, injector, rendezvous, inbox);
+        FleetHealthMonitor monitor = new FleetHealthMonitor(agents,
+                () -> List.of(member("term_a", MemberSession.State.READY, 0, 0)), messages,
+                new ScheduledThreadPoolExecutor(1), () -> 1, 60, 600, messages::abandon);
+
+        String ticket = messages.sendAsync("term_a", "task that asks");
+        long deadline = System.currentTimeMillis() + 2000;
+        while (!rendezvous.isWaiting("term_a") && System.currentTimeMillis() < deadline) {
+            Thread.sleep(5);
+        }
+        assertTrue(rendezvous.isWaiting("term_a"), "the async send should have opened its waiter");
+        injector.onStatus("term_a", AgentStatus.IDLE);    // deliver the task
+        injector.onStatus("term_a", AgentStatus.WORKING); // the worker picks it up
+
+        // The worker asks, with a short timeout so its own fleet_ask lapses quickly in test time.
+        CompletableFuture<MessageService.AskResult> ask = CompletableFuture.supplyAsync(
+                () -> messages.ask("term_a", "which config?", 200));
+        awaitPhase(messages, ticket, MessageService.Phase.ASKING);
+
+        monitor.reportTransition("term_a", HealthState.GONE);
+        assertEquals(MessageService.Phase.ASKING, messages.poll(ticket).phase(),
+                "the first sweep must not fail a ticket that is still genuinely being asked");
+
+        // The worker's own fleet_ask now lapses on its own — task.question clears to null.
+        assertEquals(MessageService.AskOutcome.TIMED_OUT, ask.get(5, TimeUnit.SECONDS).outcome());
+
+        // An unchanged tick still must not refire (CB-580).
+        monitor.reportTransition("term_a", HealthState.GONE);
+        assertEquals(MessageService.Phase.PENDING, messages.poll(ticket).phase());
+
+        // The delayed recheck scheduled for the original transition finally sweeps it.
+        monitor.recheckTerminalTarget("term_a", HealthState.GONE);
+        assertEquals(MessageService.Phase.FAILED, messages.poll(ticket).phase());
+
+        monitor.stop();
+    }
+
+    private static MessageService.TaskView awaitPhase(MessageService messages, String ticket,
+            MessageService.Phase phase) throws InterruptedException {
+        long deadline = System.currentTimeMillis() + 2000;
+        MessageService.TaskView view;
+        do {
+            view = messages.poll(ticket);
+            if (view.phase() == phase) {
+                return view;
+            }
+            Thread.sleep(5);
+        } while (System.currentTimeMillis() < deadline);
+        assertEquals(phase, view.phase());
+        return view;
+    }
 }

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

@@ -40,6 +40,7 @@ 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<>();
@@ -84,6 +85,12 @@ 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;
@@ -311,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(

fleetd/src/test/java/dev/ltms/fleet/member/ClaudeCodeLauncherTest.java

@@ -1165,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");
@@ -1182,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 --------------------------------------