fleetd #439: omit fleet_list's coordinator key for non-primary callers
CI / contract (pull_request) Successful in 56s
CI / build (pull_request) Successful in 1m40s

The coordinator row is lead-to-lead coordination state (coord-ids, mailbox
facts, held-message previews). fleet_list returned it to every caller,
including a worker or an architect, because coordinatorView() had no way to
know who was asking.

Gate at the call site inside listFleet: a new overload takes
callerIsPrimary and only assembles/attaches the coordinator row when it is
true, so the key is absent (not empty) for a worker or an architect. The
MCP handler now passes principal(exchange).isPrimary(); every other
listFleet overload keeps passing true, so callers with no caller identity
(existing unit tests, the no-op wrappers) are unaffected -- confirmed by a
byte-for-byte comparison test against the pre-fix overload.

Authz's READ case is untouched: it stays shared by fleet_status,
fleet_profiles and fleet_whoami, and the gate here is purely inside
fleet_list's own result assembly.
This commit is contained in:
Dai Ha
2026-09-10 18:16:48 +07:00
parent 3c5873dfe2
commit e54e3d87ea
2 changed files with 132 additions and 5 deletions
@@ -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<McpSyncServerExchange, McpSchema.CallToolRequest, McpSchema.CallToolResult> 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}).
*
* <p>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<String, String> 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
* <strong>absent</strong>, 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<String, String> leads, String selfTerm,
CoordinationSource coordination, boolean callerIsPrimary) {
try {
Map<String, Agent> live = workers.list().stream()
.map(Agent.class::cast)
@@ -1371,9 +1404,14 @@ public final class FleetMcp {
Map<String, Object> result = new LinkedHashMap<>();
result.put("leads", leadRows); result.put("members", out);
result.put("healthCoverage", healthCoverage.value().get());
Map<String, Object> 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<String, Object> 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,
@@ -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 <strong>absent</strong> -- 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();