From a196d34455e4364c6a4109b2b819771ae9beb27f Mon Sep 17 00:00:00 2001 From: Dai Ha Date: Thu, 10 Sep 2026 13:06:56 +0700 Subject: [PATCH 1/4] fleetd #431: pin profileForSlot, isSlot and nameForSlot against a live reload MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit #424 made MemberRegistry.slots() re-read fleet: on every call, but only roleForSlot was tested against a real reload. profileForSlot, isSlot and nameForSlot all have the same live-read line and none was pinned — proved by freezing each to a construction-time snapshot and watching the full suite stay green. Adds 4 tests to MemberRegistryLiveTest, each driving a real ConfigRef.reload() against a @TempDir config file (never two frozen registries compared in memory, which would test the constructor instead of the reload): - profileForSlotReflectsAProfileChangedByReload - isSlotStopsReportingASlotRemovedByReload / isSlotStartsReportingASlotAddedByReload - nameForSlotReflectsANameChangedByReload No production change. profileForSlot has no call site anywhere in src/main yet, so there is no spawn-lifecycle seam to drive the test through beyond the accessor itself. --- .../fleet/auth/MemberRegistryLiveTest.java | 98 +++++++++++++++++++ 1 file changed, 98 insertions(+) diff --git a/fleetd/src/test/java/dev/ltms/fleet/auth/MemberRegistryLiveTest.java b/fleetd/src/test/java/dev/ltms/fleet/auth/MemberRegistryLiveTest.java index ea177e1..fde51aa 100644 --- a/fleetd/src/test/java/dev/ltms/fleet/auth/MemberRegistryLiveTest.java +++ b/fleetd/src/test/java/dev/ltms/fleet/auth/MemberRegistryLiveTest.java @@ -69,6 +69,22 @@ class MemberRegistryLiveTest { profile: sonnet """; + /** Same slot name ({@code designer}) as {@link #WITH_SONNET_SLOT}, repointed to a different profile. */ + private static final String WITH_OPUS_SLOT = """ + fleet: + architects: + designer: + profile: opus + """; + + /** Same profile ({@code sonnet}) as {@link #WITH_SONNET_SLOT}, but the pool key is renamed. */ + private static final String WITH_RENAMED_SLOT = """ + fleet: + architects: + architect-lead: + profile: sonnet + """; + private static ConfigRef refFor(Path f) { return new ConfigRef(f, FleetConfig.load(f)); } @@ -155,6 +171,88 @@ class MemberRegistryLiveTest { "a slot added by reload must be reservable with no restart"); } + // ── profileForSlot, isSlot and nameForSlot are live too (fleetd #431) ───────────────────────── + // #424 pinned roleForSlot against a live reload but left these three untested — proved by + // mutating each to read a snapshot flattened once at construction: the full suite stayed green + // for all three. profileForSlot is the one with a real production stake: the spawn lifecycle + // reads it to pick an architect slot's backend, and fleetd has no call site for it yet (grepped + // "profileForSlot" across src/main — only this file and MemberRegistryTest reference it), so + // there is no seam to drive this test through beyond the accessor itself. + + @Test + void profileForSlotReflectsAProfileChangedByReload(@TempDir Path dir) throws Exception { + Path f = dir.resolve("fleetd.yaml"); + Files.writeString(f, yaml(WITH_SONNET_SLOT)); + ConfigRef ref = refFor(f); + MemberRegistry registry = MemberRegistry.live(() -> ref.get().fleet()); + + assertEquals("sonnet", registry.profileForSlot("architect:designer"), + "the slot's profile before the reload"); + + Files.writeString(f, yaml(WITH_OPUS_SLOT)); + ConfigRef.Outcome out = ref.reload(); + assertTrue(out.applied(), "the reload must actually take effect: " + out.summary()); + + assertEquals("opus", registry.profileForSlot("architect:designer"), + "repointing the slot to a different profile must take effect with no restart — " + + "this is what the spawn lifecycle reads to pick an architect's backend"); + } + + @Test + void isSlotStopsReportingASlotRemovedByReload(@TempDir Path dir) throws Exception { + Path f = dir.resolve("fleetd.yaml"); + Files.writeString(f, yaml(WITH_SONNET_SLOT)); + ConfigRef ref = refFor(f); + MemberRegistry registry = MemberRegistry.live(() -> ref.get().fleet()); + + assertTrue(registry.isSlot("architect:designer"), "the slot is configured before the reload"); + + Files.writeString(f, yaml(WITHOUT_ARCHITECT_SLOTS)); + ConfigRef.Outcome out = ref.reload(); + assertTrue(out.applied(), "the reload must actually take effect: " + out.summary()); + + assertFalse(registry.isSlot("architect:designer"), + "removing the slot from config must make isSlot say so on the very next call, " + + "with no restart"); + } + + @Test + void isSlotStartsReportingASlotAddedByReload(@TempDir Path dir) throws Exception { + Path f = dir.resolve("fleetd.yaml"); + Files.writeString(f, yaml(WITHOUT_ARCHITECT_SLOTS)); + ConfigRef ref = refFor(f); + MemberRegistry registry = MemberRegistry.live(() -> ref.get().fleet()); + + assertFalse(registry.isSlot("architect:designer"), "no architect slot is configured yet"); + + Files.writeString(f, yaml(WITH_SONNET_SLOT)); + ConfigRef.Outcome out = ref.reload(); + assertTrue(out.applied(), "the reload must actually take effect: " + out.summary()); + + assertTrue(registry.isSlot("architect:designer"), + "a slot added by reload must be visible to isSlot with no restart"); + } + + @Test + void nameForSlotReflectsANameChangedByReload(@TempDir Path dir) throws Exception { + Path f = dir.resolve("fleetd.yaml"); + Files.writeString(f, yaml(WITH_SONNET_SLOT)); + ConfigRef ref = refFor(f); + MemberRegistry registry = MemberRegistry.live(() -> ref.get().fleet()); + + assertEquals("designer", registry.nameForSlot("architect:designer"), + "the configured name before the reload"); + + Files.writeString(f, yaml(WITH_RENAMED_SLOT)); + ConfigRef.Outcome out = ref.reload(); + assertTrue(out.applied(), "the reload must actually take effect: " + out.summary()); + + assertNull(registry.nameForSlot("architect:designer"), + "the old key no longer names a configured slot — it was renamed away by the reload"); + assertEquals("architect-lead", registry.nameForSlot("architect:architect-lead"), + "the new name must be visible under its new qualified key with no restart"); + } + // ── a bound architect is demoted, but the binding itself is not touched (fleetd #424) ─────── // The lead's corrected ruling: the PRIVILEGE a slot grants is revoked on the bound session's // very next request, but the terminalToSlot BINDING itself is untouched by a reload — dropping From 7fd914df1aae351ac94c59c8242f788e112e2ce1 Mon Sep 17 00:00:00 2001 From: Dai Ha Date: Thu, 10 Sep 2026 13:18:52 +0700 Subject: [PATCH 2/4] fleetd #422 follow-up: make the model gate's own state observable MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit PeerLauncher.disabledModels() reported an empty set both when no models: block exists and when a block exists with nothing off, so fleet_profiles/GET /profiles and the startup log could not tell an inert gate from an armed one reporting zero. Add PeerLauncher.ModelGateState (configured + off), a modelGateState() default method disabledModels() now delegates to, and a CompositePeerLauncher override that reads models0() once and distinguishes the null-supplier case (no models: block) from a real, config-supplied block via identity against the NO_MODELS_CONFIGURED sentinel — reusing the exact accessor the spawn gate itself reads, per the fleetd #404 lesson. Wires the state into a new startup log line (Fleetd.modelGateCoverageLine) and a new modelGateArmed field in FleetMcp.profilesView, reported unconditionally alongside the existing modelsOff set. --- .../src/main/java/dev/ltms/fleet/Fleetd.java | 33 ++++++ .../java/dev/ltms/fleet/mcp/FleetMcp.java | 17 ++- .../fleet/member/CompositePeerLauncher.java | 25 +++- .../dev/ltms/fleet/peer/PeerLauncher.java | 50 +++++++- .../FleetdModelGateCoverageLineTest.java | 62 ++++++++++ .../mcp/FleetProfilesModelGateStateTest.java | 109 ++++++++++++++++++ .../member/CompositePeerLauncherTest.java | 102 ++++++++++++++++ 7 files changed, 388 insertions(+), 10 deletions(-) create mode 100644 fleetd/src/test/java/dev/ltms/fleet/FleetdModelGateCoverageLineTest.java create mode 100644 fleetd/src/test/java/dev/ltms/fleet/mcp/FleetProfilesModelGateStateTest.java 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 792305a..20afe80 100644 --- a/fleetd/src/main/java/dev/ltms/fleet/mcp/FleetMcp.java +++ b/fleetd/src/main/java/dev/ltms/fleet/mcp/FleetMcp.java @@ -1181,12 +1181,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/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..9d695b2 100644 --- a/fleetd/src/test/java/dev/ltms/fleet/member/CompositePeerLauncherTest.java +++ b/fleetd/src/test/java/dev/ltms/fleet/member/CompositePeerLauncherTest.java @@ -1453,6 +1453,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() { From f8b0d42a5cb63e6c449c41c0f5047e2c8c64e701 Mon Sep 17 00:00:00 2001 From: Dai Ha Date: Thu, 10 Sep 2026 13:25:19 +0700 Subject: [PATCH 3/4] fleetd #431 follow-up: profileForSlot's javadoc named a caller that does not exist MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The javadoc said "what the spawn lifecycle reads". Nothing in src/main calls profileForSlot at all, in either the ".profileForSlot(" or the "::profileForSlot" form. My own #431 ticket text repeated that sentence as a fact and ranked the three accessors by it, and the #432 worker copied it into the test file's comment and one assertion message. Corrected in all three places; the ticket correction is posted on #431. What the spawn lifecycle actually reads for an architect's profile is the SlotReservation that reserve() returns — SessionManager.java:225, "reservation == null ? profile : reservation.profile()". The corrected ranking, measured rather than read off the javadoc: - nameForSlot is wired, at CallerResolver.java:137 (method reference, which is why a ".nameForSlot(" grep missed it) - isSlot is reached through bind, called at MemberRegistry.java:235 and :376 - profileForSlot has no caller at all Prose only. No behaviour change. --- .../dev/ltms/fleet/auth/MemberRegistry.java | 10 +++++++++- .../ltms/fleet/auth/MemberRegistryLiveTest.java | 17 +++++++++++------ 2 files changed, 20 insertions(+), 7 deletions(-) diff --git a/fleetd/src/main/java/dev/ltms/fleet/auth/MemberRegistry.java b/fleetd/src/main/java/dev/ltms/fleet/auth/MemberRegistry.java index 3007842..5adb19e 100644 --- a/fleetd/src/main/java/dev/ltms/fleet/auth/MemberRegistry.java +++ b/fleetd/src/main/java/dev/ltms/fleet/auth/MemberRegistry.java @@ -177,7 +177,15 @@ public final class MemberRegistry implements MemberLifecycle { } /** - * The strong-model profile a slot runs under — what the spawn lifecycle reads. + * The strong-model profile a slot runs under, as of this call. + * + *

