diff --git a/fleetd/src/main/java/dev/ltms/fleet/Fleetd.java b/fleetd/src/main/java/dev/ltms/fleet/Fleetd.java index 823d9ae..c63aa07 100644 --- a/fleetd/src/main/java/dev/ltms/fleet/Fleetd.java +++ b/fleetd/src/main/java/dev/ltms/fleet/Fleetd.java @@ -498,8 +498,11 @@ public final class Fleetd { // MCP server face (CB-105): fleet_send/fleet_reply/fleet_status, mounted at /mcp. // Caller identity is resolved from the connection (peer PID → herdr pane), not arguments. + // CB-185: a caller's pane can live on either daemon (a lead's on the lead daemon, a + // member's on the member daemon) — search both, lead first. Collapses to one scan when + // memberHerdrSocket is unset (herdr == memberHerdr). ConnectionIdentity identity = new ConnectionIdentity( - new PaneLocator(memberHerdr), new LsofPeerPidLookup(), new LsofProcessCwdLookup()); + new PaneLocator(herdr, memberHerdr), new LsofPeerPidLookup(), new LsofProcessCwdLookup()); // CB-501: one resolver behind both entry paths. Worker identity still comes from the // connection and is never token-gated, so enabling token mode cannot lock the fleet out. @@ -599,7 +602,9 @@ public final class Fleetd { router.close(); })); - Javalin app = new FleetApp(herdr, workers, sessions, messages, presence, mcp.servlet(), + // CB-185: give FleetApp both daemons — /healthz must require both to answer and + // GET /sessions must merge across both, or a down/unpolled member daemon is invisible. + Javalin app = new FleetApp(herdr, memberHerdr, workers, sessions, messages, presence, mcp.servlet(), callers, metrics, deliverable).build(); app.start(cfg.bind().host(), cfg.bind().port()); log.info("fleetd listening on {}:{}, herdr socket {}", diff --git a/fleetd/src/main/java/dev/ltms/fleet/herdr/PaneLocator.java b/fleetd/src/main/java/dev/ltms/fleet/herdr/PaneLocator.java index 9e2e355..f2a0af4 100644 --- a/fleetd/src/main/java/dev/ltms/fleet/herdr/PaneLocator.java +++ b/fleetd/src/main/java/dev/ltms/fleet/herdr/PaneLocator.java @@ -2,6 +2,7 @@ package dev.ltms.fleet.herdr; import com.fasterxml.jackson.databind.JsonNode; +import java.util.List; import java.util.Map; /** @@ -13,33 +14,61 @@ import java.util.Map; *

herdr owns the PID→pane truth: {@code pane.process_info} reports each pane's {@code shell_pid} * and foreground process PIDs. This scans agent panes; a spawn-time {@code pid→terminal} cache is * the obvious optimization once wired into {@code ClaudeCodeLauncher}. + * + *

CB-185 split the fleet across two herdr daemons — lead operations on one, members on the + * other ({@code memberHerdrSocket}). A caller's pane can live on either daemon (a lead's + * MCP connection resolves against the lead daemon; a member's against the member daemon), so this + * must be able to search more than one client. {@link #PaneLocator(HerdrClient, HerdrClient)} + * searches the lead client first, then the member client, and collapses to a single scan when the + * two are the same object (the historical single-daemon deployment). */ public final class PaneLocator { - private final HerdrClient herdr; + private final List herdrs; + /** Search only this client — the single-daemon deployment. */ public PaneLocator(HerdrClient herdr) { - this.herdr = herdr; + this.herdrs = List.of(herdr); + } + + /** + * Search {@code lead} first, then {@code member} — the two-daemon deployment (CB-185). When + * the caller passes the same client for both (no {@code memberHerdrSocket} configured), this + * collapses to one client and one scan, exactly {@link #PaneLocator(HerdrClient)}'s behaviour. + */ + public PaneLocator(HerdrClient lead, HerdrClient member) { + this.herdrs = lead == member ? List.of(lead) : List.of(lead, member); } /** * The {@code terminal_id} of the agent pane whose process tree contains {@code pid}, or - * {@code null} if no agent pane owns it (e.g. the caller is the primary, or off-host). + * {@code null} if no agent pane on any searched daemon owns it (e.g. the caller is the + * primary, or off-host). */ public String terminalForPid(long pid) { if (pid <= 0) { return null; } + for (HerdrClient herdr : herdrs) { + String terminal = terminalForPid(herdr, pid); + if (terminal != null) { + return terminal; + } + } + return null; + } + + private static String terminalForPid(HerdrClient herdr, long pid) { for (JsonNode pane : herdr.call("pane.list", Map.of()).path("panes")) { String paneId = pane.path("pane_id").asText(null); - if (paneId != null && paneOwnsPid(paneId, pid)) { + if (paneId != null && paneOwnsPid(herdr, paneId, pid)) { return pane.path("terminal_id").asText(null); } } return null; } - private boolean paneOwnsPid(String paneId, long pid) { + private static boolean paneOwnsPid(HerdrClient herdr, String paneId, long pid) { JsonNode info; try { info = herdr.call("pane.process_info", Map.of("pane_id", paneId)).path("process_info"); diff --git a/fleetd/src/main/java/dev/ltms/fleet/inject/StatusPoller.java b/fleetd/src/main/java/dev/ltms/fleet/inject/StatusPoller.java index 5dda76c..a2e7966 100644 --- a/fleetd/src/main/java/dev/ltms/fleet/inject/StatusPoller.java +++ b/fleetd/src/main/java/dev/ltms/fleet/inject/StatusPoller.java @@ -47,6 +47,10 @@ public final class StatusPoller { this.agents = null; this.router = router; this.injector = injector; + // CB-185: this refiner's own AgentControl (member) is only a default for the legacy 2-arg + // refine() overload — the loop below always calls the 3-arg refine(target, raw, control) + // with the per-target control from router.agentsFor(target), so a lead target is refined + // against the LEAD daemon even though this field points at the member one. this.refiner = new StatusRefiner(router.memberAgents()); this.intervalMillis = intervalMillis; } @@ -67,8 +71,11 @@ public final class StatusPoller { try { // herdr's agent_status can misreport a settled worker as `unknown`; refine it // against the pane content before it drives delivery/completion (CB-115). + // CB-185: refine THROUGH the same control the raw status came from — a router + // splits lead/member targets across two herdr daemons, and reading a lead's pane + // through the (fixed) member refiner never finds it, wedging that lead at UNKNOWN. AgentControl control = router != null ? router.agentsFor(target) : agents; - AgentStatus status = refiner.refine(target, control.status(target)); + AgentStatus status = refiner.refine(target, control.status(target), control); injector.onStatus(target, status); } catch (HerdrException e) { // The worker's agent is gone — stop trying and unblock its waiters. diff --git a/fleetd/src/main/java/dev/ltms/fleet/inject/StatusRefiner.java b/fleetd/src/main/java/dev/ltms/fleet/inject/StatusRefiner.java index bc55d02..309b9b5 100644 --- a/fleetd/src/main/java/dev/ltms/fleet/inject/StatusRefiner.java +++ b/fleetd/src/main/java/dev/ltms/fleet/inject/StatusRefiner.java @@ -41,15 +41,32 @@ public final class StatusRefiner { } /** - * Return a trustworthy status for {@code target}. Any non-{@code UNKNOWN} {@code raw} is returned - * unchanged; an {@code UNKNOWN} triggers a pane read and content classification. A read failure - * leaves it {@code UNKNOWN} (the safe default: no delivery, and the stall path still applies). + * Return a trustworthy status for {@code target}, reading its pane through this refiner's own + * {@link AgentControl}. Equivalent to {@link #refine(String, AgentStatus, AgentControl)} with + * that control — kept for callers that only ever talk to one herdr daemon. */ public AgentStatus refine(String target, AgentStatus raw) { + return refine(target, raw, agents); + } + + /** + * Return a trustworthy status for {@code target}. Any non-{@code UNKNOWN} {@code raw} is returned + * unchanged; an {@code UNKNOWN} triggers a pane read (through {@code control}) and content + * classification. A read failure leaves it {@code UNKNOWN} (the safe default: no delivery, and + * the stall path still applies). + * + *

CB-185: {@code control} must be the {@link AgentControl} for the same daemon the + * raw status was sampled from — a router splits lead and member targets across two herdr + * daemons, and reading a lead's pane through the member client (or vice versa) fails to find + * the pane and leaves the target wedged at {@code UNKNOWN} forever. Callers that route per + * target (e.g. {@code StatusPoller}) must pass that target's control explicitly rather than + * relying on the control fixed at construction. + */ + public AgentStatus refine(String target, AgentStatus raw, AgentControl control) { if (raw != AgentStatus.UNKNOWN) return raw; String pane; try { - pane = agents.read(target, PROBE_SOURCE); + pane = control.read(target, PROBE_SOURCE); } catch (RuntimeException e) { log.debug("status refine read for {} failed; leaving UNKNOWN: {}", target, e.getMessage()); return AgentStatus.UNKNOWN; diff --git a/fleetd/src/main/java/dev/ltms/fleet/rest/FleetApp.java b/fleetd/src/main/java/dev/ltms/fleet/rest/FleetApp.java index e634390..16a0cf0 100644 --- a/fleetd/src/main/java/dev/ltms/fleet/rest/FleetApp.java +++ b/fleetd/src/main/java/dev/ltms/fleet/rest/FleetApp.java @@ -55,7 +55,8 @@ public final class FleetApp { /** Context attribute under which the resolved caller is stashed by the auth filter. */ private static final String CALLER = "fleetd.caller"; - private final HerdrClient herdr; + private final HerdrClient herdr; // lead daemon + private final HerdrClient memberHerdr; // CB-185: member daemon (same object when unconfigured) private final PeerLauncher workers; private final SessionManager sessions; // CB-301: authoritative session registry private final MessageService messages; @@ -95,7 +96,23 @@ public final class FleetApp { MessageService messages, MemberPresence presence, HttpServlet mcpServlet, CallerResolver auth, Metrics metrics, Predicate deliverable) { + this(herdr, herdr, workers, sessions, messages, presence, mcpServlet, auth, metrics, deliverable); + } + + /** + * @param herdr the lead daemon's client + * @param memberHerdr the member daemon's client (CB-185); pass the same instance as + * {@code herdr} for a single-daemon deployment — {@code healthz}/{@code + * sessions} then make exactly one herdr call each, unchanged from before + * the two-daemon router existed + * @param deliverable the injector's readiness gate, shared so status reports its real result + */ + public FleetApp(HerdrClient herdr, HerdrClient memberHerdr, PeerLauncher workers, SessionManager sessions, + MessageService messages, MemberPresence presence, + HttpServlet mcpServlet, CallerResolver auth, Metrics metrics, + Predicate deliverable) { this.herdr = herdr; + this.memberHerdr = memberHerdr != null ? memberHerdr : herdr; this.workers = workers; this.sessions = sessions; this.messages = messages; @@ -190,30 +207,64 @@ public final class FleetApp { ctx.status(200).contentType("text/plain; version=0.0.4; charset=utf-8").result(metrics.render()); } - /** Liveness + herdr reachability. 200 when herdr answers ping, 503 otherwise. */ + /** + * Liveness + herdr reachability. 200 only when BOTH daemons answer ping — 503 otherwise + * (CB-185). With no {@code memberHerdrSocket} configured {@code memberHerdr == herdr}, so this + * makes exactly the one {@code ping} call it always did and reports the same body; with a + * second daemon configured, a member daemon that is down must not be masked by a healthy lead + * daemon — every spawn goes through the member daemon and would otherwise fail silently behind + * a green {@code /healthz}. + */ private void healthz(Context ctx) { + JsonNode pong; try { - JsonNode pong = herdr.call("ping"); - ctx.status(200).json(Map.of( - "status", "ok", - "herdr", Map.of( - "version", pong.path("version").asText(""), - "protocol", pong.path("protocol").asInt()))); + pong = herdr.call("ping"); } catch (HerdrException e) { ctx.status(503).json(Map.of( "status", "degraded", "herdr", "unreachable", "detail", e.getMessage())); + return; } + if (memberHerdr != herdr) { + try { + memberHerdr.call("ping"); + } catch (HerdrException e) { + ctx.status(503).json(Map.of( + "status", "degraded", + "herdr", "member unreachable", + "detail", e.getMessage())); + return; + } + } + ctx.status(200).json(Map.of( + "status", "ok", + "herdr", Map.of( + "version", pong.path("version").asText(""), + "protocol", pong.path("protocol").asInt()))); } - /** Sessions view derived from herdr {@code workspace.list} (one workspace → one row). */ + /** + * Sessions view derived from herdr {@code workspace.list} (one workspace → one row), merged + * across both daemons (CB-185). With no {@code memberHerdrSocket} configured {@code + * memberHerdr == herdr}, so this calls {@code workspace.list} exactly once, same as before the + * router existed; with a second daemon configured, calling it twice would silently drop every + * member workspace (they live on the member daemon only). + */ private void sessions(Context ctx) { if (!allow(ctx, Authz.Action.READ, null)) { return; } - JsonNode result = herdr.call("workspace.list"); List> out = new ArrayList<>(); + collectSessions(herdr, out); + if (memberHerdr != herdr) { + collectSessions(memberHerdr, out); + } + ctx.status(200).json(Map.of("sessions", out)); + } + + private static void collectSessions(HerdrClient client, List> out) { + JsonNode result = client.call("workspace.list"); for (JsonNode w : result.path("workspaces")) { out.add(Map.of( "id", w.path("workspace_id").asText(""), @@ -222,7 +273,6 @@ public final class FleetApp { "paneCount", w.path("pane_count").asInt(), "agentStatus", w.path("agent_status").asText("unknown"))); } - ctx.status(200).json(Map.of("sessions", out)); } /** Discovery: every agent herdr tracks, keyed by its Claude session UUID. */ diff --git a/fleetd/src/test/java/dev/ltms/fleet/FleetdConnectionIdentityConstructionTest.java b/fleetd/src/test/java/dev/ltms/fleet/FleetdConnectionIdentityConstructionTest.java new file mode 100644 index 0000000..7274264 --- /dev/null +++ b/fleetd/src/test/java/dev/ltms/fleet/FleetdConnectionIdentityConstructionTest.java @@ -0,0 +1,31 @@ +package dev.ltms.fleet; + +import java.nio.file.Files; +import java.nio.file.Path; +import org.junit.jupiter.api.Test; + +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertTrue; + +/** + * CB-185: {@code ConnectionIdentity} must resolve a caller's pane on EITHER herdr daemon (a + * lead's MCP connection resolves against the lead daemon; a member's against the member daemon). + * Pinning {@code PaneLocator} to {@code memberHerdr} alone — the bug this guards against — leaves + * every lead's own connection unresolvable ({@code callerTerminal == null}) the moment + * {@code memberHerdrSocket} names a second daemon, which breaks {@code fleet_reply}/{@code + * fleet_ask} and {@code fleet_whoami} for a lead. A unit test on {@link + * dev.ltms.fleet.herdr.PaneLocator} alone (see {@code PaneLocatorTest}) proves the class CAN + * search two clients, but not that {@code Fleetd.main} actually wires it that way — hence this + * source-level assertion, the same technique {@code FleetdHerdrControlConstructionTest} uses. + */ +class FleetdConnectionIdentityConstructionTest { + @Test + void connectionIdentitySearchesBothDaemonsNotJustTheMemberOne() throws Exception { + String source = Files.readString(Path.of("src/main/java/dev/ltms/fleet/Fleetd.java")); + assertFalse(source.contains("new PaneLocator(memberHerdr)"), + "PaneLocator must not be pinned to the member daemon alone — a lead's own " + + "connection resolves against the LEAD daemon and would never be found"); + assertTrue(source.contains("new PaneLocator(herdr, memberHerdr)"), + "PaneLocator must search the lead daemon first, then the member daemon"); + } +} diff --git a/fleetd/src/test/java/dev/ltms/fleet/FleetdFleetAppConstructionTest.java b/fleetd/src/test/java/dev/ltms/fleet/FleetdFleetAppConstructionTest.java new file mode 100644 index 0000000..a7e8482 --- /dev/null +++ b/fleetd/src/test/java/dev/ltms/fleet/FleetdFleetAppConstructionTest.java @@ -0,0 +1,29 @@ +package dev.ltms.fleet; + +import java.nio.file.Files; +import java.nio.file.Path; +import org.junit.jupiter.api.Test; + +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertTrue; + +/** + * CB-185: {@code FleetApp} must be constructed with BOTH herdr clients (the lead's and the + * member's), never the raw lead-only {@code herdr}. Passing only {@code herdr} — the bug this + * guards against — makes {@code GET /healthz} green while the member daemon is down (so every + * spawn fails invisibly) and silently drops every member workspace from {@code GET /sessions}. + * A behavioural test on {@code FleetApp} alone (see {@code FleetAppTwoDaemonTest}) proves the + * class merges/gates correctly when given two clients, but not that {@code Fleetd.main} actually + * passes it two — hence this source-level assertion, mirroring + * {@code FleetdHerdrControlConstructionTest}. + */ +class FleetdFleetAppConstructionTest { + @Test + void fleetAppIsConstructedWithBothHerdrDaemons() throws Exception { + String source = Files.readString(Path.of("src/main/java/dev/ltms/fleet/Fleetd.java")); + assertFalse(source.contains("new FleetApp(herdr, workers,"), + "FleetApp must not be constructed with the lead-only herdr client"); + assertTrue(source.contains("new FleetApp(herdr, memberHerdr, workers,"), + "FleetApp must be constructed with both the lead and the member herdr client"); + } +} diff --git a/fleetd/src/test/java/dev/ltms/fleet/herdr/FakeHerdr.java b/fleetd/src/test/java/dev/ltms/fleet/herdr/FakeHerdr.java index cf86610..591b39c 100644 --- a/fleetd/src/test/java/dev/ltms/fleet/herdr/FakeHerdr.java +++ b/fleetd/src/test/java/dev/ltms/fleet/herdr/FakeHerdr.java @@ -40,6 +40,7 @@ public final class FakeHerdr implements HerdrClient { private int workerTabPaneCount = 1; private String paneCloseErrorCode = null; private String agentSendErrorCode = null; + private boolean noPanes = false; 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 @@ -75,6 +76,15 @@ public final class FakeHerdr implements HerdrClient { return this; } + /** + * Make {@code pane.list} report no panes at all — models a second herdr daemon (CB-185) that + * simply does not host the pane a {@link PaneLocator} is searching for. + */ + public FakeHerdr withNoPanes() { + this.noPanes = true; + return this; + } + /** Set the {@code agent_status} that {@code agent.get} reports (drives the injector). */ public FakeHerdr agentStatus(String status) { this.agentStatus = status; @@ -268,7 +278,9 @@ public final class FakeHerdr implements HerdrClient { case "pane.get" -> mapper.readTree(""" {"type":"pane_info","pane":{"pane_id":"w9:pW","workspace_id":"w9", "tab_id":"w9:t2","agent_status":"idle"}}"""); - case "pane.list" -> mapper.readTree(""" + case "pane.list" -> noPanes + ? mapper.readTree("{\"type\":\"pane_list\",\"panes\":[]}") + : mapper.readTree(""" {"type":"pane_list","panes":[ {"pane_id":"w2:p7","terminal_id":"term_a","workspace_id":"w2","tab_id":"w2:t7","agent":"claude"}, {"pane_id":"w2:p9","terminal_id":"term_shell","workspace_id":"w2","tab_id":"w2:t8"}]}"""); diff --git a/fleetd/src/test/java/dev/ltms/fleet/herdr/PaneLocatorTest.java b/fleetd/src/test/java/dev/ltms/fleet/herdr/PaneLocatorTest.java index 7b01877..929ddc1 100644 --- a/fleetd/src/test/java/dev/ltms/fleet/herdr/PaneLocatorTest.java +++ b/fleetd/src/test/java/dev/ltms/fleet/herdr/PaneLocatorTest.java @@ -24,4 +24,43 @@ class PaneLocatorTest { assertNull(loc.terminalForPid(0)); assertNull(loc.terminalForPid(-1)); } + + // --- two-daemon fallback (CB-185) ----------------------------------------- + + @Test + void fallsBackToTheMemberClientWhenTheLeadHasNoMatch() { + // The caller's pane lives on the member daemon only (e.g. the caller is a spawned + // member) — the lead client reports no panes at all, so the locator must fall back. + HerdrClient lead = new FakeHerdr().withNoPanes(); + HerdrClient member = new FakeHerdr(); + PaneLocator two = new PaneLocator(lead, member); + assertEquals("term_a", two.terminalForPid(FakeHerdr.WORKER_PID)); + } + + @Test + void searchesTheLeadClientBeforeTheMemberClient() { + // The caller's pane lives on the LEAD daemon (e.g. the caller is a peer lead) — with two + // daemons, resolving it must not depend on the member client having a matching pane. + HerdrClient lead = new FakeHerdr(); + HerdrClient member = new FakeHerdr().withNoPanes(); + PaneLocator two = new PaneLocator(lead, member); + assertEquals("term_a", two.terminalForPid(FakeHerdr.WORKER_PID)); + } + + @Test + void nullWhenNeitherClientHasTheMatch() { + PaneLocator two = new PaneLocator(new FakeHerdr().withNoPanes(), new FakeHerdr().withNoPanes()); + assertNull(two.terminalForPid(FakeHerdr.WORKER_PID)); + } + + @Test + void collapsesToOneScanWhenLeadAndMemberAreTheSameClient() { + // The single-daemon deployment (no memberHerdrSocket configured): the two-arg constructor + // must behave exactly like the one-arg constructor, including making only one herdr call. + FakeHerdr shared = new FakeHerdr(); + PaneLocator two = new PaneLocator(shared, shared); + assertEquals("term_a", two.terminalForPid(FakeHerdr.WORKER_PID)); + long paneListCalls = shared.calls.stream().filter(c -> c.method().equals("pane.list")).count(); + assertEquals(1, paneListCalls, "same-object lead/member must scan exactly once, not twice"); + } } diff --git a/fleetd/src/test/java/dev/ltms/fleet/inject/StatusPollerRoutingTest.java b/fleetd/src/test/java/dev/ltms/fleet/inject/StatusPollerRoutingTest.java new file mode 100644 index 0000000..d748534 --- /dev/null +++ b/fleetd/src/test/java/dev/ltms/fleet/inject/StatusPollerRoutingTest.java @@ -0,0 +1,75 @@ +package dev.ltms.fleet.inject; + +import dev.ltms.fleet.herdr.FakeHerdr; +import dev.ltms.fleet.herdr.HerdrRouter; +import dev.ltms.fleet.msg.TestTurnTokens; +import org.junit.jupiter.api.Test; + +import java.util.concurrent.CompletableFuture; +import java.util.concurrent.TimeUnit; +import java.util.concurrent.TimeoutException; + +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; + +/** + * CB-185: with a router split across two herdr daemons, {@link StatusPoller} must refine a raw + * {@code UNKNOWN} status by reading the pane content from the SAME daemon the status was sampled + * from — the lead daemon for a lead target, the member daemon for a member target. Reading the + * wrong daemon never finds the pane, classification stays {@code UNKNOWN} forever, and the + * status-gated {@link Injector} wedges: a queued message is never delivered. + * + *

This exercises the real production classes ({@code StatusPoller(HerdrRouter, ...)}, + * {@code Injector(HerdrRouter, ...)}) wired together, not a hand-built object graph — the earlier + * three CB-185 bugs all passed exactly that kind of test while the real wiring stayed broken. + */ +class StatusPollerRoutingTest { + + private static final String LEAD_TARGET = "term_a"; + + @Test + void refinesALeadTargetFromTheLeadDaemonAndDelivers() throws Exception { + // The lead daemon's pane is at a settled idle prompt; the member daemon's pane content is + // unclassifiable garbage. A correct refiner reads the LEAD daemon and delivers. + FakeHerdr leadHerdr = new FakeHerdr().agentStatus("unknown").readText("⏺ answer\n❯ "); + FakeHerdr memberHerdr = new FakeHerdr().agentStatus("unknown") + .readText("garbled ansi noise with no prompt"); + HerdrRouter router = new HerdrRouter(leadHerdr, memberHerdr, LEAD_TARGET::equals); + Injector injector = new Injector(router, TurnListener.NOOP, _ -> true, _ -> { + }); + StatusPoller poller = new StatusPoller(router, injector, 10); + poller.start(); + try { + CompletableFuture delivered = + injector.enqueue(LEAD_TARGET, "via-poller", TestTurnTokens.inert(LEAD_TARGET)); + // Must resolve quickly: refining against the WRONG daemon (member) never classifies + // out of UNKNOWN, so this would time out under the bug. + delivered.get(2, TimeUnit.SECONDS); + } finally { + poller.stop(); + } + assertTrue(leadHerdr.called("agent.read"), "refine must probe the LEAD daemon's pane content"); + } + + @Test + void aLeadTargetNeverDeliversWhenOnlyTheMemberDaemonIsClassifiable() throws Exception { + // Inverted control: the member daemon's content WOULD classify to idle, but this is a lead + // target — a correct implementation must not use it, so delivery must NOT happen. + FakeHerdr leadHerdr = new FakeHerdr().agentStatus("unknown") + .readText("garbled ansi noise with no prompt"); + FakeHerdr memberHerdr = new FakeHerdr().agentStatus("unknown").readText("⏺ answer\n❯ "); + HerdrRouter router = new HerdrRouter(leadHerdr, memberHerdr, LEAD_TARGET::equals); + Injector injector = new Injector(router, TurnListener.NOOP, _ -> true, _ -> { + }); + StatusPoller poller = new StatusPoller(router, injector, 10); + poller.start(); + try { + CompletableFuture delivered = + injector.enqueue(LEAD_TARGET, "via-poller", TestTurnTokens.inert(LEAD_TARGET)); + assertThrows(TimeoutException.class, () -> delivered.get(500, TimeUnit.MILLISECONDS), + "a lead target must never be refined from the member daemon's pane content"); + } finally { + poller.stop(); + } + } +} diff --git a/fleetd/src/test/java/dev/ltms/fleet/inject/StatusRefinerTest.java b/fleetd/src/test/java/dev/ltms/fleet/inject/StatusRefinerTest.java index 7034bea..642eadd 100644 --- a/fleetd/src/test/java/dev/ltms/fleet/inject/StatusRefinerTest.java +++ b/fleetd/src/test/java/dev/ltms/fleet/inject/StatusRefinerTest.java @@ -85,4 +85,34 @@ class StatusRefinerTest { assertEquals(AgentStatus.UNKNOWN, refiner.refine("term_a", AgentStatus.UNKNOWN)); } + + // --- refine(target, raw, control) — CB-185 per-call routing --------------- + + @Test + void threeArgRefineReadsThroughTheGivenControlNotTheConstructedOne() { + // The refiner is CONSTRUCTED with one control (standing in for "the member daemon"), but + // a call names a DIFFERENT control (standing in for "the lead daemon") — the read must go + // to the one passed to the call, since that is the daemon the raw status came from. + FakeHerdr constructedWith = new FakeHerdr().readText("nothing recognizable here"); + FakeHerdr passedToCall = new FakeHerdr().readText("⏺ answer\n❯ "); + StatusRefiner refiner = new StatusRefiner(new AgentControl(constructedWith)); + + AgentStatus result = refiner.refine("term_a", AgentStatus.UNKNOWN, new AgentControl(passedToCall)); + + assertEquals(AgentStatus.IDLE, result, "must classify from the PASSED control's pane content"); + assertTrue(passedToCall.called("agent.read")); + assertFalse(constructedWith.called("agent.read"), + "the control fixed at construction must not be read when a call-site control is given"); + } + + @Test + void twoArgRefineStillReadsTheConstructedControl() { + // The legacy 2-arg overload (single-daemon callers) must keep using the constructed + // control — this is refine(target, raw, control) called with the field as `control`. + FakeHerdr herdr = new FakeHerdr().readText("⏺ answer\n❯ "); + StatusRefiner refiner = new StatusRefiner(new AgentControl(herdr)); + + assertEquals(AgentStatus.IDLE, refiner.refine("term_a", AgentStatus.UNKNOWN)); + assertTrue(herdr.called("agent.read")); + } } diff --git a/fleetd/src/test/java/dev/ltms/fleet/rest/FleetAppTwoDaemonTest.java b/fleetd/src/test/java/dev/ltms/fleet/rest/FleetAppTwoDaemonTest.java new file mode 100644 index 0000000..efbba3d --- /dev/null +++ b/fleetd/src/test/java/dev/ltms/fleet/rest/FleetAppTwoDaemonTest.java @@ -0,0 +1,99 @@ +package dev.ltms.fleet.rest; + +import dev.ltms.fleet.herdr.FakeHerdr; +import dev.ltms.fleet.herdr.HerdrClient; +import io.javalin.Javalin; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.Test; + +import java.net.URI; +import java.net.http.HttpClient; +import java.net.http.HttpRequest; +import java.net.http.HttpResponse; + +import static org.junit.jupiter.api.Assertions.*; + +/** + * CB-185: with a router split across two herdr daemons (lead + {@code memberHerdrSocket}), + * {@link FleetApp#healthz} must require BOTH daemons to answer and {@link FleetApp#sessions} + * (which the {@code GET /sessions} route calls) must merge workspaces from both — the bug this + * guards against had {@code FleetApp} constructed with the raw lead-only client, so a down member + * daemon was invisible behind a green {@code /healthz} (every spawn then fails) and every member + * workspace was silently dropped from {@code GET /sessions}. + * + *

Builds the real {@link FleetApp} directly (not a hand-rolled stand-in) against only the two + * herdr clients — the other collaborators are unused by the two routes under test here. + */ +class FleetAppTwoDaemonTest { + + private final HttpClient http = HttpClient.newHttpClient(); + private Javalin app; + + @AfterEach + void stop() { + if (app != null) app.stop(); + } + + private int start(HerdrClient lead, HerdrClient member) { + app = new FleetApp(lead, member, null, null, null, null, null, null, null, ignored -> false) + .build().start("127.0.0.1", 0); + return app.port(); + } + + private HttpResponse get(int port, String path) throws Exception { + HttpRequest req = HttpRequest.newBuilder(URI.create("http://127.0.0.1:" + port + path)).GET().build(); + return http.send(req, HttpResponse.BodyHandlers.ofString()); + } + + @Test + void healthzIsGreenWhenBothDaemonsAnswer() throws Exception { + int port = start(new FakeHerdr(), new FakeHerdr()); + assertEquals(200, get(port, "/healthz").statusCode()); + } + + @Test + void healthzIsDegradedWhenOnlyTheMemberDaemonIsDown() throws Exception { + int port = start(new FakeHerdr(), new FakeHerdr().healthy(false)); + HttpResponse res = get(port, "/healthz"); + assertEquals(503, res.statusCode(), + "a down MEMBER daemon must not be masked by a healthy lead — every spawn goes " + + "through the member daemon"); + } + + @Test + void healthzIsDegradedWhenOnlyTheLeadDaemonIsDown() throws Exception { + int port = start(new FakeHerdr().healthy(false), new FakeHerdr()); + assertEquals(503, get(port, "/healthz").statusCode()); + } + + @Test + void healthzMakesExactlyOneCallWhenLeadAndMemberAreTheSameClient() throws Exception { + // Single-daemon deployment (no memberHerdrSocket) — must be byte-for-byte the old + // behaviour: one ping call, 200 on success. + FakeHerdr shared = new FakeHerdr(); + int port = start(shared, shared); + assertEquals(200, get(port, "/healthz").statusCode()); + long pings = shared.calls.stream().filter(c -> c.method().equals("ping")).count(); + assertEquals(1, pings, "single-daemon deployment must make exactly one ping call"); + } + + @Test + void sessionsMergesWorkspacesFromBothDaemons() throws Exception { + FakeHerdr lead = new FakeHerdr(); + FakeHerdr member = new FakeHerdr().withWorkspace("w9", "member-only-workspace"); + int port = start(lead, member); + HttpResponse res = get(port, "/sessions"); + assertEquals(200, res.statusCode(), res.body()); + assertTrue(res.body().contains("member-only-workspace"), + "GET /sessions must not silently drop the member daemon's workspaces"); + } + + @Test + void sessionsMakesExactlyOneWorkspaceListCallWhenLeadAndMemberAreTheSameClient() throws Exception { + FakeHerdr shared = new FakeHerdr(); + int port = start(shared, shared); + assertEquals(200, get(port, "/sessions").statusCode()); + long calls = shared.calls.stream().filter(c -> c.method().equals("workspace.list")).count(); + assertEquals(1, calls, "single-daemon deployment must call workspace.list exactly once"); + } +}