From 24f404f989ad1dcc7e54f87e7d0a72a4f4a25635 Mon Sep 17 00:00:00 2001 From: Dai Ha Date: Sat, 29 Aug 2026 06:20:32 +0700 Subject: [PATCH] CB-185: fix three connection-identity/status/health gaps a second herdr daemon exposes memberHerdrSocket splits lead operations from member operations onto two herdr daemons. Three seams still assumed one shared daemon and broke silently when the two clients differ (all three collapse to today's behaviour when they are the same object): 1. ConnectionIdentity's PaneLocator was pinned to the member daemon only, so a lead's own MCP connection (which lives on the LEAD daemon) resolved to terminal == null, breaking fleet_reply/fleet_ask/fleet_whoami for a lead. PaneLocator now searches the lead client first, then the member client. 2. StatusPoller's StatusRefiner was pinned to the member daemon, so refining an UNKNOWN status for a lead target read the wrong daemon's pane content and never left UNKNOWN, wedging status-gated delivery to that lead forever. StatusRefiner gained a refine(target, raw, control) overload and the poller now refines through the same AgentControl the raw status was sampled from. 3. FleetApp was constructed with the raw lead-only herdr client, so /healthz stayed green while the member daemon was down (every spawn then fails invisibly) and GET /sessions silently dropped every member workspace. FleetApp now takes both clients: healthz requires both to answer, sessions merges workspaces from both. Each fix has a test proven to fail without it (verified by reverting the production change and re-running): FleetdConnectionIdentityConstructionTest / FleetdFleetAppConstructionTest assert the actual Fleetd.java wiring (the same technique as FleetdHerdrControlConstructionTest); StatusPollerRoutingTest and the new PaneLocatorTest/FleetAppTwoDaemonTest cases exercise the real production classes end to end rather than a hand-built object graph. --- .../src/main/java/dev/ltms/fleet/Fleetd.java | 9 +- .../dev/ltms/fleet/herdr/PaneLocator.java | 39 +++++++- .../dev/ltms/fleet/inject/StatusPoller.java | 9 +- .../dev/ltms/fleet/inject/StatusRefiner.java | 25 ++++- .../java/dev/ltms/fleet/rest/FleetApp.java | 72 +++++++++++--- ...etdConnectionIdentityConstructionTest.java | 31 ++++++ .../fleet/FleetdFleetAppConstructionTest.java | 29 ++++++ .../java/dev/ltms/fleet/herdr/FakeHerdr.java | 14 ++- .../dev/ltms/fleet/herdr/PaneLocatorTest.java | 39 ++++++++ .../fleet/inject/StatusPollerRoutingTest.java | 75 ++++++++++++++ .../ltms/fleet/inject/StatusRefinerTest.java | 30 ++++++ .../fleet/rest/FleetAppTwoDaemonTest.java | 99 +++++++++++++++++++ 12 files changed, 447 insertions(+), 24 deletions(-) create mode 100644 fleetd/src/test/java/dev/ltms/fleet/FleetdConnectionIdentityConstructionTest.java create mode 100644 fleetd/src/test/java/dev/ltms/fleet/FleetdFleetAppConstructionTest.java create mode 100644 fleetd/src/test/java/dev/ltms/fleet/inject/StatusPollerRoutingTest.java create mode 100644 fleetd/src/test/java/dev/ltms/fleet/rest/FleetAppTwoDaemonTest.java 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"); + } +} -- 2.52.0