Nothing in {@code src/main} calls this (fleetd #431 — grepped both the {@code + * .profileForSlot(} and the {@code ::profileForSlot} form). This javadoc used to say "what the + * spawn lifecycle reads", and that seam does not exist: the spawn lifecycle takes its profile + * from the {@link MemberLifecycle.SlotReservation} that {@code reserve} returns, never from + * here. Kept and pinned rather than deleted because it is the natural accessor for that seam + * if one is added; live for the same reason as {@link #roleForSlot}, so a reload cannot leave + * it answering for the old config. * * @return the slot's configured {@code profile}, or {@code null} if the slot is unknown or * declares none diff --git a/fleetd/src/test/java/dev/ltms/fleet/auth/MemberRegistryLiveTest.java b/fleetd/src/test/java/dev/ltms/fleet/auth/MemberRegistryLiveTest.java index fde51aa..5ca1904 100644 --- a/fleetd/src/test/java/dev/ltms/fleet/auth/MemberRegistryLiveTest.java +++ b/fleetd/src/test/java/dev/ltms/fleet/auth/MemberRegistryLiveTest.java @@ -174,10 +174,16 @@ class MemberRegistryLiveTest { // ── profileForSlot, isSlot and nameForSlot are live too (fleetd #431) ───────────────────────── // #424 pinned roleForSlot against a live reload but left these three untested — proved by // mutating each to read a snapshot flattened once at construction: the full suite stayed green - // for all three. profileForSlot is the one with a real production stake: the spawn lifecycle - // reads it to pick an architect slot's backend, and fleetd has no call site for it yet (grepped - // "profileForSlot" across src/main — only this file and MemberRegistryTest reference it), so - // there is no seam to drive this test through beyond the accessor itself. + // for all three. + // + // The three differ in how much production behaviour depends on them, and the ticket first got + // this ranking wrong. nameForSlot is wired: CallerResolver passes members::nameForSlot, next to + // members::roleForSlot. isSlot is reached through bind, which calls it to refuse an unknown + // slot. profileForSlot has NO caller in src/main at all — grepped both the ".profileForSlot(" + // and the "::profileForSlot" form — so there is no seam to drive its test through beyond the + // accessor itself, and its own javadoc ("what the spawn lifecycle reads") describes a caller + // that does not exist. These tests pin the accessors as they are; whether profileForSlot should + // be wired or deleted is a separate question. @Test void profileForSlotReflectsAProfileChangedByReload(@TempDir Path dir) throws Exception { @@ -194,8 +200,7 @@ class MemberRegistryLiveTest { assertTrue(out.applied(), "the reload must actually take effect: " + out.summary()); assertEquals("opus", registry.profileForSlot("architect:designer"), - "repointing the slot to a different profile must take effect with no restart — " - + "this is what the spawn lifecycle reads to pick an architect's backend"); + "repointing the slot to a different profile must take effect with no restart"); } @Test From ed2027b2020baa8863ad221f884c740de20d9f5a Mon Sep 17 00:00:00 2001 From: Dai Ha Date: Thu, 10 Sep 2026 13:46:06 +0700 Subject: [PATCH 4/4] fleetd #435: make FixedPlacementPolicy honor maxLoad FixedPlacementPolicy (the default placement policy) never consulted maxLoad, so an at-cap default was chosen anyway on every unqualified spawn -- the cap was advisory, not enforced, for the one policy every config uses by default. weighted/round-robin already gated on it via PlacementPolicyUtil.available(). Extract the "at cap" predicate into PlacementPolicyUtil.atCap(ctx, c) so all three policies share one definition, and consult it at both of FixedPlacementPolicy's filter sites (the default fast path and the candidate walk), mirroring the existing weightExcluded pattern. An at-cap default now falls through to the next candidate instead of refusing the spawn -- only when every candidate is unusable does the policy still throw, naming the cap in the message. Update the class javadoc (five exceptions -> six) and the reason-priority comments to match CompositePeerLauncher's explicit-spawn order (quarantine, cooling off, max load, model-off). --- .../fleet/placement/FixedPlacementPolicy.java | 90 +++++++++++++------ .../fleet/placement/PlacementPolicyUtil.java | 32 ++++--- .../member/CompositePeerLauncherTest.java | 25 ++++++ .../fleet/placement/PlacementPolicyTest.java | 87 ++++++++++++++++++ 4 files changed, 196 insertions(+), 38 deletions(-) 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/member/CompositePeerLauncherTest.java b/fleetd/src/test/java/dev/ltms/fleet/member/CompositePeerLauncherTest.java index 9d695b2..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(); 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"));