diff --git a/fleetd/src/main/java/dev/ltms/fleet/mcp/FleetMcp.java b/fleetd/src/main/java/dev/ltms/fleet/mcp/FleetMcp.java index e351f8a..3143175 100644 --- a/fleetd/src/main/java/dev/ltms/fleet/mcp/FleetMcp.java +++ b/fleetd/src/main/java/dev/ltms/fleet/mcp/FleetMcp.java @@ -426,7 +426,8 @@ public final class FleetMcp { return listFleet(workers, sessions, messages, capacity, healthCoverage, quarantine, outage, leadSeats, callers == null ? Map.of() : callers.leads(), callerTerminal(exchange), - new CoordinationSource(leadChannel, peers)); + new CoordinationSource(leadChannel, peers), + principal(exchange).isPrimary()); }; BiFunction stopHandler = (exchange, req) -> { @@ -1344,12 +1345,44 @@ public final class FleetMcp { LeadSeatSource.none(), leads, selfTerm, coordination); } - /** As above, plus fleetd #176 lead-seat facts (see {@link LeadSeatSource}). */ + /** + * As above, plus fleetd #176 lead-seat facts (see {@link LeadSeatSource}). + * + *

Assumes the caller is the primary — every wrapper overload above delegates here without + * carrying a caller identity, which is exactly right for them: they exist for call sites (and + * unit tests) that have no {@link Principal} to hand over, and this preserves their pre-#439 + * behavior unchanged. The one call site that has a real caller ({@code fleet_list}'s MCP + * handler) uses {@link #listFleet(PeerLauncher, SessionManager, MessageService, CapacitySource, + * HealthCoverageSource, QuarantineSource, OutageSource, LeadSeatSource, Map, String, + * CoordinationSource, boolean)} instead, so it can pass the true answer. + */ static McpSchema.CallToolResult listFleet(PeerLauncher workers, SessionManager sessions, MessageService messages, CapacitySource capacity, HealthCoverageSource healthCoverage, QuarantineSource quarantine, OutageSource outage, LeadSeatSource leadSeats, Map leads, String selfTerm, CoordinationSource coordination) { + return listFleet(workers, sessions, messages, capacity, healthCoverage, quarantine, outage, + leadSeats, leads, selfTerm, coordination, true); + } + + /** + * As above, gated by the caller's role (fleetd #439). The {@code coordinator} row is + * lead-to-lead coordination state — coordination between orchestrators, not roster + * observation — so it is assembled and included only when {@code callerIsPrimary} is + * {@code true}. A worker or an architect gets a result with the {@code coordinator} key + * absent, never an empty or redacted one, and never pays the cost of + * {@link #coordinatorView} probing peer mailboxes for a row it will not receive. + * + * @param callerIsPrimary whether the {@code fleet_list} caller is the primary; only the MCP + * handler computes this from the real connection (see + * {@code Principal#isPrimary()}) — every other overload passes + * {@code true} + */ + static McpSchema.CallToolResult listFleet(PeerLauncher workers, SessionManager sessions, MessageService messages, + CapacitySource capacity, HealthCoverageSource healthCoverage, + QuarantineSource quarantine, OutageSource outage, + LeadSeatSource leadSeats, Map leads, String selfTerm, + CoordinationSource coordination, boolean callerIsPrimary) { try { Map live = workers.list().stream() .map(Agent.class::cast) @@ -1371,9 +1404,14 @@ public final class FleetMcp { Map result = new LinkedHashMap<>(); result.put("leads", leadRows); result.put("members", out); result.put("healthCoverage", healthCoverage.value().get()); - Map coordinatorRow = coordinatorView(coordination); - if (coordinatorRow != null) { - result.put("coordinator", coordinatorRow); + // fleetd #439: coordinator/coordinatorView is lead-to-lead coordination state and must + // never reach a worker or an architect -- gate BEFORE assembling it, not after, so the + // key is absent rather than present-and-empty. + if (callerIsPrimary) { + Map coordinatorRow = coordinatorView(coordination); + if (coordinatorRow != null) { + result.put("coordinator", coordinatorRow); + } } if (capacity.available()) result.put("capacity", profiles.stream() .map(profile -> capacityView(profile, capacity.liveCount(), capacity.maxLoad(), roster, messages, diff --git a/fleetd/src/test/java/dev/ltms/fleet/mcp/FleetMcpTest.java b/fleetd/src/test/java/dev/ltms/fleet/mcp/FleetMcpTest.java index d1aac0d..3904167 100644 --- a/fleetd/src/test/java/dev/ltms/fleet/mcp/FleetMcpTest.java +++ b/fleetd/src/test/java/dev/ltms/fleet/mcp/FleetMcpTest.java @@ -676,6 +676,95 @@ class FleetMcpTest { "an ordinary fleet's output must be unchanged by this feature"); } + /** + * fleetd #439: a worker calling {@code fleet_list} must get a result with the {@code + * coordinator} key absent -- not an empty object, not a redacted one -- even + * though lead coordination is fully configured and would otherwise report a row. This drives + * the same {@code callerIsPrimary} value the MCP handler computes ({@code + * Principal.worker(...).isPrimary()}), so it pins the real production boolean, not a literal. + */ + @Test + void listOmitsTheCoordinatorKeyEntirelyForAWorkerEvenWhenLeadCoordinationIsOn() { + FakeHerdr h = new FakeHerdr(); + SessionManager sessions = new SessionManager(workerService(h, "http://gx00.gw:8000", Set.of("gx00.gw"))); + FakeLeadChannel channel = new FakeLeadChannel("mac-opus") + .withMailbox("mac-opus", LeadChannel.MailboxState.exists("mac-opus", 0, 1)); + boolean callerIsPrimary = Principal.worker("term_a", 1).isPrimary(); + + McpSchema.CallToolResult res = FleetMcp.listFleet( + workerService(h, "http://gx00.gw:8000", Set.of("gx00.gw")), sessions, null, + FleetMcp.CapacitySource.none(), new FleetMcp.HealthCoverageSource(() -> "off"), + FleetMcp.QuarantineSource.none(), FleetMcp.OutageSource.none(), FleetMcp.LeadSeatSource.none(), + Map.of(), "", new FleetMcp.CoordinationSource(channel, List.of()), callerIsPrimary); + + String out = textOf(res); + assertFalse(out.contains("\"coordinator\""), "a worker must never see the coordinator key at all: " + out); + assertFalse(out.contains("mac-opus"), "no fragment of the coordinator row may leak either: " + out); + assertTrue(out.contains("\"leads\""), "the rest of the result must still be present: " + out); + assertTrue(out.contains("\"members\""), out); + assertTrue(out.contains("\"healthCoverage\""), out); + } + + /** + * fleetd #439 acceptance criterion 2: an architect gets exactly the same treatment as a worker. + * This is a real, executed test (not just reasoning by analogy) -- it drives the actual + * {@code Principal.architect(...).isPrimary()} value the production handler would compute for + * an architect caller, through the same gate a worker's call goes through. + */ + @Test + void listOmitsTheCoordinatorKeyEntirelyForAnArchitectToo() { + FakeHerdr h = new FakeHerdr(); + SessionManager sessions = new SessionManager(workerService(h, "http://gx00.gw:8000", Set.of("gx00.gw"))); + FakeLeadChannel channel = new FakeLeadChannel("mac-opus") + .withMailbox("mac-opus", LeadChannel.MailboxState.exists("mac-opus", 0, 1)); + boolean callerIsPrimary = Principal.architect("lead-designer", "term_design", 400).isPrimary(); + + McpSchema.CallToolResult res = FleetMcp.listFleet( + workerService(h, "http://gx00.gw:8000", Set.of("gx00.gw")), sessions, null, + FleetMcp.CapacitySource.none(), new FleetMcp.HealthCoverageSource(() -> "off"), + FleetMcp.QuarantineSource.none(), FleetMcp.OutageSource.none(), FleetMcp.LeadSeatSource.none(), + Map.of(), "", new FleetMcp.CoordinationSource(channel, List.of()), callerIsPrimary); + + String out = textOf(res); + assertFalse(out.contains("\"coordinator\""), "an architect must never see the coordinator key either: " + out); + } + + /** + * fleetd #439 acceptance criterion 3: the primary's {@code fleet_list} is byte-for-byte + * unchanged by this fix. Proven by comparing the new gated overload (with {@code + * callerIsPrimary=true}, exactly what the MCP handler passes for the primary) against the + * pre-#439 overload that always assembled the row -- if the gate changed anything for a + * primary caller, these two strings would differ. + */ + @Test + void listIsByteForByteUnchangedForThePrimaryCaller() { + FakeHerdr h = new FakeHerdr(); + SessionManager sessions = new SessionManager(workerService(h, "http://gx00.gw:8000", Set.of("gx00.gw"))); + FakeLeadChannel channel = new FakeLeadChannel("mac-opus") + .withMailbox("mac-opus", LeadChannel.MailboxState.exists("mac-opus", 0, 1)); + + String preExisting = textOf(FleetMcp.listFleet( + workerService(h, "http://gx00.gw:8000", Set.of("gx00.gw")), sessions, null, + FleetMcp.CapacitySource.none(), new FleetMcp.HealthCoverageSource(() -> "off"), + FleetMcp.QuarantineSource.none(), Map.of(), "", + new FleetMcp.CoordinationSource(channel, List.of()))); + String gatedAsPrimary = textOf(FleetMcp.listFleet( + workerService(h, "http://gx00.gw:8000", Set.of("gx00.gw")), sessions, null, + FleetMcp.CapacitySource.none(), new FleetMcp.HealthCoverageSource(() -> "off"), + FleetMcp.QuarantineSource.none(), FleetMcp.OutageSource.none(), FleetMcp.LeadSeatSource.none(), + Map.of(), "", new FleetMcp.CoordinationSource(channel, List.of()), true)); + + assertEquals(preExisting, gatedAsPrimary, + "a primary caller must see byte-for-byte the same result as before this fix"); + assertTrue(gatedAsPrimary.contains("\"coordinator\""), gatedAsPrimary); + assertTrue(gatedAsPrimary.contains("\"selfId\":\"mac-opus\""), gatedAsPrimary); + assertTrue(gatedAsPrimary.contains("\"mailbox\""), gatedAsPrimary); + assertTrue(gatedAsPrimary.contains("\"heldCount\""), gatedAsPrimary); + assertTrue(gatedAsPrimary.contains("\"heldDurable\""), gatedAsPrimary); + assertTrue(gatedAsPrimary.contains("\"held\""), gatedAsPrimary); + assertTrue(gatedAsPrimary.contains("\"peers\""), gatedAsPrimary); + } + @Test void listReportsHeldMessagesWithATruncatedPreviewNeverTheFullBody() { FakeHerdr h = new FakeHerdr();