diff --git a/fleetd/src/main/java/dev/ltms/fleet/config/ConfigRef.java b/fleetd/src/main/java/dev/ltms/fleet/config/ConfigRef.java index 5ee4dcd..52d8e15 100644 --- a/fleetd/src/main/java/dev/ltms/fleet/config/ConfigRef.java +++ b/fleetd/src/main/java/dev/ltms/fleet/config/ConfigRef.java @@ -34,7 +34,16 @@ import java.util.function.Supplier; * {@code tabLabel} — is read the same live way, through the same supplier * ({@code () -> config.get().fleet()}). But {@code fleet:} as a whole is NOT in this * class: {@code fleet.leaders} inside the same key is frozen, which is exactly what - * makes {@code fleet:} split rather than hot — see below. + * makes {@code fleet:} split rather than hot — see below. {@code models:} (fleetd #422) joined + * this class whole: {@link FleetConfig#validateModels()} re-runs fully against the fresh + * config on every {@link #reload()} (via {@link FleetConfig#validateAll()}), refusing a bad + * edit outright rather than caching a stale copy anywhere, and the on/off half added by + * fleetd #422 is read live both by {@code CompositePeerLauncher}'s spawn gate + * ({@code enforceModelEnabled} and its candidate filter) and by {@code fleet_profiles}/ + * {@code GET /profiles} (via {@code PeerLauncher.disabledModels()}). Nothing about + * {@code models:} is baked into an object built at startup, so — unlike the deferred keys + * below — there is no frozen half left to report; it moved here from deferred rather than + * joining split. *
  • Deferred — accepted into the new snapshot, but the wiring built at startup * keeps the old value until a restart: {@code lifecycle:}, {@code leadHeartbeat:}, * {@code idleSleepGuard:} ({@code Fleetd.java} reads it once, at startup, to decide whether @@ -43,12 +52,6 @@ import java.util.function.Supplier; * running daemon keeps whatever this was at startup regardless of a later edit), * {@code spawnReadyTimeoutMs} / {@code spawnReadyPollMs}, {@code quarantineCooldownSeconds} * (CB-578 stage B — baked once into the {@code BackendQuarantine} built at startup), - * {@code models:} (fleetd ticket "central allow-list of usable models" — {@link - * FleetConfig#validateModels()} re-runs against the fresh config in {@link #reload()} - * (via {@link FleetConfig#validateAll()}), so a - * models.allow: edit that would refuse to boot still refuses the reload; a change that - * passes has nothing built at startup to rebuild, so it is reported deferred rather than - * silently accepted with no report at all), * {@code guard:}, {@code worktreeRoot:}, {@code worktreeGroup:} and {@code memberSkills:} * (all three of the latter baked once into the {@code GitWorktrees} built at * {@code Fleetd.java:251} and never rebuilt — fleetd #323 instance 2 found @@ -141,16 +144,18 @@ import java.util.function.Supplier; * * *

    The denominator, measured on 2026-09-04 (fleetd #330; recounted for fleetd #333); - * recounted again for fleetd #362, again after {@code idleSleepGuard:} was added, and again after - * {@code models:} was added. - * {@code FleetConfig} has 25 top-level record components: 5 cold, 14 deferred, 3 split, 3 - * hot-excluded. Three of them are named nowhere in this file, and the reason is the same for all - * three: {@code placement}, {@code memberCredentials} and {@code memberLoginShell} are - * hot and correctly absent — all three are read live off {@code config.get()} + * recounted again for fleetd #362, again after {@code idleSleepGuard:} was added, again after + * {@code models:} was added as deferred, and again for fleetd #422, which moved {@code models:} + * from deferred to hot-excluded once its on/off half was read live everywhere. + * {@code FleetConfig} has 25 top-level record components: 5 cold, 13 deferred, 3 split, 4 + * hot-excluded. Four of them are named nowhere in this file, and the reason is the same for all + * four: {@code placement}, {@code memberCredentials}, {@code memberLoginShell} and {@code models} + * are hot and correctly absent — all four are read live off {@code config.get()} * (placement through the {@code CompositePeerLauncher} supplier the Hot bullet names; * {@code memberCredentials}/{@code memberLoginShell} at spawn time, {@code Fleetd.java:198, 205, 729} - * and {@code HerdrPeerLauncher#configuredMemberLoginShell}), so a reload takes effect on the next - * spawn with no entry needed here. + * and {@code HerdrPeerLauncher#configuredMemberLoginShell}; {@code models} the same way, through the + * Hot bullet's {@code models:} paragraph), so a reload takes effect on the next spawn (or, for + * {@code models}, the next reported status) with no entry needed here. * {@code health} and {@code coordinator} used to be a third kind — undecided, not * hot — until fleetd #330 added the split class above and gave them a home. A * reload touching either used to report a bare "config reloaded", which under-claimed; now it names @@ -226,7 +231,7 @@ public final class ConfigRef implements Supplier { static final Set DEFERRED_KEYS = Set.of( "guard", "worktreeRoot", "worktreeGroup", "memberSkills", "primary", "configReload", "leadHeartbeat", "lifecycle", "spawnReadyTimeoutMs", "spawnReadyPollMs", - "quarantineCooldownSeconds", "profiles", "idleSleepGuard", "models"); + "quarantineCooldownSeconds", "profiles", "idleSleepGuard"); private final Path path; private final AtomicReference current; @@ -446,15 +451,6 @@ public final class ConfigRef implements Supplier { if (!Objects.equals(old.idleSleepGuard(), fresh.idleSleepGuard())) { changed.add("idleSleepGuard"); } - // fleetd ticket "central allow-list of usable models": validateModels() runs again in - // reload() above (via validateAll()), so a bad edit is already refused as cold-adjacent - // (the whole reload is refused via the catch block, never partially applied). A GOOD edit - // to the allow-list - // itself has nothing built at startup to rebuild — it only ever mattered to the validation - // call that already ran — so report it deferred rather than silently swallowing the change. - if (!Objects.equals(old.models(), fresh.models())) { - changed.add("models"); - } if (!Objects.equals(old.spawnReadyTimeoutMs(), fresh.spawnReadyTimeoutMs()) || !Objects.equals(old.spawnReadyPollMs(), fresh.spawnReadyPollMs())) { changed.add("spawnReady*"); diff --git a/fleetd/src/main/java/dev/ltms/fleet/config/FleetConfig.java b/fleetd/src/main/java/dev/ltms/fleet/config/FleetConfig.java index 9ec3452..68660de 100644 --- a/fleetd/src/main/java/dev/ltms/fleet/config/FleetConfig.java +++ b/fleetd/src/main/java/dev/ltms/fleet/config/FleetConfig.java @@ -133,8 +133,10 @@ import java.util.regex.PatternSyntaxException; * nothing and every existing config keeps working exactly as it does today. * When non-empty, a profile whose {@code model:} is not one of {@link * Models#ids()} fails config load, naming both the model and the profile — - * see {@link #validateModels()}. This block only decides what may be - * CONFIGURED; nothing here enforces it at spawn time. See {@link Models}. + * see {@link #validateModels()}. This block decides what may be CONFIGURED; + * fleetd #422 added the separate on/off question — whether a configured model + * may be spawned onto RIGHT NOW ({@link Models.ModelEntry#enabled}) — enforced + * live at spawn by {@code CompositePeerLauncher}, not here. See {@link Models}. */ @JsonIgnoreProperties(ignoreUnknown = true) public record FleetConfig( @@ -1362,9 +1364,14 @@ public record FleetConfig( * the set of permitted models — only editing {@code models.allow:} itself can. This is the * invariant the ticket asked for: the two blocks are validated in one direction only. * - *

    Out of scope here, deliberately: nothing in this block is read at spawn time — - * enforcing it against a live spawn, an on/off runtime switch, and any interaction with {@code - * BackendQuarantine} are separate units. This block is config-load validation only. + *

    Spawn-time enforcement (fleetd #422, units 2+3) lives outside this record — + * {@code CompositePeerLauncher.enforceModelEnabled} and its candidate-set filter read this + * block LIVE (through the same kind of supplier {@code weight}/{@code maxLoad} already use), + * so the on/off state below is hot: no restart needed. This block itself still only decides + * what may be CONFIGURED (membership in {@link #allow}); {@link ModelEntry#enabled} decides + * whether a member of that list is currently spawnable. The two questions are deliberately + * separate — see {@link ModelEntry}'s javadoc for why turning a model off must never mean + * removing it from {@link #allow}. * * @param allow the permitted models, each its own {@link ModelEntry} rather than a bare * string — see that record's javadoc for why. {@code null}/empty ⇒ the block is @@ -1378,24 +1385,56 @@ public record FleetConfig( } /** - * One permitted model, named as a record rather than a bare string on purpose: a later unit - * needs to hang an on/off state and a load-limit state off each entry, and a bare {@code - * List} cannot grow those fields without changing the YAML shape underneath every - * operator who already wrote one. {@link #model()} is intentionally a single flat, - * opaque-string namespace — a bare Claude id ({@code claude-sonnet-5}) and an opencode - * provider-prefixed id ({@code openai/gpt-5.6-terra}) both fit it unchanged, because - * {@link FleetConfig#validateModels()} only ever compares a profile's {@code model:} value - * against this string for exact equality; it never parses a provider prefix or branches on - * a profile's {@code kind:}. + * One permitted model, named as a record rather than a bare string on purpose: this ticket + * (fleetd #422) is the "later unit" the original comment here predicted — it hangs an on/off + * state ({@link #enabled}) off each entry, and a bare {@code List} could not have + * grown that field without changing the YAML shape underneath every operator who already + * wrote one. {@link #model()} is intentionally a single flat, opaque-string namespace — a + * bare Claude id ({@code claude-sonnet-5}) and an opencode provider-prefixed id + * ({@code openai/gpt-5.6-terra}) both fit it unchanged, because {@link + * FleetConfig#validateModels()} only ever compares a profile's {@code model:} value against + * this string for exact equality; it never parses a provider prefix or branches on a + * profile's {@code kind:}. * - * @param model the model id exactly as a {@code profiles:} entry's {@code model:} field - * would name it + * @param model the model id exactly as a {@code profiles:} entry's {@code model:} field + * would name it + * @param enabled {@code false} turns spawning onto this model off; {@code null} (the field + * omitted — every config written before fleetd #422 is this shape) or + * {@code true} leaves it on. Turning a model off must NEVER remove it from + * {@link Models#allow} — {@link FleetConfig#validateModels()} checks + * membership only, never the on/off state, so an off entry stays a + * valid thing for a {@code profiles:} entry to name; only + * {@code CompositePeerLauncher}'s spawn-time gate reads {@link #enabled}. + * Collapsing the two — turning a model off by deleting its {@code allow:} + * entry — would make {@link FleetConfig#validateModels()} refuse the whole + * config reload the moment a still-configured profile names it, which is + * exactly the restart-to-flip-a-switch problem this field exists to avoid. + *

    A model id can be named by more than one {@code profiles:} entry (e.g. + * {@code deepseek-v4-flash} backs both {@code local} and {@code + * local-direct} in the live config) — turning it off disables every profile + * that names it, on purpose: the model is what a subscription's rate limit + * actually constrains, not any one profile alias for it. */ @JsonIgnoreProperties(ignoreUnknown = true) - public record ModelEntry(String model) { + public record ModelEntry(String model, Boolean enabled) { public ModelEntry { model = (model == null || model.isBlank()) ? null : model.trim(); } + + /** + * Back-compat form before {@link #enabled} was added (fleetd #422) — the model is + * unconditionally on, exactly as every {@code ModelEntry} behaved before this field + * existed. Keeps pre-#422 call sites (and any YAML that omits {@code enabled:}) + * compiling and behaving identically. + */ + public ModelEntry(String model) { + this(model, null); + } + + /** {@code true} unless {@link #enabled} is explicitly {@code false} — absent means on. */ + public boolean isEnabled() { + return !Boolean.FALSE.equals(enabled); + } } /** {@link #allow}'s model ids, as a set for membership checks. Blank/null entries are dropped. */ @@ -1408,6 +1447,23 @@ public record FleetConfig( } return Collections.unmodifiableSet(ids); } + + /** + * Model ids currently turned off (fleetd #422: {@link ModelEntry#isEnabled()} {@code + * false}). Read live by {@code CompositePeerLauncher}'s spawn gate and by {@code + * fleet_profiles}/{@code GET /profiles} — both must read this same accessor off the same + * live config so the two surfaces cannot disagree about which model is off (the fleetd + * #404 lesson: a status field must read the source the behaviour reads). + */ + public Set offIds() { + Set off = new java.util.LinkedHashSet<>(); + for (ModelEntry e : allow) { + if (e != null && e.model() != null && !e.isEnabled()) { + off.add(e.model()); + } + } + return Collections.unmodifiableSet(off); + } } /** 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 69943ac..792305a 100644 --- a/fleetd/src/main/java/dev/ltms/fleet/mcp/FleetMcp.java +++ b/fleetd/src/main/java/dev/ltms/fleet/mcp/FleetMcp.java @@ -37,6 +37,7 @@ import io.modelcontextprotocol.spec.McpSchema; import com.fasterxml.jackson.databind.ObjectMapper; import jakarta.servlet.http.HttpServlet; +import java.util.ArrayList; import java.util.LinkedHashMap; import java.util.List; import java.util.Map; @@ -1179,6 +1180,14 @@ public final class FleetMcp { if (!coolingOff.isEmpty()) { 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 + // 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)); + } 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 f4eced5..49c64fe 100644 --- a/fleetd/src/main/java/dev/ltms/fleet/member/CompositePeerLauncher.java +++ b/fleetd/src/main/java/dev/ltms/fleet/member/CompositePeerLauncher.java @@ -90,6 +90,19 @@ public final class CompositePeerLauncher implements PeerLauncher { private final Supplier> profileConfigs; private final Supplier placementPolicy; + /** + * fleetd #422: the central model allow-list's on/off state, read live per spawn — same reason + * {@link #profileConfigs} is a supplier rather than a captured map (see the class doc above and + * {@link #enforceModelEnabled}). A caller with no {@code models:} block to read from (the + * simpler, map-based constructors used throughout this class's own tests) wires this to a + * constant {@code null}, which {@link #models0} treats as "nothing configured, gate never + * fires" — the pre-#422 behaviour. + */ + private final Supplier models; + + /** The value {@link #models0} normalizes a {@code null} supplier result to. */ + private static final FleetConfig.Models NO_MODELS_CONFIGURED = new FleetConfig.Models(List.of()); + /** CB-578 stage B: credential cooldown, checked before an explicit spawn and filtered into placement. */ private final BackendQuarantine quarantine; @@ -205,12 +218,44 @@ public final class CompositePeerLauncher implements PeerLauncher { FleetConfig.Fleet fleet, BackendQuarantine quarantine, BackendOutagePolicy outagePolicy) { + // No models: block to read from a plain profiles Map — fleetd #422's gate is wired to a + // constant null (see enforceModelEnabled/models0), the pre-#422 behaviour for every caller + // of this overload. Use the 9-arg overload below to test the gate against a LIVE supplier. + this(delegates, defaultProfile, profileConfigs, placementPolicy, liveCount, fleet, quarantine, + outagePolicy, constant(null)); + } + + /** + * As above, plus a LIVE model-gate source (fleetd #422). The 8-arg overload above wires + * {@code models} to a constant {@code null} because it has only a static {@code Map}, never a full {@code FleetConfig}, to read one from; this overload exists so a test + * can prove {@link #enforceModelEnabled} and its candidate filter re-read {@link + * FleetConfig.Models} on every call rather than a value captured once at construction — the + * exact distinction fleetd #422 exists to get right (see the class doc's CB-559 note on {@link + * #profileConfigs}, which this follows). Production wiring uses the + * {@code Supplier} constructor below instead, which already threads a live + * {@code config.get().models()} through. + * + * @param models required — pass a supplier returning {@code null} for a caller that has no + * {@code models:} block to gate against, never a defaulting overload (the same + * "explicit opt-out, never a silent default" rule {@code quarantine}/ + * {@code outagePolicy} already follow). + */ + public CompositePeerLauncher(List delegates, + String defaultProfile, + Map profileConfigs, + PlacementPolicy placementPolicy, + Function liveCount, + FleetConfig.Fleet fleet, + BackendQuarantine quarantine, + BackendOutagePolicy outagePolicy, + Supplier models) { // LinkedHashMap, not Map.copyOf: candidates() promises definition order and the weighted // policy breaks exact-weight ties on it, so a salted iteration order would make placement // differ from one JVM run to the next. this(delegates, defaultProfile, constant(Collections.unmodifiableMap(new LinkedHashMap<>(profileConfigs))), - constant(placementPolicy), liveCount, constant(fleet), quarantine, outagePolicy); + constant(placementPolicy), liveCount, constant(fleet), quarantine, outagePolicy, models); } /** @@ -250,7 +295,10 @@ public final class CompositePeerLauncher implements PeerLauncher { liveCount, () -> config.get().fleet(), quarantine, - outagePolicy); + outagePolicy, + // fleetd #422: read live, same as profiles/placement/fleet above — a models.allow + // edit (on/off or otherwise) is visible to the very next spawn, no restart needed. + () -> config.get().models()); } /** The all-suppliers form every other constructor funnels into. */ @@ -261,7 +309,8 @@ public final class CompositePeerLauncher implements PeerLauncher { Function liveCount, Supplier fleet, BackendQuarantine quarantine, - BackendOutagePolicy outagePolicy) { + BackendOutagePolicy outagePolicy, + Supplier models) { this.fleet = fleet; this.quarantine = Objects.requireNonNull(quarantine, "quarantine"); this.outagePolicy = Objects.requireNonNull(outagePolicy, "outagePolicy"); @@ -273,6 +322,7 @@ public final class CompositePeerLauncher implements PeerLauncher { this.profileConfigs = profileConfigs; this.placementPolicy = placementPolicy; this.liveCount = liveCount; + this.models = Objects.requireNonNull(models, "models"); Map index = new LinkedHashMap<>(); for (HerdrPeerLauncher d : this.delegates) { for (String profile : d.profiles()) { @@ -303,6 +353,16 @@ public final class CompositePeerLauncher implements PeerLauncher { return m == null ? Map.of() : m; } + /** + * The currently-configured {@code models:} block, never null (fleetd #422). Read fresh on + * every call, the same reason {@link #profiles0} is — a config reload's on/off edit must reach + * the very next spawn. + */ + private FleetConfig.Models models0() { + FleetConfig.Models m = models.get(); + return m == null ? NO_MODELS_CONFIGURED : m; + } + /** The adapter owning {@code profileName} (null/blank → the default). Throws on an unknown profile. */ private HerdrPeerLauncher route(String profileName) { String resolved = (profileName == null || profileName.isBlank()) ? defaultProfile : profileName; @@ -332,6 +392,7 @@ public final class CompositePeerLauncher implements PeerLauncher { enforceNotQuarantined(requestedProfile); enforceNotCoolingOff(requestedProfile); enforceMaxLoad(requestedProfile); + enforceModelEnabled(requestedProfile); PeerHandle handle = d.spawn(req); spawnedBy.put(handle.id(), d); return handle; @@ -349,8 +410,11 @@ public final class CompositePeerLauncher implements PeerLauncher { Set quarantined = quarantinedProfiles(candidates); // fleetd #201 Unit 5: a distinct set from quarantined — see PlacementContext.coolingOff. Set coolingOff = coolingOffProfiles(candidates); + // fleetd #422: read live per spawn, same as quarantined/coolingOff above — a config reload + // that flips a model's enabled state is visible to the very next unqualified spawn. + Set modelOff = modelOffProfiles(candidates); PlacementContext ctx = new PlacementContext(roleDefault, candidates, liveCount, unreachable, - quarantined, coolingOff); + quarantined, coolingOff, modelOff); int maxAttempts = candidates.isEmpty() ? 1 : candidates.size(); for (int attempt = 0; attempt < maxAttempts; attempt++) { @@ -380,7 +444,7 @@ public final class CompositePeerLauncher implements PeerLauncher { unreachable.add(chosen.profile()); // Update the context for the next selection so the policy excludes this profile. ctx = new PlacementContext(roleDefault, candidates, liveCount, unreachable, - quarantined, coolingOff); + quarantined, coolingOff, modelOff); } } @@ -492,6 +556,49 @@ public final class CompositePeerLauncher implements PeerLauncher { } } + /** + * Refuse an explicit-profile spawn whose {@code model:} the operator has turned off in the + * central {@code models.allow:} list (fleetd #422). Deliberately worded apart from {@link + * #enforceNotQuarantined} and {@link #enforceNotCoolingOff}: those two report a BACKEND-reported + * outage (exhaustion, repeated errors); this one reports an OPERATOR decision, so the message + * says "turned off" and names the model, never "quarantined" or "cooling off". A FOURTH, + * independent reason to refuse a spawn — never layered onto {@code BackendQuarantine} or {@code + * BackendOutagePolicy}, which would misattribute an operator's own choice to the backend. + * + *

    Reads {@link #models0()} fresh on every call — the same liveness {@link #profiles0()} + * already has — so flipping {@code enabled: false} and reloading takes effect on the very next + * spawn, no restart (criterion 4). A profile naming no model, or a model absent from {@code + * models.allow:} entirely (nothing to gate against), is never refused here. + * + * @throws PlacementException naming the model and the profile, distinct from quarantine/cool-off + */ + private void enforceModelEnabled(String profile) { + FleetConfig.Profile cfg = profiles0().get(profile); + String model = (cfg == null) ? null : cfg.model(); + if (model == null) { + return; + } + if (models0().offIds().contains(model)) { + throw new PlacementException("worker profile '" + profile + "' names model '" + model + + "', which the operator has turned off in models.allow — refusing spawn"); + } + } + + /** The subset of {@code candidates} whose {@code model:} is currently turned off (fleetd #422). */ + private Set modelOffProfiles(List candidates) { + Set off = models0().offIds(); + if (off.isEmpty()) { + return Set.of(); + } + return candidates.stream() + .map(PlacementCandidate::profile) + .filter(p -> { + FleetConfig.Profile cfg = profiles0().get(p); + return cfg != null && cfg.model() != null && off.contains(cfg.model()); + }) + .collect(Collectors.toSet()); + } + /** * The profile names {@code role} may be placed on, in definition order. * @@ -536,6 +643,17 @@ public final class CompositePeerLauncher implements PeerLauncher { return route(profileName).parityOverlay(profileName); } + /** + * fleetd #422: read live off {@link #models0()} — the exact same accessor {@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). + */ + @Override + public Set disabledModels() { + return models0().offIds(); + } + @Override public void stop(String id) { HerdrPeerLauncher d = spawnedBy.get(id); 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 a80184c..2024a3e 100644 --- a/fleetd/src/main/java/dev/ltms/fleet/peer/PeerLauncher.java +++ b/fleetd/src/main/java/dev/ltms/fleet/peer/PeerLauncher.java @@ -192,4 +192,18 @@ public interface PeerLauncher { * @return {@code true} when a reset was sent and its status transition must settle before reuse */ boolean clearContext(String id); + + /** + * Model ids the operator has currently turned off in the central {@code models.allow:} list + * (fleetd #422) — empty for a launcher with nothing to gate against. {@code fleet_profiles}/ + * {@code GET /profiles} (via {@code FleetMcp.profilesView}) call this to report which models + * are off, and MUST read this exact accessor rather than deriving their own answer: the fleetd + * #404 lesson is that a status field reading a different source than the behaviour it describes + * 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. + */ + default Set disabledModels() { + return Set.of(); + } } 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 1389eef..4a0ca61 100644 --- a/fleetd/src/main/java/dev/ltms/fleet/placement/FixedPlacementPolicy.java +++ b/fleetd/src/main/java/dev/ltms/fleet/placement/FixedPlacementPolicy.java @@ -12,7 +12,7 @@ import java.util.List; * 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. * - *

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

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

    - * A fleet where nothing is ever quarantined, cooling 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, 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 { @@ -42,12 +51,13 @@ 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.unreachable().contains(d) && !weightExcluded(ctx, d)) { + && !ctx.modelOff().contains(d) && !ctx.unreachable().contains(d) && !weightExcluded(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.unreachable().contains(c.profile()) && !c.excluded()) { + && !ctx.modelOff().contains(c.profile()) && !ctx.unreachable().contains(c.profile()) + && !c.excluded()) { return new PlacementCandidate(c.profile(), null, c.weight(), c.maxLoad()); } } @@ -56,9 +66,14 @@ 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 + // CompositePeerLauncher's explicit-spawn check order (quarantine, cooling off, max load, + // then model-off). + boolean dModelOff = !dQuarantined && !dCoolingOff && ctx.modelOff().contains(d); boolean dUnreachable = ctx.unreachable().contains(d); boolean dWeightExcluded = weightExcluded(ctx, d); - if (dQuarantined || dCoolingOff || dUnreachable || dWeightExcluded) { + if (dQuarantined || dCoolingOff || dModelOff || dUnreachable || dWeightExcluded) { List reasons = new ArrayList<>(); if (dQuarantined) { reasons.add("is quarantined (backend exhausted)"); @@ -66,6 +81,9 @@ final class FixedPlacementPolicy implements PlacementPolicy { if (dCoolingOff) { reasons.add("is cooling off after repeated backend errors"); } + if (dModelOff) { + reasons.add("names a model the operator has turned off in models.allow"); + } if (dUnreachable) { reasons.add("is unreachable"); } @@ -78,7 +96,7 @@ final class FixedPlacementPolicy implements PlacementPolicy { } if (!ctx.candidates().isEmpty()) { throw new PlacementException("all worker profiles are excluded from automatic " - + "selection (quarantined, cooling off, unreachable, or weight-0)"); + + "selection (quarantined, cooling off, model-off, unreachable, or weight-0)"); } throw new PlacementException("no worker profiles configured"); } diff --git a/fleetd/src/main/java/dev/ltms/fleet/placement/PlacementContext.java b/fleetd/src/main/java/dev/ltms/fleet/placement/PlacementContext.java index 0665c75..5eeff8f 100644 --- a/fleetd/src/main/java/dev/ltms/fleet/placement/PlacementContext.java +++ b/fleetd/src/main/java/dev/ltms/fleet/placement/PlacementContext.java @@ -22,11 +22,29 @@ import java.util.function.Function; * the two apart so its refusal message says "cooling off", not "exhausted", * when only this one is active. A profile can be in both sets at once; when it * is, exhaustion quarantine is reported (it takes priority). + * @param modelOff profiles whose {@code model:} is currently turned off in {@code + * models.allow:} (fleetd #422) — an operator decision, not a backend-reported + * outage, so a SEPARATE, independent source from both {@code quarantined} and + * {@code coolingOff}. A profile can be in this set together with either (or + * both) of the others; {@link PlacementPolicyUtil} counts it into its own + * bucket rather than merging it into theirs, the same reason + * {@code coolingOff} is kept apart from {@code quarantined}. */ public record PlacementContext(String defaultProfile, List candidates, Function liveCount, Set unreachable, Set quarantined, - Set coolingOff) { + Set coolingOff, + Set modelOff) { + + /** + * Back-compat form before the fleetd #422 model on/off gate was added — no candidate's model + * is off. Keeps pre-#422 call sites (tests included) compiling and behaving identically. + */ + public PlacementContext(String defaultProfile, List candidates, + Function liveCount, Set unreachable, + Set quarantined, Set coolingOff) { + this(defaultProfile, candidates, liveCount, unreachable, quarantined, coolingOff, Set.of()); + } } 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 5cc2e77..d80a51c 100644 --- a/fleetd/src/main/java/dev/ltms/fleet/placement/PlacementPolicyUtil.java +++ b/fleetd/src/main/java/dev/ltms/fleet/placement/PlacementPolicyUtil.java @@ -15,15 +15,18 @@ final class PlacementPolicyUtil { * 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), and have not - * reached their maxLoad. A {@code null} maxLoad means unlimited. + * 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. */ static List available(PlacementContext ctx) { List out = new ArrayList<>(); for (PlacementCandidate c : ctx.candidates()) { if (c.excluded() || ctx.unreachable().contains(c.profile()) || ctx.quarantined().contains(c.profile()) - || ctx.coolingOff().contains(c.profile())) { + || ctx.coolingOff().contains(c.profile()) + || ctx.modelOff().contains(c.profile())) { continue; } Integer cap = c.maxLoad(); @@ -40,12 +43,13 @@ final class PlacementPolicyUtil { /** * Build a clear exception describing why every candidate was dropped: all weight-0, all - * quarantined, all cooling off, all at capacity, all unreachable, or a mix. Each candidate is - * counted into exactly one bucket (weight-excluded first, then quarantined, then cooling off) - * so a candidate excluded for more than one reason is never double-counted — a candidate that is - * both quarantined (CB-578 stage B, backend exhausted) and cooling off (fleetd #201 Unit 5, - * repeated backend errors) counts only as quarantined, matching {@code CompositePeerLauncher}'s - * explicit-spawn ordering: exhaustion quarantine takes priority when both are active. + * quarantined, all cooling off, all model-off, all at capacity, all unreachable, or a mix. Each + * candidate is counted into exactly one bucket (weight-excluded first, then quarantined, then + * cooling off, then model-off) so a candidate excluded for more than one reason is never + * double-counted — a candidate that is both quarantined (CB-578 stage B, backend exhausted) and + * cooling off (fleetd #201 Unit 5, repeated backend errors) counts only as quarantined, matching + * {@code CompositePeerLauncher}'s explicit-spawn ordering: exhaustion quarantine takes priority + * when more than one applies. */ static PlacementException emptyException(PlacementContext ctx) { int weightExcluded = 0; @@ -53,6 +57,7 @@ final class PlacementPolicyUtil { int unreachable = 0; int quarantined = 0; int coolingOff = 0; + int modelOff = 0; for (PlacementCandidate c : ctx.candidates()) { Integer cap = c.maxLoad(); if (c.excluded()) { @@ -61,6 +66,8 @@ final class PlacementPolicyUtil { quarantined++; } else if (ctx.coolingOff().contains(c.profile())) { coolingOff++; + } else if (ctx.modelOff().contains(c.profile())) { + modelOff++; } else if (ctx.unreachable().contains(c.profile())) { unreachable++; } else if (cap != null && ctx.liveCount().apply(c.profile()) >= cap) { @@ -83,6 +90,10 @@ final class PlacementPolicyUtil { return new PlacementException( "all worker profiles are cooling off after repeated backend errors"); } + if (modelOff == total) { + return new PlacementException( + "all worker profiles name a model the operator has turned off"); + } if (atCap == total) { return new PlacementException("all worker profiles are at maxLoad"); } @@ -92,8 +103,9 @@ final class PlacementPolicyUtil { return new PlacementException("no worker profile available: " + atCap + " at maxLoad, " + unreachable + " unreachable, " + quarantined + " quarantined, " + coolingOff + " cooling off, " + + modelOff + " model-off, " + weightExcluded + " weight-0, " - + (total - atCap - unreachable - quarantined - coolingOff - weightExcluded) + + (total - atCap - unreachable - quarantined - coolingOff - modelOff - weightExcluded) + " remaining"); } } diff --git a/fleetd/src/test/java/dev/ltms/fleet/config/ConfigRefTopLevelCoverageTest.java b/fleetd/src/test/java/dev/ltms/fleet/config/ConfigRefTopLevelCoverageTest.java index b89856a..42a276a 100644 --- a/fleetd/src/test/java/dev/ltms/fleet/config/ConfigRefTopLevelCoverageTest.java +++ b/fleetd/src/test/java/dev/ltms/fleet/config/ConfigRefTopLevelCoverageTest.java @@ -79,6 +79,14 @@ class ConfigRefTopLevelCoverageTest { *
  • {@code memberCredentials} — read live at {@code Fleetd.java:198, 205, 729}.
    • *
  • {@code memberLoginShell} — read live at * {@code HerdrPeerLauncher#configuredMemberLoginShell}.
    • + *
  • {@code models} (fleetd #422) — membership is re-validated in full against the fresh + * config on every {@code ConfigRef.reload()} (via {@code FleetConfig#validateAll()}), so + * a bad edit is refused, never cached stale; the on/off half is read live by {@code + * CompositePeerLauncher.enforceModelEnabled} and its candidate filter, and by {@code + * fleet_profiles}/{@code GET /profiles} through {@code PeerLauncher.disabledModels()}. + * Unlike {@code fleet} below, nothing about {@code models} is baked into a startup-built + * object anywhere — there is no frozen half, so it belongs here whole rather than in + * {@code SPLIT_KEYS}.
    • * * *

    {@code fleet} used to sit here too, on the strength of most of it (role pools, charters, @@ -93,7 +101,7 @@ class ConfigRefTopLevelCoverageTest { * while this test stayed green throughout.

    */ private static final Set HOT_EXCLUDED_TOP_LEVEL_KEYS = - Set.of("placement", "memberCredentials", "memberLoginShell"); + Set.of("placement", "memberCredentials", "memberLoginShell", "models"); @Test void everyTopLevelComponentIsAccountedForInExactlyOneClass() { @@ -120,7 +128,7 @@ class ConfigRefTopLevelCoverageTest { // The escape hatch is pinned. Growing it requires editing this line — a visible, deliberate // diff, not a quiet one. See the field javadoc above for what "belongs here" actually means. - assertEquals(Set.of("placement", "memberCredentials", "memberLoginShell"), hot, + assertEquals(Set.of("placement", "memberCredentials", "memberLoginShell", "models"), hot, "HOT_EXCLUDED_TOP_LEVEL_KEYS changed. A component belongs here ONLY if it is read " + "live off the config supplier, never because adding it makes this test " + "pass. If you are adding one to silence this test, that is fleetd #323 " diff --git a/fleetd/src/test/java/dev/ltms/fleet/config/FleetConfigTest.java b/fleetd/src/test/java/dev/ltms/fleet/config/FleetConfigTest.java index 19baf53..0e2ad93 100644 --- a/fleetd/src/test/java/dev/ltms/fleet/config/FleetConfigTest.java +++ b/fleetd/src/test/java/dev/ltms/fleet/config/FleetConfigTest.java @@ -2901,4 +2901,119 @@ class FleetConfigTest { void modelsIsAKnownTopLevelKey() { assertTrue(FleetConfig.KNOWN_TOP_LEVEL_KEYS.contains("models")); } + + // --- fleetd #422: model on/off (spawn-time gate + hot reload) ----------------------------- + + /** + * Criterion 3: an {@code allow:} entry written before {@code enabled:} existed — no such key in + * the YAML at all — must behave exactly as it always did: on, and absent from {@link + * FleetConfig.Models#offIds()}. This is the old-style fixture the ticket asks for, proven + * through a real YAML load rather than only through the {@code ModelEntry(String)} back-compat + * constructor. + */ + @Test + void anOldStyleAllowEntryWithNoEnabledFieldStaysOn(@TempDir Path dir) throws Exception { + Path f = dir.resolve("fleetd.yaml"); + Files.writeString(f, """ + bind: + port: 8080 + profiles: + sonnet: + baseUrl: http://gx10.gw:8000 + model: claude-sonnet-5 + models: + allow: + - model: claude-sonnet-5 + """); + + FleetConfig cfg = FleetConfig.load(f); + assertDoesNotThrow(cfg::validateModels); + assertTrue(cfg.models().ids().contains("claude-sonnet-5"), "membership is unaffected"); + assertTrue(cfg.models().offIds().isEmpty(), "no enabled: field ⇒ nothing is off"); + } + + /** + * Criterion 7 (the whole point of the ticket, mirroring criterion 3's shape): a profile naming a + * model that is turned off ({@code enabled: false}) must still be VALID config — + * {@code validateModels()} checks membership only, never the on/off state, so it must not throw. + * Collapsing "off" into "removed from allow:" would make this test fail, which is exactly the + * design trap the ticket calls out. + */ + @Test + void anOffModelIsStillValidConfigForValidateModels(@TempDir Path dir) throws Exception { + Path f = dir.resolve("fleetd.yaml"); + Files.writeString(f, """ + bind: + port: 8080 + profiles: + local: + baseUrl: http://local.gw:8000 + model: deepseek-v4-flash + models: + allow: + - model: deepseek-v4-flash + enabled: false + """); + + FleetConfig cfg = FleetConfig.load(f); + assertDoesNotThrow(cfg::validateModels, + "an off model must stay a valid allow-list member — only the spawn gate reads enabled"); + assertTrue(cfg.models().ids().contains("deepseek-v4-flash")); + assertTrue(cfg.models().offIds().contains("deepseek-v4-flash")); + } + + /** + * Criterion 8: one {@code enabled: false} entry names a model, not a profile — every profile + * naming that model is off, on purpose (the live example: {@code deepseek-v4-flash} backs both + * {@code local} and {@code local-direct}). Proven here at the {@code Models}/config-load level; + * {@code CompositePeerLauncherTest} proves the spawn-time consequence for both profiles. + */ + @Test + void turningOffOneModelIsIndependentOfHowManyProfilesNameIt(@TempDir Path dir) throws Exception { + Path f = dir.resolve("fleetd.yaml"); + Files.writeString(f, """ + bind: + port: 8080 + profiles: + local: + baseUrl: http://local.gw:8000 + model: deepseek-v4-flash + local-direct: + baseUrl: http://local.gw:8001 + model: deepseek-v4-flash + models: + allow: + - model: deepseek-v4-flash + enabled: false + """); + + FleetConfig cfg = FleetConfig.load(f); + assertDoesNotThrow(cfg::validateModels); + // One allow-list entry, one off id — the fan-out to both profiles happens at the reader + // (CompositePeerLauncher), not by duplicating the entry per profile. + assertEquals(Set.of("deepseek-v4-flash"), cfg.models().offIds()); + assertEquals("deepseek-v4-flash", cfg.profiles().get("local").model()); + assertEquals("deepseek-v4-flash", cfg.profiles().get("local-direct").model()); + } + + /** An {@code enabled: true} entry (explicit, not just absent) also stays on — not just null. */ + @Test + void explicitlyEnabledTrueStaysOn(@TempDir Path dir) throws Exception { + Path f = dir.resolve("fleetd.yaml"); + Files.writeString(f, """ + bind: + port: 8080 + profiles: + sonnet: + baseUrl: http://gx10.gw:8000 + model: claude-sonnet-5 + models: + allow: + - model: claude-sonnet-5 + enabled: true + """); + + FleetConfig cfg = FleetConfig.load(f); + assertTrue(cfg.models().offIds().isEmpty()); + } } 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 3ef032c..ad05a68 100644 --- a/fleetd/src/test/java/dev/ltms/fleet/member/CompositePeerLauncherTest.java +++ b/fleetd/src/test/java/dev/ltms/fleet/member/CompositePeerLauncherTest.java @@ -3,6 +3,7 @@ package dev.ltms.fleet.member; import ch.qos.logback.classic.Logger; import ch.qos.logback.classic.spi.ILoggingEvent; import ch.qos.logback.core.read.ListAppender; +import dev.ltms.fleet.config.ConfigRef; import dev.ltms.fleet.config.FleetConfig; import dev.ltms.fleet.guard.SubscriptionGuard; import dev.ltms.fleet.herdr.Agent; @@ -22,8 +23,11 @@ import dev.ltms.fleet.placement.BackendQuarantine; import dev.ltms.fleet.placement.PlacementException; import dev.ltms.fleet.placement.PlacementPolicies; import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.io.TempDir; import org.slf4j.LoggerFactory; +import java.nio.file.Files; +import java.nio.file.Path; import java.util.EnumSet; import java.util.HashMap; import java.util.LinkedHashMap; @@ -141,6 +145,13 @@ class CompositePeerLauncherTest { null, null, credentialId, null); } + /** Like {@link #stubWorker(String)}, but with an explicit {@code model:} for fleetd #422 tests. */ + private static FleetConfig.Profile stubWorkerModel(String profile, String model) { + return new FleetConfig.Profile(profile, "http://gx00.gw:8000", model, + null, "FLEETD_WORKER_TOKEN", List.of("claude"), "tab", "fleetd-workers", + "w #{n}", null, null, null, null, null, null, null, null, null); + } + /** * An order-preserving profile map. Never {@code Map.of} here: its iteration order is * salted per JVM run, and the weighted policy breaks an exact-weight tie on candidate order — @@ -1189,4 +1200,282 @@ class CompositePeerLauncherTest { assertDoesNotThrow(() -> composite.spawn(new SpawnRequest("claude", null, null))); assertDoesNotThrow(() -> composite.spawn(new SpawnRequest("gemini", null, null))); } + + // ── fleetd #422: the model on/off gate — a FOURTH, independent reason to refuse a spawn ────── + // ── (an operator decision, never a backend-reported outage) — never merged with quarantine ──── + // ── or cool-off above ───────────────────────────────────────────────────────────────────────── + + private static final BackendOutagePolicy NO_OUTAGE = new BackendOutagePolicy(() -> 0L); + + /** Criterion 1: an explicit spawn onto a profile whose model is off is refused. */ + @Test + void explicitSpawnOntoAnOffModelProfileIsRefused() { + FakeHerdr herdr = new FakeHerdr(); + Map profiles = ordered( + "local", stubWorkerModel("local", "deepseek-v4-flash"), + "sonnet", stubWorkerModel("sonnet", "claude-sonnet-5")); + StubLauncher adapter = new StubLauncher("claude", herdr, profiles, "local", Set.of()); + FleetConfig.Models models = new FleetConfig.Models(List.of( + new FleetConfig.Models.ModelEntry("deepseek-v4-flash", false))); + CompositePeerLauncher composite = new CompositePeerLauncher(List.of(adapter), "local", profiles, + PlacementPolicies.fixed(), _ -> 0, null, BackendQuarantine.none(), NO_OUTAGE, + () -> models); + + PlacementException e = assertThrows(PlacementException.class, + () -> composite.spawn(new SpawnRequest("local", null, null))); + assertTrue(e.getMessage().contains("local"), "message names the profile: " + e.getMessage()); + assertTrue(e.getMessage().contains("deepseek-v4-flash"), + "message names the model: " + e.getMessage()); + assertTrue(e.getMessage().contains("operator") && e.getMessage().contains("turned off"), + "wording says the OPERATOR turned it off: " + e.getMessage()); + // Distinct from quarantine/cool-off wording (criterion 1's explicit requirement). + assertFalse(e.getMessage().contains("quarantined"), "must not read like quarantine: " + e.getMessage()); + assertFalse(e.getMessage().contains("cooling off"), "must not read like cool-off: " + e.getMessage()); + assertEquals(0, adapter.spawnCount("local"), "the off-model profile is never delegated to"); + } + + /** A profile whose model is NOT off spawns normally even while another model is off. */ + @Test + void explicitSpawnOntoAnEnabledModelProfileSucceedsWhileAnotherModelIsOff() { + FakeHerdr herdr = new FakeHerdr(); + Map profiles = ordered( + "local", stubWorkerModel("local", "deepseek-v4-flash"), + "sonnet", stubWorkerModel("sonnet", "claude-sonnet-5")); + StubLauncher adapter = new StubLauncher("claude", herdr, profiles, "local", Set.of()); + FleetConfig.Models models = new FleetConfig.Models(List.of( + new FleetConfig.Models.ModelEntry("deepseek-v4-flash", false))); + CompositePeerLauncher composite = new CompositePeerLauncher(List.of(adapter), "local", profiles, + PlacementPolicies.fixed(), _ -> 0, null, BackendQuarantine.none(), NO_OUTAGE, + () -> models); + + assertDoesNotThrow(() -> composite.spawn(new SpawnRequest("sonnet", null, null))); + assertEquals(1, adapter.spawnCount("sonnet")); + } + + /** + * Criterion 8: one {@code enabled: false} entry disables EVERY profile naming that model — the + * live example, {@code deepseek-v4-flash} backing both {@code local} and {@code local-direct}. + */ + @Test + void turningOffOneModelRefusesEveryProfileThatNamesIt() { + FakeHerdr herdr = new FakeHerdr(); + Map profiles = ordered( + "local", stubWorkerModel("local", "deepseek-v4-flash"), + "local-direct", stubWorkerModel("local-direct", "deepseek-v4-flash")); + StubLauncher adapter = new StubLauncher("claude", herdr, profiles, "local", Set.of()); + FleetConfig.Models models = new FleetConfig.Models(List.of( + new FleetConfig.Models.ModelEntry("deepseek-v4-flash", false))); + CompositePeerLauncher composite = new CompositePeerLauncher(List.of(adapter), "local", profiles, + PlacementPolicies.fixed(), _ -> 0, null, BackendQuarantine.none(), NO_OUTAGE, + () -> models); + + assertThrows(PlacementException.class, () -> composite.spawn(new SpawnRequest("local", null, null))); + assertThrows(PlacementException.class, + () -> composite.spawn(new SpawnRequest("local-direct", null, null)), + "local-direct shares local's model, so it must be refused too"); + assertEquals(0, adapter.spawnCount("local")); + assertEquals(0, adapter.spawnCount("local-direct")); + } + + /** Criterion 3: an old-style entry with no {@code enabled} field never refuses a spawn. */ + @Test + void anEntryWithNoEnabledFieldNeverRefusesASpawn() { + FakeHerdr herdr = new FakeHerdr(); + Map profiles = Map.of("local", stubWorkerModel("local", "deepseek-v4-flash")); + StubLauncher adapter = new StubLauncher("claude", herdr, profiles, "local", Set.of()); + // The back-compat single-arg ModelEntry constructor — no enabled field in the shape at all. + FleetConfig.Models models = new FleetConfig.Models( + List.of(new FleetConfig.Models.ModelEntry("deepseek-v4-flash"))); + CompositePeerLauncher composite = new CompositePeerLauncher(List.of(adapter), "local", profiles, + PlacementPolicies.fixed(), _ -> 0, null, BackendQuarantine.none(), NO_OUTAGE, + () -> models); + + assertDoesNotThrow(() -> composite.spawn(new SpawnRequest("local", null, null))); + assertEquals(1, adapter.spawnCount("local")); + } + + /** A model absent from {@code models.allow:} entirely (nothing to gate against) is never refused. */ + @Test + void aModelNotConfiguredInTheAllowListIsNeverGated() { + FakeHerdr herdr = new FakeHerdr(); + Map profiles = Map.of("local", stubWorkerModel("local", "unlisted-model")); + StubLauncher adapter = new StubLauncher("claude", herdr, profiles, "local", Set.of()); + FleetConfig.Models models = new FleetConfig.Models(List.of( + new FleetConfig.Models.ModelEntry("deepseek-v4-flash", false))); + CompositePeerLauncher composite = new CompositePeerLauncher(List.of(adapter), "local", profiles, + PlacementPolicies.fixed(), _ -> 0, null, BackendQuarantine.none(), NO_OUTAGE, + () -> models); + + assertDoesNotThrow(() -> composite.spawn(new SpawnRequest("local", null, null))); + } + + /** Criterion 2: an unqualified spawn skips an off-model candidate and lands on another one. */ + @Test + void placementSkipsAnOffModelProfileAndRoutesToAnotherOne() { + FakeHerdr herdr = new FakeHerdr(); + Map profiles = ordered( + "local", stubWorkerModel("local", "deepseek-v4-flash"), + "sonnet", stubWorkerModel("sonnet", "claude-sonnet-5")); + StubLauncher adapter = new StubLauncher("claude", herdr, profiles, "local", Set.of()); + FleetConfig.Models models = new FleetConfig.Models(List.of( + new FleetConfig.Models.ModelEntry("deepseek-v4-flash", false))); + CompositePeerLauncher composite = new CompositePeerLauncher(List.of(adapter), "local", profiles, + PlacementPolicies.weighted(), _ -> 0, null, BackendQuarantine.none(), NO_OUTAGE, + () -> models); + + PeerHandle h = composite.spawn(new SpawnRequest(null, null, null)); + assertEquals("sonnet", h.profile(), "local's model is off, so an unqualified spawn must land on sonnet"); + assertEquals(0, adapter.spawnCount("local")); + } + + /** + * Criterion 2's second half: when EVERY candidate's model is off, the placement exception must + * name that as the cause — not a generic "no candidates" message. + */ + @Test + void automaticPlacementNamesModelOffWhenEveryCandidateIsOffModel() { + FakeHerdr herdr = new FakeHerdr(); + Map profiles = ordered( + "local", stubWorkerModel("local", "deepseek-v4-flash"), + "local-direct", stubWorkerModel("local-direct", "deepseek-v4-flash")); + StubLauncher adapter = new StubLauncher("claude", herdr, profiles, "local", Set.of()); + FleetConfig.Models models = new FleetConfig.Models(List.of( + new FleetConfig.Models.ModelEntry("deepseek-v4-flash", false))); + CompositePeerLauncher composite = new CompositePeerLauncher(List.of(adapter), "local", profiles, + PlacementPolicies.weighted(), _ -> 0, null, BackendQuarantine.none(), NO_OUTAGE, + () -> models); + + PlacementException e = assertThrows(PlacementException.class, + () -> composite.spawn(new SpawnRequest(null, null, null)), + "both candidates share the off model — nothing is available"); + assertTrue(e.getMessage().contains("model") && e.getMessage().contains("turned off"), + "message names model-off as the cause, not a generic no-candidates message: " + + e.getMessage()); + } + + /** + * fleetd #422 follow-up: {@code fixed} is the DEFAULT placement policy ({@code + * PlacementPolicies.fromName} returns it for an absent/blank name), and it built its own inline + * candidate filter instead of calling {@code PlacementPolicyUtil.available()} — so it never + * checked {@code modelOff()}. Mirrors {@code placementSkipsAnOffModelProfileAndRoutesToAnotherOne} + * above with only the policy swapped, to prove the gate now fires on the path most fleets use. + */ + @Test + void fixedPlacementSkipsAnOffModelProfileToo() { + FakeHerdr herdr = new FakeHerdr(); + Map profiles = ordered( + "local", stubWorkerModel("local", "deepseek-v4-flash"), + "sonnet", stubWorkerModel("sonnet", "claude-sonnet-5")); + StubLauncher adapter = new StubLauncher("claude", herdr, profiles, "local", Set.of()); + FleetConfig.Models models = new FleetConfig.Models(List.of( + new FleetConfig.Models.ModelEntry("deepseek-v4-flash", false))); + CompositePeerLauncher composite = new CompositePeerLauncher(List.of(adapter), "local", profiles, + PlacementPolicies.fixed(), _ -> 0, null, BackendQuarantine.none(), NO_OUTAGE, + () -> models); + + PeerHandle h = composite.spawn(new SpawnRequest(null, null, null)); + assertEquals("sonnet", h.profile(), "local's model is off, so an unqualified spawn must land on sonnet"); + assertEquals(0, adapter.spawnCount("local")); + } + + /** + * Criterion 4: turning a model off/on is HOT — no restart — proven through a REAL + * {@code ConfigRef.reload()}, not a hand-rolled supplier swap. Also proves {@code models} is + * correctly reclassified: the reload's {@link ConfigRef.Outcome#applied()} is {@code true} and + * {@code "models"} never appears in {@link ConfigRef.Outcome#deferred()}. + */ + @Test + void modelOnOffIsHotReloadedThroughARealConfigRef(@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 + models: + allow: + - 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); + + assertDoesNotThrow(() -> composite.spawn(new SpawnRequest("local", null, null)), + "the model starts enabled"); + assertEquals(1, adapter.spawnCount("local")); + + 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 + """); + ConfigRef.Outcome outcome = configRef.reload(); + assertTrue(outcome.applied(), "a models.allow on/off edit must apply live, never be refused"); + assertFalse(outcome.deferred().contains("models"), + "models is hot-excluded now — it must never be reported as a deferred key"); + + PlacementException e = assertThrows(PlacementException.class, + () -> composite.spawn(new SpawnRequest("local", null, null)), + "the very next spawn must see the reload, with no restart"); + assertTrue(e.getMessage().contains("deepseek-v4-flash")); + assertEquals(1, adapter.spawnCount("local"), "still just the one successful spawn from before"); + + // And back on, still hot, still no restart. + 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 + """); + ConfigRef.Outcome reEnabled = configRef.reload(); + assertTrue(reEnabled.applied()); + assertDoesNotThrow(() -> composite.spawn(new SpawnRequest("local", null, null))); + assertEquals(2, adapter.spawnCount("local")); + } + + /** {@code fleet_profiles}/{@code GET /profiles} must read the exact same live source the gate reads. */ + @Test + void disabledModelsReportsWhatTheGateActuallyEnforces() { + FakeHerdr herdr = new FakeHerdr(); + Map profiles = ordered( + "local", stubWorkerModel("local", "deepseek-v4-flash"), + "sonnet", stubWorkerModel("sonnet", "claude-sonnet-5")); + StubLauncher adapter = new StubLauncher("claude", herdr, profiles, "local", Set.of()); + FleetConfig.Models models = new FleetConfig.Models(List.of( + new FleetConfig.Models.ModelEntry("deepseek-v4-flash", false), + new FleetConfig.Models.ModelEntry("claude-sonnet-5", true))); + CompositePeerLauncher composite = new CompositePeerLauncher(List.of(adapter), "local", profiles, + PlacementPolicies.fixed(), _ -> 0, null, BackendQuarantine.none(), NO_OUTAGE, + () -> models); + + assertEquals(Set.of("deepseek-v4-flash"), composite.disabledModels()); + } + + /** A caller with no {@code models:} block (the map-based constructors) reports nothing off. */ + @Test + void disabledModelsIsEmptyWithNoModelsConfigured() { + FakeHerdr herdr = new FakeHerdr(); + PeerLauncher composite = composite(herdr); + assertEquals(Set.of(), composite.disabledModels()); + } } 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 69e5de4..8f0f73b 100644 --- a/fleetd/src/test/java/dev/ltms/fleet/placement/PlacementPolicyTest.java +++ b/fleetd/src/test/java/dev/ltms/fleet/placement/PlacementPolicyTest.java @@ -90,6 +90,63 @@ class PlacementPolicyTest { assertTrue(e.getMessage().contains("quarantined"), e.getMessage()); } + // --- fleetd #422: FixedPlacementPolicy must consult modelOff too, at BOTH filter sites ------ + + /** The default-profile fast path (:44) must skip a default whose model is off. */ + @Test + void fixedSkipsModelOffDefault() { + PlacementPolicy policy = PlacementPolicies.fixed(); + PlacementContext ctx = new PlacementContext("b", + List.of(PlacementCandidate.profile("a"), PlacementCandidate.profile("b")), + noSessions(), Set.of(), Set.of(), Set.of(), Set.of("b")); + assertEquals("a", policy.select(ctx).profile(), + "the default 'b' names an off model, so fixed falls through to the first available candidate"); + } + + /** + * The fallback walk (:48-53) must skip an off-model 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 fixedFallbackWalkSkipsModelOffCandidate() { + PlacementPolicy policy = PlacementPolicies.fixed(); + PlacementContext ctx = new PlacementContext(null, + List.of(PlacementCandidate.profile("a"), PlacementCandidate.profile("b")), + noSessions(), Set.of(), Set.of(), Set.of(), Set.of("a")); + assertEquals("b", policy.select(ctx).profile(), + "candidate 'a' names an off model, so the fallback walk skips it and picks 'b'"); + } + + @Test + void fixedThrowsWhenDefaultAndEveryCandidateModelOff() { + PlacementPolicy policy = PlacementPolicies.fixed(); + PlacementContext ctx = new PlacementContext("b", + List.of(PlacementCandidate.profile("a"), PlacementCandidate.profile("b")), + noSessions(), Set.of(), Set.of(), Set.of(), Set.of("a", "b")); + PlacementException e = assertThrows(PlacementException.class, () -> policy.select(ctx)); + assertTrue(e.getMessage().contains("turned off"), + "message names model-off as the cause: " + e.getMessage()); + assertFalse(e.getMessage().contains("quarantined"), "must not read like quarantine: " + e.getMessage()); + assertFalse(e.getMessage().contains("cooling off"), "must not read like cool-off: " + e.getMessage()); + assertFalse(e.getMessage().contains("weight 0"), "must not read like weight-0: " + e.getMessage()); + } + + /** + * Quarantine still wins when a profile is both quarantined and model-off (mirrors {@code + * fixedThrowsWhenDefaultAndEveryCandidateQuarantined}'s priority over cooling off). + */ + @Test + void fixedReportsQuarantineNotModelOffWhenBothApply() { + PlacementPolicy policy = PlacementPolicies.fixed(); + PlacementContext ctx = new PlacementContext("b", + List.of(PlacementCandidate.profile("a"), PlacementCandidate.profile("b")), + noSessions(), Set.of(), Set.of("a", "b"), Set.of(), Set.of("a", "b")); + PlacementException e = assertThrows(PlacementException.class, () -> policy.select(ctx)); + assertTrue(e.getMessage().contains("quarantined"), e.getMessage()); + assertFalse(e.getMessage().contains("turned off"), + "quarantine takes priority over model-off in the message: " + e.getMessage()); + } + @Test void roundRobinCyclesThroughAvailableProfiles() { PlacementPolicy policy = PlacementPolicies.roundRobin();