From 25726a5ae7bdea9219e81057375eb145ff5a5b45 Mon Sep 17 00:00:00 2001 From: Ha Trong Dai Date: Fri, 28 Aug 2026 09:36:39 +0700 Subject: [PATCH] #185: reject ambiguous unowned pane ids --- .../fleet/member/CompositePeerLauncher.java | 12 +++-- .../member/CompositePeerLauncherTest.java | 48 +++++++++++++++++++ 2 files changed, 56 insertions(+), 4 deletions(-) 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 1469b30..465a6fe 100644 --- a/fleetd/src/main/java/dev/ltms/fleet/member/CompositePeerLauncher.java +++ b/fleetd/src/main/java/dev/ltms/fleet/member/CompositePeerLauncher.java @@ -44,9 +44,9 @@ 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 (only real for a caller that - * hand-rolls an id) falls back to the first delegate; teardown is pane-id addressed and - * tab cleanup is single-occupant guarded, so it is safe either way.
  • + * (recorded at spawn time). A pane the composite never spawned can use the single delegate + * in a one-daemon fleet. With more than one delegate, 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.
  • @@ -437,7 +437,11 @@ public final class CompositePeerLauncher implements PeerLauncher { public void stop(String id) { HerdrPeerLauncher d = spawnedBy.remove(id); if (d == null) { - log.debug("stop({}) — no recorded owner, routing to the first adapter (pane-addressed)", id); + if (delegates.size() != 1) { + throw new IllegalArgumentException("ambiguous paneId '" + id + + "': no owning herdr daemon was recorded"); + } + log.debug("stop({}) — no recorded owner in a single-daemon fleet", id); d = delegates.getFirst(); } d.stop(id); 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 48ed960..060fc28 100644 --- a/fleetd/src/test/java/dev/ltms/fleet/member/CompositePeerLauncherTest.java +++ b/fleetd/src/test/java/dev/ltms/fleet/member/CompositePeerLauncherTest.java @@ -248,6 +248,54 @@ class CompositePeerLauncherTest { "stop routes to the spawning adapter and closes exactly that worker's pane"); } + @Test + void stopKeepsSameHerdrPaneIdSeparateByOwningAdapter() { + // Separate herdr daemons can both issue w9:pRoot_1. The opaque handles identify their + // spawning adapters, so each stop reaches only its recorded owner. + FakeHerdr first = new FakeHerdr(); + FakeHerdr second = new FakeHerdr(); + PeerLauncher composite = new CompositePeerLauncher( + List.of(claudeAdapter(first), opencodeAdapter(second)), "claude"); + + PeerHandle claude = composite.spawn(new SpawnRequest("claude", null, null)); + PeerHandle opencode = composite.spawn(new SpawnRequest("gemini", null, null)); + + assertNotEquals(claude.id(), opencode.id(), "each public paneId keeps its adapter owner"); + composite.stop(claude.id()); + assertTrue(first.calls.stream().anyMatch(c -> c.method().equals("pane.close") + && "w9:pRoot_1".equals(((Map) c.params()).get("pane_id"))), + "the first daemon closes its own pane"); + assertFalse(second.called("pane.close"), "the matching pane on the second daemon stays live"); + + composite.stop(opencode.id()); + assertTrue(second.calls.stream().anyMatch(c -> c.method().equals("pane.close") + && "w9:pRoot_1".equals(((Map) c.params()).get("pane_id"))), + "the second daemon then closes its own pane"); + } + + @Test + void stopAllowsALegacyBarePaneIdWithOneDaemon() { + FakeHerdr herdr = new FakeHerdr(); + PeerLauncher composite = new CompositePeerLauncher(List.of(claudeAdapter(herdr)), "claude"); + + composite.stop("w9:pRoot_1"); + + assertTrue(herdr.calls.stream().anyMatch(c -> c.method().equals("pane.close") + && "w9:pRoot_1".equals(((Map) c.params()).get("pane_id"))), + "one daemon keeps the legacy bare-pane routing behaviour"); + } + + @Test + void stopRejectsAnUnownedPaneIdWhenMultipleDaemonsCouldOwnIt() { + PeerLauncher composite = new CompositePeerLauncher( + List.of(claudeAdapter(new FakeHerdr()), opencodeAdapter(new FakeHerdr())), "claude"); + + IllegalArgumentException error = assertThrows(IllegalArgumentException.class, + () -> composite.stop("w1:p1")); + + assertEquals("ambiguous paneId 'w1:p1': no owning herdr daemon was recorded", error.getMessage()); + } + @Test void opencodeContextResetIsANoOpAndWarnsOnlyOnce() { FakeHerdr herdr = new FakeHerdr();