diff --git a/fleetd/src/main/java/dev/ltms/fleet/member/CompositePeerLauncher.java b/fleetd/src/main/java/dev/ltms/fleet/member/CompositePeerLauncher.java index c329741..ee83b17 100644 --- a/fleetd/src/main/java/dev/ltms/fleet/member/CompositePeerLauncher.java +++ b/fleetd/src/main/java/dev/ltms/fleet/member/CompositePeerLauncher.java @@ -3,6 +3,7 @@ package dev.ltms.fleet.member; import dev.ltms.fleet.config.FleetConfig; import dev.ltms.fleet.herdr.Agent; import dev.ltms.fleet.herdr.HerdrClient; +import dev.ltms.fleet.herdr.HerdrException; import dev.ltms.fleet.peer.Capability; import dev.ltms.fleet.peer.MemberRole; import dev.ltms.fleet.peer.PeerHandle; @@ -46,9 +47,13 @@ import java.util.stream.Collectors; * the single adapter that declares it. Profiles partition cleanly across adapters: the * constructor rejects a name claimed by two. *
  • By pane id — {@link #stop} routes to the adapter that spawned that pane - * (recorded at spawn time). A pane the composite never spawned can use the fallback route - * in a one-daemon fleet. With more than one herdr daemon, its owner is unknown, so stop refuses - * the ambiguous id rather than closing a pane on an arbitrary herdr daemon.
  • + * (recorded at spawn time). A pane the composite never spawned, or one whose record was lost + * to a daemon restart (CB-185 blocker 1 — {@link #spawnedBy} is in-memory only), can use the + * fallback route in a one-daemon fleet. With more than one herdr daemon, {@link #probeOwner} + * asks each configured daemon which one actually knows the pane: exactly one match routes + * (and caches); no match is treated as already-gone; more than one match is a genuine + * ambiguity (pane ids are per-daemon counters, so two daemons really can both hold, say, + * {@code w1:p1}) and stop refuses rather than closing a pane on an arbitrary herdr daemon. *
  • Fleet-wide — {@link #reapOrphanWorkers} and {@link #capabilities} fan out * and combine. {@link #list} is deduplicated by (owning daemon, pane id): delegates that share * one herdr connection report the same global agent set, but two daemons can each hold a pane @@ -476,6 +481,11 @@ public final class CompositePeerLauncher implements PeerLauncher { * probed twice, and a pane on their shared daemon would look owned by two adapters instead of * one daemon. * + *

    A daemon that fails to answer {@code list()} (e.g. it is down) is treated as "does not know + * this pane" rather than aborting the whole probe — one unreachable daemon must never make a + * pane that a different, healthy daemon actually owns un-stoppable too, which would + * resurrect the exact bug this method exists to fix. + * * @return the owning delegate — cached into {@link #spawnedBy} so the next call is free — or * {@code null} when no daemon knows the pane * @throws IllegalArgumentException when more than one daemon claims the pane: pane ids are @@ -489,15 +499,23 @@ public final class CompositePeerLauncher implements PeerLauncher { } List owners = new ArrayList<>(); for (HerdrPeerLauncher representative : byDaemon.values()) { - boolean knows = representative.list().stream() - .anyMatch(a -> id.equals(a.paneId())); + List agents; + try { + agents = representative.list(); + } catch (HerdrException e) { + log.warn("stop({}) probe: a configured herdr daemon was unreachable ({}); " + + "treating it as not knowing this pane", id, e.getClass().getSimpleName()); + continue; + } + boolean knows = agents.stream().anyMatch(a -> id.equals(a.paneId())); if (knows) { owners.add(representative); } } if (owners.size() > 1) { - throw new IllegalArgumentException("ambiguous paneId '" + id - + "': no owning herdr daemon was recorded"); + throw new IllegalArgumentException("ambiguous paneId '" + id + "': " + + owners.size() + " configured herdr daemons report this pane — " + + "no way to tell which one the caller means"); } if (owners.isEmpty()) { return null; diff --git a/fleetd/src/test/java/dev/ltms/fleet/member/CompositePeerLauncherTest.java b/fleetd/src/test/java/dev/ltms/fleet/member/CompositePeerLauncherTest.java index cf6cbe7..1e791d5 100644 --- a/fleetd/src/test/java/dev/ltms/fleet/member/CompositePeerLauncherTest.java +++ b/fleetd/src/test/java/dev/ltms/fleet/member/CompositePeerLauncherTest.java @@ -357,7 +357,8 @@ class CompositePeerLauncherTest { IllegalArgumentException error = assertThrows(IllegalArgumentException.class, () -> composite.stop("w1:p1")); - assertEquals("ambiguous paneId 'w1:p1': no owning herdr daemon was recorded", error.getMessage()); + assertEquals("ambiguous paneId 'w1:p1': 2 configured herdr daemons report this pane — " + + "no way to tell which one the caller means", error.getMessage()); } @Test @@ -394,6 +395,24 @@ class CompositePeerLauncherTest { assertFalse(second.called("pane.close"), "the daemon that never held the pane is never touched"); } + @Test + void aProbeSurvivesOneUnreachableDaemonAndStillFindsTheOwnerOnTheOtherOne() { + // CB-185 blocker 1 (lead review): a daemon that is DOWN while we probe must not abort the + // whole probe — the pane the OPERATOR actually wants stopped can live on a different, + // healthy daemon, and that pane must not become un-stoppable because a third one is down. + FakeHerdr down = new FakeHerdr().healthy(false); + FakeHerdr owner = new FakeHerdr().withAgent("x", "term_x", "w1:p1", "w1:t1"); + PeerLauncher composite = new CompositePeerLauncher( + List.of(claudeAdapter(down), opencodeAdapter(owner)), "claude"); + + assertDoesNotThrow(() -> composite.stop("w1:p1"), + "the unreachable daemon must be skipped, not fail the whole stop"); + + assertTrue(owner.calls.stream().anyMatch(c -> c.method().equals("pane.close") + && "w1:p1".equals(((Map) c.params()).get("pane_id"))), + "the healthy daemon that actually owns the pane still closes it"); + } + @Test void aProbedOwnerIsCachedSoARetryAfterAFailedStopNeedsNoSecondProbe() { // CB-185 blocker 1: the probe's whole point is to be cheap on repeat — a failed stop (e.g.