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() {