diff --git a/fleetd/src/main/java/dev/ltms/fleet/Fleetd.java b/fleetd/src/main/java/dev/ltms/fleet/Fleetd.java index f8ee3cb..c8ac0c3 100644 --- a/fleetd/src/main/java/dev/ltms/fleet/Fleetd.java +++ b/fleetd/src/main/java/dev/ltms/fleet/Fleetd.java @@ -69,6 +69,7 @@ import java.util.List; import java.util.Map; import java.util.Optional; import java.util.Set; +import java.util.TreeSet; import java.util.concurrent.Executors; import java.util.concurrent.ScheduledExecutorService; import java.util.concurrent.TimeUnit; @@ -231,6 +232,13 @@ public final class Fleetd { profileName -> liveCountRef.get().apply(profileName), quarantine, outagePolicy); + // fleetd #422 follow-up: say which of the three model-gate states the daemon booted into — + // no models: block at all, a block armed with nothing off, or a block with N off — the same + // way exhaustedPatternCoverageLine/errorPatternCoverageLine report CB-578 stage A/fleetd + // #201 Unit 5 coverage just below. Read from workers.modelGateState() (never a separate + // config.get().models() here) so this line and fleet_profiles' modelGateArmed can never + // disagree about what CompositePeerLauncher's spawn gate actually enforces. + log.info("model gate (fleetd #422): {}", modelGateCoverageLine(workers.modelGateState())); // CB-504: under supervision (launchd/systemd) fleetd can start before herdr's socket // exists. The client itself is lazy — it connects per call — but the orphan reap below is // the first thing that actually talks to herdr, so without this wait a boot-order race @@ -844,6 +852,31 @@ public final class Fleetd { allProfiles, configuredProfiles); } + /** + * fleetd #422 follow-up: package-private factory for the startup line reporting which of the + * three central {@code models.allow:} gate states the daemon booted into. Extracted the same + * way {@link #exhaustedPatternCoverageLine}/{@link #errorPatternCoverageLine} are, so a + * dedicated test can call it directly rather than parsing log output, and so {@code main}'s + * only source for this line is {@link PeerLauncher#modelGateState()} — never a second, + * independently-derived read of {@code cfg.models()} that could disagree with what {@code + * CompositePeerLauncher}'s spawn gate actually enforces (the fleetd #404 lesson). + * + *

