#185: reject ambiguous unowned pane ids
This commit is contained in:
@@ -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.</li>
|
||||
* <li><strong>By pane id</strong> — {@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.</li>
|
||||
* (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.</li>
|
||||
* <li><strong>Fleet-wide</strong> — {@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.</li>
|
||||
@@ -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);
|
||||
|
||||
@@ -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();
|
||||
|
||||
Reference in New Issue
Block a user