CB-519: make PeerHandle.id() a host-unique opaque UUID, decoupled from the herdr pane id

This commit is contained in:
Dai Ha
2026-08-04 18:11:15 +02:00
parent 54b314ace5
commit 7ace184fe6
11 changed files with 156 additions and 41 deletions
@@ -31,6 +31,9 @@ public final class FakeHerdr implements HerdrClient {
private String agentSendErrorCode = null;
private volatile String agentStatus = "idle"; // steady-state agent.get status
private volatile String readText = "worker transcript tail"; // canned agent.read output
private int pinnedStarts = 0; // how many upcoming agent.start calls report a fixed pane
private String pinnedStartTerminal;
private String pinnedStartPane;
public FakeHerdr healthy(boolean h) {
this.healthy = h;
@@ -79,6 +82,19 @@ public final class FakeHerdr implements HerdrClient {
return this;
}
/**
* Force the next {@code n} {@code agent.start} calls to report this terminal/pane coordinate,
* instead of the fake's usual incrementing {@code term_new_n}/{@code w9:pW_n}. Lets a test make
* two spawns report the <em>same</em> herdr pane, to prove the host-unique id (CB-519) never
* collides on that coordinate.
*/
public FakeHerdr pinNextStarts(int n, String terminalId, String paneId) {
this.pinnedStarts = n;
this.pinnedStartTerminal = terminalId;
this.pinnedStartPane = paneId;
return this;
}
/**
* Seed a named agent into {@code agent.list} (e.g. an orphaned worker for CB-117 reaper tests).
@@ -175,12 +191,20 @@ public final class FakeHerdr implements HerdrClient {
"agent_name_taken", null);
}
long n = busyAdjusted - agentNameTakenFor;
// The agent starts INTO the requested pane, so its pane_id echoes the param.
boolean pinned = pinnedStarts > 0;
if (pinned) {
pinnedStarts--;
}
// Protocol 19: the agent starts INTO the requested pane, so its pane_id normally
// echoes the param. A pin overrides both coordinates, which is the only way to
// make two spawns report one pane — what CB-519's collision test needs.
String terminal = pinned ? pinnedStartTerminal : ("term_new_" + n);
Object pane = pinned ? pinnedStartPane : p.get("pane_id");
yield mapper.readTree(("""
{"type":"agent_started","agent":{
"terminal_id":"term_new_%d","name":"claude","agent_status":"unknown",
"terminal_id":"%s","name":"claude","agent_status":"unknown",
"workspace_id":"w9","tab_id":"w9:t2","pane_id":"%s"}}""")
.formatted(n, p.get("pane_id")));
.formatted(terminal, pane));
}
case "pane.split" -> mapper.readTree("""
{"type":"pane_info","pane":{"pane_id":"w1:pSplit","workspace_id":"w1",
@@ -216,12 +216,15 @@ class BridgeMcpTest {
@Test
void spawnReturnsTheNewWorkersSessionAndPane() {
FakeHerdr h = new FakeHerdr();
McpSchema.CallToolResult res = BridgeMcp.spawn(
sessionManager(h, "http://gx00.gw:8000", Set.of("gx00.gw")), null);
SessionManager sm = sessionManager(h, "http://gx00.gw:8000", Set.of("gx00.gw"));
McpSchema.CallToolResult res = BridgeMcp.spawn(sm, null);
assertNotEquals(Boolean.TRUE, res.isError());
String out = textOf(res);
assertTrue(out.contains("\"sessionId\":\"term_new_1\""), out);
assertTrue(out.contains("\"paneId\":\"w9:pRoot_1\""), out);
// CB-519: the "paneId" wire field now carries the host-unique opaque id, not the herdr pane.
WorkerSession s = sm.roster().getFirst();
assertTrue(out.contains("\"paneId\":\"" + s.paneId() + "\""), out);
assertNotEquals("w9:pRoot_1", s.paneId(), "the id is decoupled from the herdr pane coordinate");
assertTrue(out.contains("\"status\":\"spawning\""), out);
}
@@ -28,6 +28,7 @@ import java.net.http.HttpResponse;
import java.util.List;
import java.util.Map;
import java.util.Set;
import java.util.UUID;
import static org.junit.jupiter.api.Assertions.*;
@@ -156,7 +157,10 @@ class BridgedAppTest {
HttpResponse<String> res = req(port, "POST", "/workers");
assertEquals(201, res.statusCode());
JsonNode body = mapper.readTree(res.body());
assertEquals("w9:pRoot_1", body.get("paneId").asText());
// CB-519: the responded paneId is a host-unique opaque UUID, not the herdr pane coordinate.
String id = body.get("paneId").asText();
assertNotEquals("w9:pRoot_1", id, "paneId is the host-unique id, not the herdr pane");
assertDoesNotThrow(() -> UUID.fromString(id), "paneId must be a UUID: " + id);
assertEquals("spawning", body.get("state").asText());
// Subscription boundary (protocol 19): tab.create carried base_url + token in its env
@@ -172,9 +172,11 @@ class SessionManagerTest {
assertEquals(1, sessions.roster().size(), "only the fresh session remains");
assertEquals(fresh.paneId(), sessions.roster().getFirst().paneId());
// The old session was the first spawn → pane w9:pW_1 (CB-519: the registry key is the
// uuid id, so teardown is asserted on the real pane coordinate).
long paneCloseCount = herdr.calls.stream()
.filter(c -> "pane.close".equals(c.method()))
.filter(c -> oldPane.equals(((Map<?, ?>) c.params()).get("pane_id")))
.filter(c -> "w9:pW_1".equals(((Map<?, ?>) c.params()).get("pane_id")))
.count();
assertEquals(1, paneCloseCount, "the old worker was torn down");
}
@@ -307,7 +309,7 @@ class SessionManagerTest {
WorkerSession updated = sessions.get(session.paneId()).orElseThrow();
assertEquals(WorkerSession.State.DONE, updated.state(), "session finishes second turn");
assertEquals(2, updated.turnCount(), "turn count tracks both deliveries");
long releaseCloseCount = paneCloseCallsFor(herdr, session.paneId());
long releaseCloseCount = paneCloseCallsFor(herdr, "w9:pW_1"); // the real pane coordinate
assertEquals(0, releaseCloseCount, "cap disabled — no forced release of the worker pane");
}
@@ -330,7 +332,7 @@ class SessionManagerTest {
assertTrue(sessions.get(session.paneId()).isEmpty(), "session released after cap reached");
assertTrue(sessions.roster().isEmpty(), "released session leaves roster");
assertEquals(1, paneCloseCallsFor(herdr, session.paneId()),
assertEquals(1, paneCloseCallsFor(herdr, "w9:pW_1"),
"forced release tears the worker pane down exactly once");
}
@@ -351,9 +353,10 @@ class SessionManagerTest {
assertTrue(sessions.roster().isEmpty(), "drain clears the roster");
assertTrue(sessions.get(ready.paneId()).isEmpty(), "ready session is released");
assertTrue(sessions.get(busy.paneId()).isEmpty(), "busy session is released after timeout");
assertEquals(1, paneCloseCallsFor(herdr, ready.paneId()),
// ready is the first spawn → pane w9:pW_1, busy the second → w9:pW_2 (FakeHerdr order).
assertEquals(1, paneCloseCallsFor(herdr, "w9:pW_1"),
"ready worker pane is torn down");
assertEquals(1, paneCloseCallsFor(herdr, busy.paneId()),
assertEquals(1, paneCloseCallsFor(herdr, "w9:pW_2"),
"busy worker pane is torn down");
}
@@ -14,6 +14,7 @@ import org.junit.jupiter.api.Test;
import java.util.List;
import java.util.Map;
import java.util.Set;
import java.util.UUID;
import java.util.function.Function;
import static org.junit.jupiter.api.Assertions.*;
@@ -258,14 +259,18 @@ class ClaudeCodeLauncherTest {
// --- PeerHandle indirection ----------------------------------------------------------------
@Test
void spawnReturnsPeerHandleWithIdEqualToPaneId() {
void spawnReturnsPeerHandleWithHostUniqueOpaqueId() {
FakeHerdr herdr = new FakeHerdr();
ClaudeCodeLauncher svc = service(herdr, List.of("ccs", "ltms-local"), null);
PeerHandle handle = svc.spawn(new SpawnRequest(null, null, null));
assertNotNull(handle, "spawn must return a non-null handle");
assertEquals("w9:pRoot_1", handle.id(), "handle.id() must equal the agent's paneId");
// CB-519: id() is a host-unique opaque UUID, decoupled from the herdr pane coordinate.
assertNotEquals("w9:pRoot_1", handle.id(),
"handle.id() must NOT be the herdr pane id");
assertDoesNotThrow(() -> UUID.fromString(handle.id()),
"handle.id() must be a UUID: " + handle.id());
}
@Test
@@ -351,6 +356,40 @@ class ClaudeCodeLauncherTest {
assertTrue(herdr.called("pane.close"), "stop via handle.id() must close the pane");
}
// --- CB-519: host-unique id, decoupled from the pane coordinate ------------------------------
@Test
void twoSpawnsOnTheSamePaneNeverCollideOnHostUniqueId() {
// Two spawns may be placed on the same herdr pane coordinate (e.g. a pane that was reused
// or re-reported after a restart); the host-unique id must not collide even then.
FakeHerdr herdr = new FakeHerdr().pinNextStarts(2, "term_shared", "w9:pShared");
ClaudeCodeLauncher svc = service(herdr, List.of("ccs", "ltms-local"), null);
PeerHandle a = svc.spawn(new SpawnRequest(null, null, null));
PeerHandle b = svc.spawn(new SpawnRequest(null, null, null));
assertNotEquals(a.id(), b.id(),
"two spawns on the same pane coordinate get distinct host-unique ids");
assertNotEquals("w9:pShared", a.id(), "id is not the pane coordinate");
assertNotEquals("w9:pShared", b.id(), "id is not the pane coordinate");
}
@Test
void stopResolvesTheHostUniqueIdToThePaneThatSpawnedIt() {
// CB-519: id() != paneId, so stop(id) must tear down the exact pane the id names — and no
// other live peer's pane.
FakeHerdr herdr = new FakeHerdr(); // deterministic panes w9:pW_1, w9:pW_2 per spawn
ClaudeCodeLauncher svc = service(herdr, List.of("ccs", "ltms-local"), null);
PeerHandle a = svc.spawn(new SpawnRequest(null, null, null));
PeerHandle b = svc.spawn(new SpawnRequest(null, null, null));
svc.stop(b.id());
assertEquals(1, paneCloseCount(herdr, "w9:pW_2"), "stop(b.id()) closes only b's pane");
assertEquals(0, paneCloseCount(herdr, "w9:pW_1"), "a's pane is untouched");
}
// --- CB-306 spawn-readiness gate -----------------------------------------------------------
private static Map<String, BridgedConfig.Worker> workerConfigMap(String profile, String mcpUrl) {
@@ -386,7 +425,8 @@ class ClaudeCodeLauncherTest {
PeerHandle handle = svc.spawn(new SpawnRequest(null, null, null));
assertNotNull(handle, "spawn returns a handle when worker becomes injectable");
assertEquals("w9:pRoot_1", handle.id(), "handle id matches the started pane");
assertNotEquals("w9:pRoot_1", handle.id(),
"handle id is a host-unique opaque id, not the started pane");
assertEquals(0, paneCloseCount(herdr, "w9:pRoot_1"),
"no pane.close when worker becomes injectable before timeout");
}
@@ -445,8 +485,9 @@ class ClaudeCodeLauncherTest {
PeerHandle handle = svc.spawn(new SpawnRequest(null, null, null));
assertNotNull(handle, "spawn still succeeds with zero timeout");
assertEquals(0, paneCloseCount(herdr, handle.id()),
assertEquals(0, paneCloseCount(herdr, "w9:pW_1"),
"no orphan pane close from the gate path");
assertDoesNotThrow(() -> UUID.fromString(handle.id()));
}
// --- CB-511: worker environment seeding -----------------------------------------------------
@@ -127,15 +127,18 @@ class CompositePeerLauncherTest {
@Test
void stopTearsDownAPaneSpawnedThroughTheComposite() {
// CB-519: handle.id() is a host-unique opaque UUID, not the herdr pane — stop(id) must
// resolve it through the owning adapter down to the actual pane coordinate it spawned.
FakeHerdr herdr = new FakeHerdr();
PeerLauncher composite = composite(herdr);
PeerHandle handle = composite.spawn(new SpawnRequest("gemini", null, null));
assertNotEquals("w9:pW_1", handle.id(), "the id is decoupled from the pane coordinate");
composite.stop(handle.id());
assertTrue(herdr.calls.stream()
.anyMatch(c -> c.method().equals("pane.close")
&& handle.id().equals(((Map<?, ?>) c.params()).get("pane_id"))),
"stop routes to the spawning adapter and closes that worker's pane");
&& "w9:pW_1".equals(((Map<?, ?>) c.params()).get("pane_id"))),
"stop routes to the spawning adapter and closes exactly that worker's pane");
}
@Test