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 f2d5144..379a2b3 100644 --- a/fleetd/src/main/java/dev/ltms/fleet/mcp/FleetMcp.java +++ b/fleetd/src/main/java/dev/ltms/fleet/mcp/FleetMcp.java @@ -1362,10 +1362,12 @@ public final class FleetMcp { /** * 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 + *

Assumes the caller is not the primary (fleetd #463) — 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 a missing identity should fail closed rather than fail open onto lead-to-lead state. A + * test that wants the {@code coordinator} row must call the canonical overload below with an + * explicit {@code true}. 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. @@ -1376,7 +1378,7 @@ public final class FleetMcp { LeadSeatSource leadSeats, Map leads, String selfTerm, CoordinationSource coordination) { return listFleet(workers, sessions, messages, capacity, healthCoverage, quarantine, outage, - leadSeats, leads, selfTerm, coordination, true); + leadSeats, leads, selfTerm, coordination, false); } /** @@ -1390,7 +1392,9 @@ public final class FleetMcp { * @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} + * {@code false} (fleetd #463: a forgotten argument fails closed, not + * open), so a test that wants the {@code coordinator} row must pass + * an explicit {@code true} */ static McpSchema.CallToolResult listFleet(PeerLauncher workers, SessionManager sessions, MessageService messages, CapacitySource capacity, HealthCoverageSource healthCoverage, 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 3904167..85fa9c4 100644 --- a/fleetd/src/test/java/dev/ltms/fleet/mcp/FleetMcpTest.java +++ b/fleetd/src/test/java/dev/ltms/fleet/mcp/FleetMcpTest.java @@ -626,8 +626,8 @@ class FleetMcpTest { 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(), Map.of(), "", - new FleetMcp.CoordinationSource(channel, List.of())); + FleetMcp.QuarantineSource.none(), FleetMcp.OutageSource.none(), FleetMcp.LeadSeatSource.none(), + Map.of(), "", new FleetMcp.CoordinationSource(channel, List.of()), true); String out = textOf(res); // fleetd #361: reports both which coord-id a peer must use to reach ME, and this daemon's @@ -655,8 +655,8 @@ class FleetMcpTest { 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(), Map.of(), "", - new FleetMcp.CoordinationSource(channel, List.of())); + FleetMcp.QuarantineSource.none(), FleetMcp.OutageSource.none(), FleetMcp.LeadSeatSource.none(), + Map.of(), "", new FleetMcp.CoordinationSource(channel, List.of()), true); String out = textOf(res); assertTrue(out.contains("\"mailbox\":{\"status\":\"unknown\"}"), out); @@ -676,6 +676,32 @@ class FleetMcpTest { "an ordinary fleet's output must be unchanged by this feature"); } + /** + * fleetd #463: a compat overload called with no {@code callerIsPrimary} argument at all must + * fail closed, not open. Before this fix the hidden default was {@code true}, so a caller that + * forgot the argument silently got lead-to-lead coordination state. Lead coordination is fully + * configured here (a real channel, a real mailbox) specifically so this is not conflated with + * {@link #listOmitsTheCoordinatorRowWhenLeadCoordinationIsOff} -- the row is capable of being + * assembled, and the missing argument is the only reason it is not. + */ + @Test + void listCompatOverloadWithNoCallerIsPrimaryArgumentOmitsTheCoordinatorKey() { + 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)); + + 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(), Map.of(), "", + new FleetMcp.CoordinationSource(channel, List.of())); + + String out = textOf(res); + assertFalse(out.contains("\"coordinator\""), + "no callerIsPrimary argument must fail closed (absent), not open (present): " + out); + } + /** * 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 @@ -730,11 +756,15 @@ class FleetMcpTest { } /** - * 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. + * fleetd #439 acceptance criterion 3: an explicitly-primary caller sees the coordinator row + * fully assembled, with the same content #439 always produced for a primary. + * + *

fleetd #463 flipped the compat overloads' hidden default from {@code true} to + * {@code false} (fail closed), so the old "pre-#439 overload" this test used to compare + * against no longer stands in for a primary caller -- it is now exactly the implicit-default + * path #463 closes. Verifying the primary path means calling the canonical overload with an + * explicit {@code callerIsPrimary=true} directly, as the production {@code fleet_list} handler + * does. */ @Test void listIsByteForByteUnchangedForThePrimaryCaller() { @@ -743,19 +773,12 @@ class FleetMcpTest { 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); @@ -780,8 +803,8 @@ class FleetMcpTest { 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(), Map.of(), "", - new FleetMcp.CoordinationSource(channel, List.of())); + FleetMcp.QuarantineSource.none(), FleetMcp.OutageSource.none(), FleetMcp.LeadSeatSource.none(), + Map.of(), "", new FleetMcp.CoordinationSource(channel, List.of()), true); String out = textOf(res); assertTrue(out.contains("\"msgId\":\"m1\""), out); @@ -812,8 +835,8 @@ class FleetMcpTest { 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(), Map.of(), "", - new FleetMcp.CoordinationSource(channel, List.of())); + FleetMcp.QuarantineSource.none(), FleetMcp.OutageSource.none(), FleetMcp.LeadSeatSource.none(), + Map.of(), "", new FleetMcp.CoordinationSource(channel, List.of()), true); String out = textOf(res); assertTrue(out.contains("\"pending\":0"), out); @@ -841,8 +864,8 @@ class FleetMcpTest { 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(), Map.of(), "", - new FleetMcp.CoordinationSource(channel, List.of())); + FleetMcp.QuarantineSource.none(), FleetMcp.OutageSource.none(), FleetMcp.LeadSeatSource.none(), + Map.of(), "", new FleetMcp.CoordinationSource(channel, List.of()), true); String out = textOf(res); assertTrue(out.contains("\"heldDurable\":false"), @@ -909,8 +932,9 @@ class FleetMcpTest { 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(), Map.of(), "", - new FleetMcp.CoordinationSource(channel, List.of("fleet01-lead", "fleet02-lead", "fleet03-lead"))); + FleetMcp.QuarantineSource.none(), FleetMcp.OutageSource.none(), FleetMcp.LeadSeatSource.none(), + Map.of(), "", + new FleetMcp.CoordinationSource(channel, List.of("fleet01-lead", "fleet02-lead", "fleet03-lead")), true); String out = textOf(res); assertTrue(out.contains("\"coordId\":\"fleet01-lead\",\"status\":\"exists\",\"pending\":2,\"consumers\":1"), out);