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 30daf3b..70cf73e 100644 --- a/fleetd/src/main/java/dev/ltms/fleet/member/CompositePeerLauncher.java +++ b/fleetd/src/main/java/dev/ltms/fleet/member/CompositePeerLauncher.java @@ -50,8 +50,9 @@ import java.util.stream.Collectors; * 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. *
  • Fleet-wide — {@link #reapOrphanWorkers} and {@link #capabilities} fan out - * and combine. {@link #list} is deduplicated by pane id because every herdr-backed delegate - * shares one herdr connection and so reports the same global agent set.
  • + * 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 + * called {@code w1:p1}, so the daemon has to be part of the key. * * *

    CB-518: an unqualified spawn is routed through a {@link PlacementPolicy}. The default @@ -437,7 +438,7 @@ public final class CompositePeerLauncher implements PeerLauncher { @Override public void stop(String id) { - HerdrPeerLauncher d = spawnedBy.remove(id); + HerdrPeerLauncher d = spawnedBy.get(id); if (d == null) { if (herdrDaemonCount() != 1) { throw new IllegalArgumentException("ambiguous paneId '" + id @@ -446,7 +447,11 @@ public final class CompositePeerLauncher implements PeerLauncher { log.debug("stop({}) — no recorded owner in a single-daemon fleet", id); d = delegates.getFirst(); } + // Drop the owner record only after the delegate accepted the stop. Removing it first meant a + // delegate that threw left the pane alive with its owner forgotten, so the retry fell into + // the ambiguous branch above and refused the id for good. d.stop(id); + spawnedBy.remove(id); } /** @@ -454,7 +459,7 @@ public final class CompositePeerLauncher implements PeerLauncher { * objects may represent different daemons even if a client later implements value equality. */ private int herdrDaemonCount() { - Set daemons = java.util.Collections.newSetFromMap(new IdentityHashMap<>()); + Set daemons = Collections.newSetFromMap(new IdentityHashMap<>()); for (HerdrPeerLauncher delegate : delegates) { daemons.add(delegate.herdr()); } @@ -493,14 +498,24 @@ public final class CompositePeerLauncher implements PeerLauncher { return route(profileName).capabilities(); } - /** Every herdr agent, deduplicated by pane id (all delegates share one herdr and list globally). */ + /** + * Every herdr agent, deduplicated by (owning daemon, pane id). + * + *

    Delegates that share one {@link HerdrClient} see the same global agent set, so listing them + * both would report every agent twice — that is what the dedupe is for. But pane ids are + * per-daemon counters, so two daemons really can both hold {@code w1:p1} on different panes. + * Keying on the pane id alone would silently drop one of them from {@code fleet_list} and from + * every status view built on it. The daemon is part of the key for exactly that reason. + */ @Override public List list() { + Map daemonIndex = new IdentityHashMap<>(); Map byPane = new LinkedHashMap<>(); for (HerdrPeerLauncher d : delegates) { + int daemon = daemonIndex.computeIfAbsent(d.herdr(), _ -> daemonIndex.size()); for (Agent a : d.list()) { if (a.paneId() != null) { - byPane.putIfAbsent(a.paneId(), a); + byPane.putIfAbsent(daemon + "\u0000" + a.paneId(), a); } } } 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 8fc49ef..ade228e 100644 --- a/fleetd/src/test/java/dev/ltms/fleet/member/CompositePeerLauncherTest.java +++ b/fleetd/src/test/java/dev/ltms/fleet/member/CompositePeerLauncherTest.java @@ -8,6 +8,7 @@ import dev.ltms.fleet.guard.SubscriptionGuard; import dev.ltms.fleet.herdr.Agent; import dev.ltms.fleet.herdr.AgentControl; import dev.ltms.fleet.herdr.FakeHerdr; +import dev.ltms.fleet.herdr.HerdrException; import dev.ltms.fleet.herdr.WorkspaceControl; import dev.ltms.fleet.peer.Capability; import dev.ltms.fleet.peer.CharterReceipt; @@ -297,6 +298,52 @@ class CompositePeerLauncherTest { "two adapter kinds sharing one daemon keep the fallback route"); } + @Test + void listKeepsBothPanesWhenTwoDaemonsShareAPaneId() { + // herdr pane ids are per-daemon counters, so two daemons really can both hold w1:p1 on + // different panes. Deduplicating on the pane id alone dropped one of the two real agents. + FakeHerdr first = new FakeHerdr().withAgent("x", "term_x", "w1:p1", "w1:t1"); + FakeHerdr second = new FakeHerdr().withAgent("y", "term_y", "w1:p1", "w1:t1"); + CompositePeerLauncher composite = new CompositePeerLauncher( + List.of(claudeAdapter(first), opencodeAdapter(second)), "claude"); + + List agents = composite.list(); + + assertEquals(2, agents.stream().filter(a -> "w1:p1".equals(a.paneId())).count(), + "one w1:p1 per daemon survives — the pane id alone is not a unique key"); + assertTrue(agents.stream().anyMatch(a -> "term_x".equals(a.terminalId()))); + assertTrue(agents.stream().anyMatch(a -> "term_y".equals(a.terminalId()))); + } + + @Test + void listStillDeduplicatesTwoAdaptersSharingOneDaemon() { + // Both adapters ask the SAME daemon, so both see the same agent set. Without the dedupe this + // would report every agent twice; the daemon key must not break that. + FakeHerdr herdr = new FakeHerdr().withAgent("x", "term_x", "w1:p1", "w1:t1"); + CompositePeerLauncher composite = composite(herdr); + + List agents = composite.list(); + + assertEquals(1, agents.stream().filter(a -> "w1:p1".equals(a.paneId())).count(), + "one daemon still reports each of its agents once"); + } + + @Test + void stopKeepsTheOwnerRecordWhenTheDelegateRefusesTheStop() { + // Removing the record before the delegate accepted the stop lost the owner on failure: the + // pane was still alive, but the retry landed in the ambiguous branch and refused it for good. + FakeHerdr first = new FakeHerdr().paneCloseFailsWith("pane_busy"); + CompositePeerLauncher composite = new CompositePeerLauncher( + List.of(claudeAdapter(first), opencodeAdapter(new FakeHerdr())), "claude"); + PeerHandle claude = composite.spawn(new SpawnRequest("claude", null, null)); + + assertThrows(HerdrException.class, () -> composite.stop(claude.id())); + + // The retry must still know its owner — a HerdrException, never "ambiguous paneId". + assertThrows(HerdrException.class, () -> composite.stop(claude.id()), + "the owner record survives a failed stop, so the retry is not ambiguous"); + } + @Test void stopRejectsAnUnownedPaneIdWhenMultipleDaemonsCouldOwnIt() { PeerLauncher composite = new CompositePeerLauncher(