Unlike the two pattern-key lines above, there is no {@code UnsetMeaning} choice to make + * here: {@link PeerLauncher.ModelGateState#configured()} already states, unambiguously, whether + * an empty {@link PeerLauncher.ModelGateState#off()} means "no {@code models:} block to gate + * with" or "a block armed and currently reporting zero off" — the exact two states a bare + * {@code disabledModels()} read could not tell apart before this ticket. + */ + static String modelGateCoverageLine(PeerLauncher.ModelGateState state) { + if (!state.configured()) { + return "not configured (no models: block — nothing is gated, and nothing can be)"; + } + Set off = state.off(); + return off.isEmpty() + ? "armed (models: block present; 0 models currently turned off)" + : "armed (" + off.size() + " model(s) turned off: " + new TreeSet<>(off) + ")"; + } + /** * fleetd #416: production source for {@code fleet_list}'s per-profile capacity facts. * 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 52d8082..6685105 100644 --- a/fleetd/src/main/java/dev/ltms/fleet/mcp/FleetMcp.java +++ b/fleetd/src/main/java/dev/ltms/fleet/mcp/FleetMcp.java @@ -1250,12 +1250,21 @@ public final class FleetMcp { result.put("coolingOff", coolingOff); } // fleetd #422: read the exact same accessor CompositePeerLauncher's spawn gate reads - // (PeerLauncher.disabledModels(), which for the composite is models0().offIds()) — never a + // (PeerLauncher.modelGateState(), which for the composite is models0() read live) — never a // separately-derived answer, so this status can never overstate or understate what the gate // actually enforces (the fleetd #404 lesson). - Set modelsOff = workers.disabledModels(); - if (!modelsOff.isEmpty()) { - result.put("modelsOff", new ArrayList<>(modelsOff)); + // + // fleetd #422 follow-up: "armed" and "off" come from the ONE modelGateState() call below, + // never two independent reads of the gate — a reload landing between two separate reads + // could otherwise make them disagree. modelGateArmed is reported unconditionally (never + // omitted like quarantined/coolingOff above) precisely so a lead can tell "no models: block + // at all" (false) apart from "a models: block with nothing currently off" (true, with + // modelsOff simply absent below) — the two states PeerLauncher.disabledModels() alone + // cannot distinguish, both reporting an empty set. + PeerLauncher.ModelGateState modelGate = workers.modelGateState(); + result.put("modelGateArmed", modelGate.configured()); + if (!modelGate.off().isEmpty()) { + result.put("modelsOff", new ArrayList<>(modelGate.off())); } return result; } diff --git a/fleetd/src/main/java/dev/ltms/fleet/member/CompositePeerLauncher.java b/fleetd/src/main/java/dev/ltms/fleet/member/CompositePeerLauncher.java index 49c64fe..e40e3b7 100644 --- a/fleetd/src/main/java/dev/ltms/fleet/member/CompositePeerLauncher.java +++ b/fleetd/src/main/java/dev/ltms/fleet/member/CompositePeerLauncher.java @@ -644,14 +644,29 @@ public final class CompositePeerLauncher implements PeerLauncher { } /** - * fleetd #422: read live off {@link #models0()} — the exact same accessor {@link + * fleetd #422 follow-up: the single live read that answers both "is the models.allow: gate + * armed" and "which models are off", off the exact same accessor ({@link #models0()}) {@link * #enforceModelEnabled} and {@link #modelOffProfiles} read — so {@code fleet_profiles}/{@code - * GET /profiles} can never report a different answer than the gate enforces (the fleetd #404 - * lesson). + * GET /profiles} (via {@link PeerLauncher#disabledModels()}, which now delegates here) can + * never report a different answer than the gate enforces (the fleetd #404 lesson), and the + * startup log line built from this can never disagree with either. + * + *

{@link #models0()} itself normalizes a {@code null} {@link #models} read to the shared + * {@link #NO_MODELS_CONFIGURED} sentinel — deliberately the one object no config-supplied + * {@code Models} instance can ever be identical to, since it is private to this class — so + * comparing by reference here recovers exactly the fact {@code models0()}'s normalization + * would otherwise erase: whether the live source was {@code null} (no {@code models:} block, + * armed = false) or a real, config-supplied block (armed = true, even one whose {@code allow:} + * is itself empty or absent — {@link FleetConfig.Models}'s "absent or empty allow: is off" + * wording governs config-load validation, a distinct question from whether this gate is armed + * for reporting). */ @Override - public Set disabledModels() { - return models0().offIds(); + public PeerLauncher.ModelGateState modelGateState() { + FleetConfig.Models m = models0(); + return m == NO_MODELS_CONFIGURED + ? PeerLauncher.ModelGateState.notConfigured() + : PeerLauncher.ModelGateState.armed(m.offIds()); } @Override diff --git a/fleetd/src/main/java/dev/ltms/fleet/peer/PeerLauncher.java b/fleetd/src/main/java/dev/ltms/fleet/peer/PeerLauncher.java index 2024a3e..fa6a9cf 100644 --- a/fleetd/src/main/java/dev/ltms/fleet/peer/PeerLauncher.java +++ b/fleetd/src/main/java/dev/ltms/fleet/peer/PeerLauncher.java @@ -202,8 +202,56 @@ public interface PeerLauncher { * can drift from what the gate ({@code CompositePeerLauncher.enforceModelEnabled} and its * candidate filter) actually enforces. A default of {@code Set.of()} keeps every other {@link * PeerLauncher} implementer (the herdr adapters, and the two test-fake implementers) unchanged. + * + *

fleetd #422 follow-up: this alone cannot tell "no {@code models:} block at all" from "a + * {@code models:} block where nothing is currently off" — both report an empty set here. Delegates + * to {@link #modelGateState()} so the two facts always come from the one read {@link + * #modelGateState()}'s implementer makes; do not override this method separately from that one. */ default Set disabledModels() { - return Set.of(); + return modelGateState().off(); + } + + /** + * Whether the central {@code models.allow:} gate (fleetd #422) is armed at all, together with + * which model ids are currently off — fleetd #422 follow-up. {@link #disabledModels()} alone + * cannot distinguish two states that both report an empty set: a host with no {@code models:} + * block (nothing is gated, and nothing can be) and a host WITH a {@code models:} block where + * nothing is currently turned off (the gate is armed and reporting zero). This method exists so + * a caller — the startup log, {@code fleet_profiles}/{@code GET /profiles} — can tell the two + * apart, the same reason {@code CompletionResolver.UnsetMeaning} exists: an accessor that can + * legitimately report "empty" must never let a caller guess why. + * + *

Default {@link ModelGateState#notConfigured()} — every launcher without a {@code models:} + * block to read from (the herdr adapters, and the two test-fake implementers), matching {@link + * #disabledModels()}'s own default of an empty set. + */ + default ModelGateState modelGateState() { + return ModelGateState.notConfigured(); + } + + /** + * fleetd #422 follow-up: the result of {@link #modelGateState()} — see that method's javadoc + * for why "armed" and "off" must be reported together from one read rather than as two + * separately-derived facts that a reload landing between them could make disagree. + * + * @param configured {@code true} when a {@code models:} block exists at all (armed), regardless + * of whether anything in it is currently turned off; {@code false} when there + * is no block to gate against + * @param off the model ids currently turned off; always empty when {@code configured} is + * {@code false} + */ + record ModelGateState(boolean configured, Set off) { + public ModelGateState { + off = Set.copyOf(off); + } + + public static ModelGateState notConfigured() { + return new ModelGateState(false, Set.of()); + } + + public static ModelGateState armed(Set off) { + return new ModelGateState(true, off); + } } } diff --git a/fleetd/src/main/java/dev/ltms/fleet/placement/FixedPlacementPolicy.java b/fleetd/src/main/java/dev/ltms/fleet/placement/FixedPlacementPolicy.java index 4a0ca61..79cb0a1 100644 --- a/fleetd/src/main/java/dev/ltms/fleet/placement/FixedPlacementPolicy.java +++ b/fleetd/src/main/java/dev/ltms/fleet/placement/FixedPlacementPolicy.java @@ -5,14 +5,12 @@ import java.util.List; /** * Backward-compatible placement: an unqualified spawn always resolves to the configured default - * profile, exactly as {@code CompositePeerLauncher} did before CB-518. This ignores caps - * ({@code maxLoad}) so that a pre-existing config behaves identically after upgrade — capacity - * gating for automatic placement is deliberately out of scope for {@code fixed}, exactly as it - * always has been. Reachability is a narrower exception (fleetd #315, below): a profile is never - * checked for reachability up front, only skipped once it has already failed in this same - * spawn call's retry loop — see the unreachable case below. + * profile, exactly as {@code CompositePeerLauncher} did before CB-518. Reachability is a narrower + * exception (fleetd #315, below): a profile is never checked for reachability up front, only + * skipped once it has already failed in this same spawn call's retry loop — see the + * unreachable case below. * - *

Five exceptions walk past the default instead of returning it unconditionally: + *

Six exceptions walk past the default instead of returning it unconditionally: *

- * A fleet where nothing is ever quarantined, cooling off, model-off, unreachable, or weight-0 never - * exercises any of these paths, so today's behaviour is unchanged — in particular, the very first - * selection of a spawn call always sees an empty {@code unreachable} set, so the first choice is - * untouched. + * A fleet where nothing is ever quarantined, cooling off, at cap, model-off, unreachable, or + * weight-0 never exercises any of these paths, so today's behaviour is unchanged — in particular, + * the very first selection of a spawn call always sees an empty {@code unreachable} set, so the + * first choice is untouched. */ final class FixedPlacementPolicy implements PlacementPolicy { @@ -51,13 +59,14 @@ final class FixedPlacementPolicy implements PlacementPolicy { public PlacementCandidate select(PlacementContext ctx) { String d = ctx.defaultProfile(); if (d != null && !d.isBlank() && !ctx.quarantined().contains(d) && !ctx.coolingOff().contains(d) - && !ctx.modelOff().contains(d) && !ctx.unreachable().contains(d) && !weightExcluded(ctx, d)) { + && !ctx.modelOff().contains(d) && !ctx.unreachable().contains(d) && !weightExcluded(ctx, d) + && !capExcluded(ctx, d)) { return new PlacementCandidate(d, null, 1.0f, null); } for (PlacementCandidate c : ctx.candidates()) { if (!ctx.quarantined().contains(c.profile()) && !ctx.coolingOff().contains(c.profile()) && !ctx.modelOff().contains(c.profile()) && !ctx.unreachable().contains(c.profile()) - && !c.excluded()) { + && !c.excluded() && !PlacementPolicyUtil.atCap(ctx, c)) { return new PlacementCandidate(c.profile(), null, c.weight(), c.maxLoad()); } } @@ -66,14 +75,18 @@ final class FixedPlacementPolicy implements PlacementPolicy { // Exhaustion quarantine takes priority: reported only when quarantine is absent, so the // message never claims "cooling off" for a profile that is really backend-exhausted. boolean dCoolingOff = !dQuarantined && ctx.coolingOff().contains(d); - // fleetd #422: model-off is a fourth, independent source (an operator decision) — but - // quarantine/cooling-off still take priority when more than one applies, matching + // fleetd #435: at-cap sits between cooling off and model-off, matching + // CompositePeerLauncher's explicit-spawn check order (quarantine, cooling off, max load, + // then model-off) — reported only when quarantine/cooling-off are both absent. + boolean dAtCap = !dQuarantined && !dCoolingOff && capExcluded(ctx, d); + // fleetd #422: model-off is a fifth, independent source (an operator decision) — but + // quarantine/cooling-off/at-cap still take priority when more than one applies, matching // CompositePeerLauncher's explicit-spawn check order (quarantine, cooling off, max load, // then model-off). - boolean dModelOff = !dQuarantined && !dCoolingOff && ctx.modelOff().contains(d); + boolean dModelOff = !dQuarantined && !dCoolingOff && !dAtCap && ctx.modelOff().contains(d); boolean dUnreachable = ctx.unreachable().contains(d); boolean dWeightExcluded = weightExcluded(ctx, d); - if (dQuarantined || dCoolingOff || dModelOff || dUnreachable || dWeightExcluded) { + if (dQuarantined || dCoolingOff || dAtCap || dModelOff || dUnreachable || dWeightExcluded) { List reasons = new ArrayList<>(); if (dQuarantined) { reasons.add("is quarantined (backend exhausted)"); @@ -81,6 +94,11 @@ final class FixedPlacementPolicy implements PlacementPolicy { if (dCoolingOff) { reasons.add("is cooling off after repeated backend errors"); } + if (dAtCap) { + PlacementCandidate c = candidateFor(ctx, d); + int live = ctx.liveCount().apply(d); + reasons.add("is at maxLoad (" + live + " live >= " + c.maxLoad() + " cap)"); + } if (dModelOff) { reasons.add("names a model the operator has turned off in models.allow"); } @@ -96,18 +114,38 @@ final class FixedPlacementPolicy implements PlacementPolicy { } if (!ctx.candidates().isEmpty()) { throw new PlacementException("all worker profiles are excluded from automatic " - + "selection (quarantined, cooling off, model-off, unreachable, or weight-0)"); + + "selection (quarantined, cooling off, at cap, model-off, unreachable, or weight-0)"); } throw new PlacementException("no worker profiles configured"); } /** Whether {@code profile} carries {@code weight <= 0} (CB-554) among {@code ctx}'s candidates. */ private static boolean weightExcluded(PlacementContext ctx, String profile) { + PlacementCandidate c = candidateFor(ctx, profile); + return c != null && c.excluded(); + } + + /** + * Whether {@code profile} has reached its {@code maxLoad} cap (fleetd #435), using the shared + * {@link PlacementPolicyUtil#atCap} definition — the same one {@code weighted}/{@code + * round-robin} already consult via {@link PlacementPolicyUtil#available}. Looked up by name, + * the same way {@link #weightExcluded} is: the default fast path above builds its own {@link + * PlacementCandidate} with {@code maxLoad} forced to {@code null} (it carries no cap of its + * own), so the candidate actually configured for {@code profile} has to be found in {@code + * ctx.candidates()} first. + */ + private static boolean capExcluded(PlacementContext ctx, String profile) { + PlacementCandidate c = candidateFor(ctx, profile); + return c != null && PlacementPolicyUtil.atCap(ctx, c); + } + + /** The configured candidate named {@code profile} in {@code ctx}, or {@code null} if none. */ + private static PlacementCandidate candidateFor(PlacementContext ctx, String profile) { for (PlacementCandidate c : ctx.candidates()) { if (c.profile().equals(profile)) { - return c.excluded(); + return c; } } - return false; + return null; } } diff --git a/fleetd/src/main/java/dev/ltms/fleet/placement/PlacementPolicyUtil.java b/fleetd/src/main/java/dev/ltms/fleet/placement/PlacementPolicyUtil.java index d80a51c..9539be3 100644 --- a/fleetd/src/main/java/dev/ltms/fleet/placement/PlacementPolicyUtil.java +++ b/fleetd/src/main/java/dev/ltms/fleet/placement/PlacementPolicyUtil.java @@ -11,14 +11,29 @@ final class PlacementPolicyUtil { private PlacementPolicyUtil() { } + /** + * True when {@code c} has reached its {@code maxLoad} cap: {@code liveCount(c.profile()) >= + * c.maxLoad()}. A {@code null} maxLoad means unlimited, so it is never at cap. + * + *

Extracted as the single shared definition of "at cap" (fleetd #435): before this fix it + * was computed inline in both {@link #available} and {@link #emptyException}, and {@code + * FixedPlacementPolicy} — not a caller of either — quietly kept its own {@code select} free of + * any cap check at all, so a capped default profile was chosen anyway under the default + * placement policy. Every automatic policy must call this, not re-derive it. + */ + static boolean atCap(PlacementContext ctx, PlacementCandidate c) { + Integer cap = c.maxLoad(); + return cap != null && ctx.liveCount().apply(c.profile()) >= cap; + } + /** * Candidates that are not weight-excluded (CB-554: explicit {@code weight <= 0}, checked * first because it is a static config choice rather than transient state), not * known-unreachable, not quarantined (CB-578 stage B), not cooling off after repeated backend * errors (fleetd #201 Unit 5 — a separate, shorter-lived source from quarantine), not naming a * model the operator has turned off (fleetd #422 — a third, independent source: an operator - * decision, never a backend-reported outage), and have not reached their maxLoad. A {@code - * null} maxLoad means unlimited. + * decision, never a backend-reported outage), and have not reached their maxLoad (see {@link + * #atCap}). A {@code null} maxLoad means unlimited. */ static List available(PlacementContext ctx) { List out = new ArrayList<>(); @@ -26,16 +41,10 @@ final class PlacementPolicyUtil { if (c.excluded() || ctx.unreachable().contains(c.profile()) || ctx.quarantined().contains(c.profile()) || ctx.coolingOff().contains(c.profile()) - || ctx.modelOff().contains(c.profile())) { + || ctx.modelOff().contains(c.profile()) + || atCap(ctx, c)) { continue; } - Integer cap = c.maxLoad(); - if (cap != null) { - int live = ctx.liveCount().apply(c.profile()); - if (live >= cap) { - continue; - } - } out.add(c); } return out; @@ -59,7 +68,6 @@ final class PlacementPolicyUtil { int coolingOff = 0; int modelOff = 0; for (PlacementCandidate c : ctx.candidates()) { - Integer cap = c.maxLoad(); if (c.excluded()) { weightExcluded++; } else if (ctx.quarantined().contains(c.profile())) { @@ -70,7 +78,7 @@ final class PlacementPolicyUtil { modelOff++; } else if (ctx.unreachable().contains(c.profile())) { unreachable++; - } else if (cap != null && ctx.liveCount().apply(c.profile()) >= cap) { + } else if (atCap(ctx, c)) { atCap++; } } diff --git a/fleetd/src/test/java/dev/ltms/fleet/FleetdModelGateCoverageLineTest.java b/fleetd/src/test/java/dev/ltms/fleet/FleetdModelGateCoverageLineTest.java new file mode 100644 index 0000000..8894a0f --- /dev/null +++ b/fleetd/src/test/java/dev/ltms/fleet/FleetdModelGateCoverageLineTest.java @@ -0,0 +1,62 @@ +package dev.ltms.fleet; + +import dev.ltms.fleet.peer.PeerLauncher; +import org.junit.jupiter.api.DisplayName; +import org.junit.jupiter.api.Test; + +import java.util.Set; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNotEquals; + +/** + * fleetd #422 follow-up: {@code Fleetd.modelGateCoverageLine} is the startup-log counterpart of + * {@code exhaustedPatternCoverageLine}/{@code errorPatternCoverageLine} — see {@code + * FleetdPatternCoverageLineTest} for the identical shape this follows — except here there is no + * {@code UnsetMeaning} choice for a caller to get backwards: {@link + * PeerLauncher.ModelGateState#configured()} already states, unambiguously, whether an empty + * {@link PeerLauncher.ModelGateState#off()} means "no {@code models:} block to gate with at all" + * or "a block armed and currently reporting zero off". This class proves {@code + * modelGateCoverageLine} words those two states — plus the third, N off — distinctly, so a + * mutation that made it ignore {@code configured()} either way is caught here. + */ +class FleetdModelGateCoverageLineTest { + + @Test + @DisplayName("no models: block reports not configured, distinct from armed-with-zero") + void noModelsBlockReportsNotConfigured() { + String line = Fleetd.modelGateCoverageLine(PeerLauncher.ModelGateState.notConfigured()); + assertEquals("not configured (no models: block — nothing is gated, and nothing can be)", line); + } + + @Test + @DisplayName("a models: block armed with nothing off reports armed, distinct from not configured") + void armedWithNothingOffReportsArmed() { + String line = Fleetd.modelGateCoverageLine(PeerLauncher.ModelGateState.armed(Set.of())); + assertEquals("armed (models: block present; 0 models currently turned off)", line); + } + + @Test + @DisplayName("a models: block with N off names the off models") + void armedWithModelsOffNamesThem() { + String line = Fleetd.modelGateCoverageLine( + PeerLauncher.ModelGateState.armed(Set.of("deepseek-v4-flash", "claude-opus-9000"))); + assertEquals("armed (2 model(s) turned off: [claude-opus-9000, deepseek-v4-flash])", line); + } + + @Test + @DisplayName("the three states produce pairwise-distinct wording for the same empty-looking input") + void theThreeStatesProduceDistinctWording() { + String notConfigured = Fleetd.modelGateCoverageLine(PeerLauncher.ModelGateState.notConfigured()); + String armedZero = Fleetd.modelGateCoverageLine(PeerLauncher.ModelGateState.armed(Set.of())); + String armedOne = Fleetd.modelGateCoverageLine(PeerLauncher.ModelGateState.armed(Set.of("x"))); + + // Pinned individually above; restated here so this test alone still catches a regression + // even if one of the three tests above were ever deleted — the exact FleetdPatternCoverageLineTest + // pattern, adapted from "two keys" to "three states of one gate". + assertNotEquals(notConfigured, armedZero, + "collapsing 'no models: block' into 'armed, zero off' is fleetd #422 follow-up's exact defect"); + assertNotEquals(armedZero, armedOne); + assertNotEquals(notConfigured, armedOne); + } +} diff --git a/fleetd/src/test/java/dev/ltms/fleet/mcp/FleetProfilesModelGateStateTest.java b/fleetd/src/test/java/dev/ltms/fleet/mcp/FleetProfilesModelGateStateTest.java new file mode 100644 index 0000000..062365e --- /dev/null +++ b/fleetd/src/test/java/dev/ltms/fleet/mcp/FleetProfilesModelGateStateTest.java @@ -0,0 +1,109 @@ +package dev.ltms.fleet.mcp; + +import dev.ltms.fleet.config.FleetConfig; +import dev.ltms.fleet.guard.SubscriptionGuard; +import dev.ltms.fleet.herdr.AgentControl; +import dev.ltms.fleet.herdr.FakeHerdr; +import dev.ltms.fleet.herdr.WorkspaceControl; +import dev.ltms.fleet.member.ClaudeCodeLauncher; +import dev.ltms.fleet.member.CompositePeerLauncher; +import dev.ltms.fleet.member.HerdrPeerLauncher; +import dev.ltms.fleet.peer.PeerLauncher; +import dev.ltms.fleet.placement.BackendOutagePolicy; +import dev.ltms.fleet.placement.BackendQuarantine; +import dev.ltms.fleet.placement.PlacementPolicies; +import org.junit.jupiter.api.Test; + +import java.util.List; +import java.util.Map; +import java.util.Set; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; + +/** + * fleetd #422 follow-up: {@code fleet_profiles}/{@code GET /profiles} — the reporting surface a + * lead actually reads — must let it tell apart the three states {@link + * dev.ltms.fleet.peer.PeerLauncher#disabledModels()} alone collapses into one empty set: no + * {@code models:} block at all, a block armed with nothing currently off, and a block with N + * models off. See {@link dev.ltms.fleet.peer.PeerLauncher.ModelGateState}'s javadoc for why a + * bare {@code disabledModels()} read cannot make this distinction, and {@link + * FleetMcp#profilesView} for where {@code modelGateArmed} is added alongside the existing {@code + * modelsOff} key. + * + *

Every assertion here goes through {@link FleetMcp#profilesView}, never {@code + * PeerLauncher.modelGateState()} directly — {@code CompositePeerLauncherTest} already proves the + * accessor itself; this class proves the surface a lead reads (fleet_profiles / GET /profiles) + * renders what that accessor reports. + */ +class FleetProfilesModelGateStateTest { + + private static FleetConfig.Profile profile(String name, String model) { + return new FleetConfig.Profile(name, "http://gx00.gw:8000", model, null, "FLEETD_WORKER_TOKEN", + null, "tab", "fleetd-workers", "worker: {profile} #{n}", null, null, null); + } + + private static FleetMcp.QuarantineSource noQuarantine() { + return new FleetMcp.QuarantineSource(_ -> null, BackendQuarantine.none(), _ -> false); + } + + private static HerdrPeerLauncher claudeAdapter(FakeHerdr h, Map profiles) { + return new ClaudeCodeLauncher(new AgentControl(h), new WorkspaceControl(h), + new SubscriptionGuard(Set.of("gx00.gw")), profiles, "local", _ -> "tok"); + } + + /** + * State 1: no {@code models:} block at all — a plain {@code ClaudeCodeLauncher} (no {@code + * models:} supplier exists for it to read) has nothing to gate against, matching the fleet01 + * host measured for this ticket: {@code grep -c '^models:' fleetd.yaml} returns 0 there. + */ + @Test + void noModelsBlockReportsGateNotArmed() { + FakeHerdr h = new FakeHerdr(); + Map profiles = Map.of("local", profile("local", "deepseek-v4-flash")); + PeerLauncher workers = claudeAdapter(h, profiles); + + Map view = FleetMcp.profilesView(workers, noQuarantine(), FleetMcp.OutageSource.none()); + + assertEquals(Boolean.FALSE, view.get("modelGateArmed"), + "no models: block to read from — nothing is gated, and nothing can be"); + assertFalse(view.containsKey("modelsOff"), "nothing configured, so no off set to report either"); + } + + /** State 2: a {@code models:} block is present, but nothing in it is currently turned off. */ + @Test + void modelsBlockWithNothingOffReportsGateArmedAndZeroOff() { + FakeHerdr h = new FakeHerdr(); + Map profiles = Map.of("local", profile("local", "deepseek-v4-flash")); + FleetConfig.Models models = new FleetConfig.Models( + List.of(new FleetConfig.Models.ModelEntry("deepseek-v4-flash", true))); + PeerLauncher workers = new CompositePeerLauncher(List.of(claudeAdapter(h, profiles)), "local", profiles, + PlacementPolicies.fixed(), _ -> 0, null, BackendQuarantine.none(), + new BackendOutagePolicy(System::nanoTime), () -> models); + + Map view = FleetMcp.profilesView(workers, noQuarantine(), FleetMcp.OutageSource.none()); + + assertEquals(Boolean.TRUE, view.get("modelGateArmed"), + "a models: block is present, so the gate is armed even though nothing is off yet"); + assertFalse(view.containsKey("modelsOff"), + "nothing is off, so the key stays absent — an empty list here would be indistinguishable " + + "from today's modelsOff omission, exactly the ambiguity modelGateArmed exists to remove"); + } + + /** State 3: a {@code models:} block is present with one model currently turned off. */ + @Test + void modelsBlockWithModelsOffReportsGateArmedAndTheOffSet() { + FakeHerdr h = new FakeHerdr(); + Map profiles = Map.of("local", profile("local", "deepseek-v4-flash")); + FleetConfig.Models models = new FleetConfig.Models( + List.of(new FleetConfig.Models.ModelEntry("deepseek-v4-flash", false))); + PeerLauncher workers = new CompositePeerLauncher(List.of(claudeAdapter(h, profiles)), "local", profiles, + PlacementPolicies.fixed(), _ -> 0, null, BackendQuarantine.none(), + new BackendOutagePolicy(System::nanoTime), () -> models); + + Map view = FleetMcp.profilesView(workers, noQuarantine(), FleetMcp.OutageSource.none()); + + assertEquals(Boolean.TRUE, view.get("modelGateArmed")); + assertEquals(List.of("deepseek-v4-flash"), view.get("modelsOff")); + } +} diff --git a/fleetd/src/test/java/dev/ltms/fleet/member/CompositePeerLauncherTest.java b/fleetd/src/test/java/dev/ltms/fleet/member/CompositePeerLauncherTest.java index ad05a68..26fdd29 100644 --- a/fleetd/src/test/java/dev/ltms/fleet/member/CompositePeerLauncherTest.java +++ b/fleetd/src/test/java/dev/ltms/fleet/member/CompositePeerLauncherTest.java @@ -536,6 +536,31 @@ class CompositePeerLauncherTest { assertEquals("claude", h.profile(), "the returned handle carries the resolved default profile"); } + /** + * fleetd #435: {@code FixedPlacementPolicy} — the default placement policy every config uses + * unless {@code placement:} is set — never consulted {@code maxLoad}, so an unqualified spawn + * (a blank profile, the normal delegation path) landed on a capped default anyway. Measured on + * 7667727: a single dev profile at {@code maxLoad: 1} with 1 live, under {@code fixed()}, + * returned "SPAWNED on profile=a". This test goes through {@code CompositePeerLauncher.spawn} + * with a blank profile, not the policy in isolation, so it proves the caller actually reaches + * the fixed default's new cap check rather than only the {@code select} method. + */ + @Test + void fixedPolicyGatesDefaultProfileAtMaxLoadOnUnqualifiedSpawn() { + FakeHerdr herdr = new FakeHerdr(); + Map profiles = ordered( + "a", stubWorker("a", 1.0f, 1), + "b", stubWorker("b", 1.0f, null)); + StubLauncher adapter = new StubLauncher("claude", herdr, profiles, "a", Set.of()); + CompositePeerLauncher composite = new CompositePeerLauncher( + List.of(adapter), "a", profiles, PlacementPolicies.fixed(), name -> "a".equals(name) ? 1 : 0); + + PeerHandle h = composite.spawn(new SpawnRequest(null, null, null)); + assertEquals("b", h.profile(), + "the default profile a is at maxLoad, so fixed placement must fall through to b"); + assertEquals(0, adapter.spawnCount("a"), "a is never spawned — it is already at cap"); + } + @Test void weightedPolicyGatesProfileAtMaxLoad() { FakeHerdr herdr = new FakeHerdr(); @@ -1453,6 +1478,108 @@ class CompositePeerLauncherTest { assertEquals(2, adapter.spawnCount("local")); } + /** + * fleetd #422 follow-up, acceptance criterion 2: {@link CompositePeerLauncher#modelGateState()} + * is LIVE — no restart — proven through a REAL {@link ConfigRef#reload()}, exactly like {@link + * #modelOnOffIsHotReloadedThroughARealConfigRef} above proves for the on/off gate itself. This + * single reload sequence walks through all three states the ticket asks for: no {@code models:} + * block, a block armed with nothing off, and a block with one model off — so a reload that flips + * between any of the three is proven live, not just the on/off edit within an already-armed block. + */ + @Test + void modelGateStateIsHotReloadedThroughARealConfigRef(@TempDir Path dir) throws Exception { + Path yaml = dir.resolve("fleetd.yaml"); + Files.writeString(yaml, """ + bind: + port: 8080 + profiles: + local: + baseUrl: http://local.gw:8000 + model: deepseek-v4-flash + """); + FleetConfig initial = FleetConfig.load(yaml); + ConfigRef configRef = new ConfigRef(yaml, initial); + + FakeHerdr herdr = new FakeHerdr(); + StubLauncher adapter = new StubLauncher("claude", herdr, + Map.of("local", stubWorker("local")), "local", Set.of()); + CompositePeerLauncher composite = new CompositePeerLauncher( + List.of(adapter), "local", configRef, _ -> 0, BackendQuarantine.none(), NO_OUTAGE); + + PeerLauncher.ModelGateState notConfigured = composite.modelGateState(); + assertFalse(notConfigured.configured(), "no models: block in the config at all"); + assertEquals(Set.of(), notConfigured.off()); + + Files.writeString(yaml, """ + bind: + port: 8080 + profiles: + local: + baseUrl: http://local.gw:8000 + model: deepseek-v4-flash + models: + allow: + - model: deepseek-v4-flash + enabled: false + """); + assertTrue(configRef.reload().applied(), "adding a models: block must apply live, no restart"); + PeerLauncher.ModelGateState armedWithOneOff = composite.modelGateState(); + assertTrue(armedWithOneOff.configured(), "a models: block now exists — the gate is armed"); + assertEquals(Set.of("deepseek-v4-flash"), armedWithOneOff.off()); + + Files.writeString(yaml, """ + bind: + port: 8080 + profiles: + local: + baseUrl: http://local.gw:8000 + model: deepseek-v4-flash + models: + allow: + - model: deepseek-v4-flash + enabled: true + """); + assertTrue(configRef.reload().applied(), "flipping the entry back on must apply live too"); + PeerLauncher.ModelGateState armedWithZeroOff = composite.modelGateState(); + assertTrue(armedWithZeroOff.configured(), + "the block is still present — armed and reporting zero, not the same as no block at all"); + assertEquals(Set.of(), armedWithZeroOff.off()); + } + + /** + * fleetd #422 follow-up, acceptance criterion 3: the invariant is that an absent {@code + * models:} block stays permitted and must never be fatal. Proved, not assumed — a config + * without one loads, validates, reports the gate as not configured, AND still spawns normally + * (no {@link PlacementException} from a gate that has nothing to check against), using the same + * production-shaped {@code Supplier} wiring {@code Fleetd.main} actually uses. + */ + @Test + void noModelsBlockConfigStillLoadsAndSpawnsNormally(@TempDir Path dir) throws Exception { + Path yaml = dir.resolve("fleetd.yaml"); + Files.writeString(yaml, """ + bind: + port: 8080 + profiles: + local: + baseUrl: http://local.gw:8000 + model: deepseek-v4-flash + """); + FleetConfig cfg = FleetConfig.load(yaml); + assertDoesNotThrow(cfg::validateAll, "a config with no models: block must load and validate cleanly"); + ConfigRef configRef = new ConfigRef(yaml, cfg); + + FakeHerdr herdr = new FakeHerdr(); + StubLauncher adapter = new StubLauncher("claude", herdr, + Map.of("local", stubWorker("local")), "local", Set.of()); + CompositePeerLauncher composite = new CompositePeerLauncher( + List.of(adapter), "local", configRef, _ -> 0, BackendQuarantine.none(), NO_OUTAGE); + + assertFalse(composite.modelGateState().configured()); + assertDoesNotThrow(() -> composite.spawn(new SpawnRequest("local", null, null)), + "no models: block means nothing to gate against — the spawn must go through"); + assertEquals(1, adapter.spawnCount("local")); + } + /** {@code fleet_profiles}/{@code GET /profiles} must read the exact same live source the gate reads. */ @Test void disabledModelsReportsWhatTheGateActuallyEnforces() { diff --git a/fleetd/src/test/java/dev/ltms/fleet/placement/PlacementPolicyTest.java b/fleetd/src/test/java/dev/ltms/fleet/placement/PlacementPolicyTest.java index 8f0f73b..16fcfd3 100644 --- a/fleetd/src/test/java/dev/ltms/fleet/placement/PlacementPolicyTest.java +++ b/fleetd/src/test/java/dev/ltms/fleet/placement/PlacementPolicyTest.java @@ -461,6 +461,93 @@ class PlacementPolicyTest { assertTrue(e.getMessage().contains("weight 0"), e.getMessage()); } + // --- fleetd #435: FixedPlacementPolicy must consult maxLoad too, at BOTH filter sites ------- + + /** + * The default-profile fast path must skip a capped default. Measured on 7667727 before this + * fix: a single dev profile at {@code maxLoad: 1} with 1 live, under {@code fixed()}, still + * returned "SPAWNED on profile=a" — the cap was advisory for every unqualified spawn. + */ + @Test + void fixedSkipsCappedDefault() { + PlacementPolicy policy = PlacementPolicies.fixed(); + PlacementContext ctx = new PlacementContext("b", + List.of(PlacementCandidate.profile("a", 1.0f, null), + PlacementCandidate.profile("b", 1.0f, 1)), + name -> "b".equals(name) ? 1 : 0, Set.of(), Set.of(), Set.of()); + assertEquals("a", policy.select(ctx).profile(), + "the default 'b' is at its maxLoad cap, so fixed falls through to the free candidate 'a'"); + } + + /** + * The fallback walk must skip a capped candidate too — exercised independently of the + * default-profile fast path by using no default at all, so this is the only filter that runs. + */ + @Test + void fixedFallbackWalkSkipsCappedCandidate() { + PlacementPolicy policy = PlacementPolicies.fixed(); + PlacementContext ctx = new PlacementContext(null, + List.of(PlacementCandidate.profile("a", 1.0f, 1), + PlacementCandidate.profile("b", 1.0f, null)), + name -> "a".equals(name) ? 1 : 0, Set.of(), Set.of(), Set.of()); + assertEquals("b", policy.select(ctx).profile(), + "candidate 'a' is at its maxLoad cap, so the fallback walk skips it and picks 'b'"); + } + + @Test + void fixedThrowsWhenDefaultAndEveryCandidateAtCap() { + PlacementPolicy policy = PlacementPolicies.fixed(); + PlacementContext ctx = new PlacementContext("b", + List.of(PlacementCandidate.profile("a", 1.0f, 1), + PlacementCandidate.profile("b", 1.0f, 1)), + _ -> 1, Set.of(), Set.of(), Set.of()); + PlacementException e = assertThrows(PlacementException.class, () -> policy.select(ctx)); + assertTrue(e.getMessage().contains("maxLoad"), "message names the cap: " + e.getMessage()); + assertTrue(e.getMessage().contains("1 cap"), "message names the cap value: " + e.getMessage()); + assertFalse(e.getMessage().contains("quarantined"), "must not read like quarantine: " + e.getMessage()); + assertFalse(e.getMessage().contains("turned off"), "must not read like model-off: " + e.getMessage()); + } + + /** The mirror: an uncapped default is still chosen, so the new term cannot exclude everything. */ + @Test + void fixedStillReturnsUncappedDefault() { + PlacementPolicy policy = PlacementPolicies.fixed(); + PlacementContext ctx = new PlacementContext("b", + List.of(PlacementCandidate.profile("a", 1.0f, null), + PlacementCandidate.profile("b", 1.0f, null)), + noSessions(), Set.of(), Set.of(), Set.of()); + assertEquals("b", policy.select(ctx).profile(), "an uncapped default is returned unconditionally"); + } + + /** CB-585: an explicit {@code maxLoad: 0} on the default caps it at zero live members. */ + @Test + void fixedSkipsMaxLoadZeroDefaultEvenWithZeroLiveWorkers() { + PlacementPolicy policy = PlacementPolicies.fixed(); + PlacementContext ctx = new PlacementContext("b", + List.of(PlacementCandidate.profile("a", 1.0f, null), + PlacementCandidate.profile("b", 1.0f, 0)), + _ -> 0, Set.of(), Set.of(), Set.of()); + assertEquals("a", policy.select(ctx).profile(), + "the default 'b' has maxLoad 0, so it is already at its cap with nobody live"); + } + + /** + * Quarantine still wins when a profile is both quarantined and at cap (mirrors {@code + * fixedReportsQuarantineNotModelOffWhenBothApply}'s priority over model-off). + */ + @Test + void fixedReportsQuarantineNotAtCapWhenBothApply() { + PlacementPolicy policy = PlacementPolicies.fixed(); + PlacementContext ctx = new PlacementContext("b", + List.of(PlacementCandidate.profile("a", 1.0f, 1), + PlacementCandidate.profile("b", 1.0f, 1)), + _ -> 1, Set.of(), Set.of("a", "b"), Set.of()); + PlacementException e = assertThrows(PlacementException.class, () -> policy.select(ctx)); + assertTrue(e.getMessage().contains("quarantined"), e.getMessage()); + assertFalse(e.getMessage().contains("maxLoad"), + "quarantine takes priority over at-cap in the message: " + e.getMessage()); + } + @Test void unknownPolicyNameThrows() { assertThrows(IllegalArgumentException.class, () -> PlacementPolicies.fromName("random"));