fleetd #480 correction round: collapse FleetMcp to one required constructor
CI / contract (pull_request) Successful in 55s
CI / build (pull_request) Successful in 1m32s

FleetMcp had a defaulted 15-argument constructor that delegated to the new
16-argument one with an implicit null for leadRollover. Dropping the
leadRollover argument from Fleetd.main's FleetMcp(...) call fell back to that
shorter overload, compiled fine, and left all 1659 tests green — the live
daemon would then answer NOT_CONFIGURED to fleet_handover forever with
nothing going red.

Delete every overload that could reach the 16-arg constructor with a
silently-defaulted leadRollover (11/12/13/14/15-arg forms all chained to it),
leaving the 16-arg constructor as FleetMcp's sole public constructor. Update
FleetMcpAuthzTest's call site to pass every parameter explicitly (leadChannel
null, OutageSource.none(), LeadSeatSource.none(), List.of(), leadRollover
null) — Fleetd.java and FleetMcpHandoverTest already called the full form.
Proved with mvn -o -q compile: removing the leadRollover argument from
Fleetd.main now fails to compile instead of silently defaulting.

No behaviour changes — NOT_CONFIGURED refusals are unchanged.
This commit is contained in:
Dai Ha
2026-09-11 07:25:08 +07:00
parent 62646957ea
commit eb0557621e
2 changed files with 51 additions and 85 deletions
@@ -251,91 +251,52 @@ public final class FleetMcp {
}
/**
* @param callers resolves each call's {@link Principal}; {@code null} disables authorization.
* This surface needs its own enforcement: {@code /mcp} is a raw servlet on
* Jetty's context handler and never passes through Javalin's {@code before}
* filter, so the REST guard does not cover it.
* @param metrics registry for auth-failure counting; may be {@code null}
* @param quarantine CB-578 stage B facts for {@code fleet_profiles}; required — pass
* {@link QuarantineSource#none()} for a caller that does not want the feature
*/
public FleetMcp(MessageService messages, PeerLauncher workers, SessionManager sessions,
ConnectionIdentity identity, MemberPresence presence, PrimaryRegistry primaryRegistry,
CallerResolver callers, Metrics metrics, CapacitySource capacity, HealthCoverageSource healthCoverage,
QuarantineSource quarantine) {
this(messages, workers, sessions, identity, presence, primaryRegistry, callers, metrics, capacity,
healthCoverage, quarantine, null, OutageSource.none(), LeadSeatSource.none());
}
/**
* As above, with this daemon's lead-to-lead channel (CB-637). {@code leadChannel} is
* {@code null} whenever no {@code coordinator:} block is configured or its broker could not be
* reached at boot — cross-daemon lead messaging is simply off, and {@code fleet_send{coordId}}
* says so rather than failing obscurely.
*/
public FleetMcp(MessageService messages, PeerLauncher workers, SessionManager sessions,
ConnectionIdentity identity, MemberPresence presence, PrimaryRegistry primaryRegistry,
CallerResolver callers, Metrics metrics, CapacitySource capacity, HealthCoverageSource healthCoverage,
QuarantineSource quarantine, LeadChannel leadChannel) {
this(messages, workers, sessions, identity, presence, primaryRegistry, callers, metrics, capacity,
healthCoverage, quarantine, leadChannel, OutageSource.none(), LeadSeatSource.none());
}
/**
* As above, with fleetd #201 Unit 5 cool-off facts for {@code fleet_list}/{@code fleet_profiles}
* (see {@link OutageSource}).
* The only constructor (fleetd #480 Unit C correction round). Every field below used to have
* its own defaulting overload — {@code leadChannel}/{@code outage}/{@code leadSeats}/
* {@code peers}/{@code leadRollover} each got a shorter, convenience constructor that silently
* filled it in ({@code null}, {@code .none()}, or {@code List.of()}) when a caller did not pass
* it. That is exactly how {@code Fleetd.main}'s wiring of {@link LeadRollover} could have gone
* silently missing: drop one argument from the real call and it just lands on a shorter
* overload instead of failing to compile, and every existing test — none of which exercises
* {@code Fleetd.main} itself — stays green while the live daemon quietly answers
* {@code NOT_CONFIGURED} to {@code fleet_handover} forever. Collapsing every overload into one
* required-everything constructor turns that mistake into a compile error instead: this
* project's own antidote for a defaulted parameter surviving as an untested decision (see
* {@code FleetdCompletionResolverWiringTest} / {@code FleetdLeadRolloverWiringTest}'s own
* javadoc for the same lesson applied to a different seam). A caller that genuinely wants a
* feature off must now say so explicitly at the call site — {@code null},
* {@link OutageSource#none()}, {@link LeadSeatSource#none()}, {@code List.of()} are all still
* perfectly fine values, just never an implicit default reached by omission.
*
* @param outage required — pass {@link OutageSource#none()} for a caller that does not want the
* feature, never a defaulting overload (the same rule {@code quarantine} follows).
*/
public FleetMcp(MessageService messages, PeerLauncher workers, SessionManager sessions,
ConnectionIdentity identity, MemberPresence presence, PrimaryRegistry primaryRegistry,
CallerResolver callers, Metrics metrics, CapacitySource capacity, HealthCoverageSource healthCoverage,
QuarantineSource quarantine, LeadChannel leadChannel, OutageSource outage) {
this(messages, workers, sessions, identity, presence, primaryRegistry, callers, metrics, capacity,
healthCoverage, quarantine, leadChannel, outage, LeadSeatSource.none());
}
/**
* As above, with fleetd #176 lead-seat facts (see {@link LeadSeatSource}).
*/
public FleetMcp(MessageService messages, PeerLauncher workers, SessionManager sessions,
ConnectionIdentity identity, MemberPresence presence, PrimaryRegistry primaryRegistry,
CallerResolver callers, Metrics metrics, CapacitySource capacity, HealthCoverageSource healthCoverage,
QuarantineSource quarantine, LeadChannel leadChannel, OutageSource outage,
LeadSeatSource leadSeats) {
this(messages, workers, sessions, identity, presence, primaryRegistry, callers, metrics, capacity,
healthCoverage, quarantine, leadChannel, outage, leadSeats, List.of());
}
/**
* As above, with fleetd #361 {@code coordinator.peers} (see {@link CoordinationSource}).
*
* @param leadSeats required — pass {@link LeadSeatSource#none()} for a caller that does not want
* the feature, never a defaulting overload (the same rule {@code quarantine} and
* {@code outage} follow).
* @param peers the coord-ids declared under {@code coordinator.peers}; empty when unset or
* when {@code leadChannel} is {@code null}.
*/
public FleetMcp(MessageService messages, PeerLauncher workers, SessionManager sessions,
ConnectionIdentity identity, MemberPresence presence, PrimaryRegistry primaryRegistry,
CallerResolver callers, Metrics metrics, CapacitySource capacity, HealthCoverageSource healthCoverage,
QuarantineSource quarantine, LeadChannel leadChannel, OutageSource outage,
LeadSeatSource leadSeats, List<String> peers) {
this(messages, workers, sessions, identity, presence, primaryRegistry, callers, metrics, capacity,
healthCoverage, quarantine, leadChannel, outage, leadSeats, peers, null);
}
/**
* As above, with fleetd #480 Unit C: the {@link LeadRollover} executor behind
* {@code fleet_handover}. This is what {@code Fleetd.main} actually wires up.
*
* @param leadRollover {@code null} whenever {@code leadRollover:} is not configured — an
* upgraded daemon must never silently acquire the ability to clear a lead's
* own pane (mirrors {@code Fleetd.leadRollover(...)}'s own construction
* gate). {@code fleet_handover} is registered unconditionally either way —
* see this class's javadoc and fleetd #474's charter tool-surface gate —
* and every action degrades to a clean refusal naming {@code NOT_CONFIGURED}
* @param callers resolves each call's {@link Principal}; {@code null} disables
* authorization. This surface needs its own enforcement: {@code /mcp} is a
* raw servlet on Jetty's context handler and never passes through
* Javalin's {@code before} filter, so the REST guard does not cover it.
* @param metrics registry for auth-failure counting; may be {@code null}
* @param quarantine CB-578 stage B facts for {@code fleet_profiles}; pass
* {@link QuarantineSource#none()} for a caller that does not want the
* feature
* @param leadChannel this daemon's lead-to-lead channel (CB-637); {@code null} whenever no
* {@code coordinator:} block is configured or its broker could not be
* reached at boot — cross-daemon lead messaging is simply off, and
* {@code fleet_send{coordId}} says so rather than failing obscurely
* @param outage fleetd #201 Unit 5 cool-off facts for {@code fleet_list}/
* {@code fleet_profiles}; pass {@link OutageSource#none()} for a caller
* that does not want the feature
* @param leadSeats fleetd #176 lead-seat facts (see {@link LeadSeatSource}); pass
* {@link LeadSeatSource#none()} for a caller that does not want the
* feature
* @param peers fleetd #361 {@code coordinator.peers} (see {@link CoordinationSource});
* the coord-ids declared there, or empty when unset or when
* {@code leadChannel} is {@code null}
* @param leadRollover fleetd #480 Unit C: the {@link LeadRollover} executor behind
* {@code fleet_handover}. {@code null} whenever {@code leadRollover:} is
* not configured — an upgraded daemon must never silently acquire the
* ability to clear a lead's own pane (mirrors
* {@code Fleetd.leadRollover(...)}'s own construction gate).
* {@code fleet_handover} is registered unconditionally either way — see
* this class's javadoc and fleetd #474's charter tool-surface gate — and
* every action degrades to a clean refusal naming {@code NOT_CONFIGURED}
* instead of throwing. See {@link #handover}.
*/
public FleetMcp(MessageService messages, PeerLauncher workers, SessionManager sessions,
@@ -26,6 +26,7 @@ import org.junit.jupiter.api.Test;
import java.nio.file.Files;
import java.nio.file.Path;
import java.util.List;
import java.util.Map;
import java.util.Set;
import java.util.regex.Matcher;
@@ -74,12 +75,16 @@ class FleetMcpAuthzTest {
ConnectionIdentity identity = new ConnectionIdentity(new PaneLocator(herdr), _ -> 999_999);
metrics = FleetMetrics.create(sessions, new InMemoryReplyInbox());
// fleetd #480 correction round: FleetMcp has one constructor now (no defaulting
// overloads — see its javadoc), so every feature this test does not exercise is passed
// its explicit "off" value here rather than being omitted.
mcp = new FleetMcp(messages, workers, sessions, identity, sessions.asPresence(),
new PrimaryRegistry(null),
enforce ? CallerResolver.withLeadsAndMembers(identity, false, null,
Map::of, new MemberRegistry(null)) : null,
metrics, FleetMcp.CapacitySource.none(), new FleetMcp.HealthCoverageSource(() -> "off"),
FleetMcp.QuarantineSource.none());
FleetMcp.QuarantineSource.none(), null, FleetMcp.OutageSource.none(),
FleetMcp.LeadSeatSource.none(), List.of(), null);
return mcp;
}