diff --git a/fleetd/src/main/java/dev/ltms/fleet/Fleetd.java b/fleetd/src/main/java/dev/ltms/fleet/Fleetd.java index 29d8086..74f4c0f 100644 --- a/fleetd/src/main/java/dev/ltms/fleet/Fleetd.java +++ b/fleetd/src/main/java/dev/ltms/fleet/Fleetd.java @@ -696,11 +696,7 @@ public final class Fleetd { FleetMcp mcp = new FleetMcp(messages, workers, sessions, identity, presence, primaryRegistry, callers, FleetMcp.AuthorizationMode.ENFORCED, metrics, capacitySource(config, cfg, profile -> liveCountRef.get().apply(profile)), - new FleetMcp.HealthCoverageSource(() -> { - var health = config.get().health(); - return FleetHealthMonitor.coverage(health != null && health.isEnabled(), - health != null && health.notifications() != null && health.notifications().configured()); - }), + healthCoverageSource(config), quarantineSource, leadMailbox, outageSource, @@ -1038,6 +1034,38 @@ public final class Fleetd { cfg.profiles()::keySet, System::nanoTime); } + /** + * fleetd #426: package-private factory for {@code fleet_list}'s {@code healthCoverage} source, + * extracted out of {@code main} for the same reason {@link #capacitySource} and {@link + * #quarantineSource} were — and the same reason {@link #exhaustedPatternCoverageLine}/{@link + * #errorPatternCoverageLine} exist: {@link FleetHealthMonitor#coverage}'s three-branch method + * is easy to pin directly (a plain {@code (boolean, boolean) -> String} call), but that proves + * nothing about whether this call site pairs the right boolean with the right meaning. + * fleetd #415's measured lesson is the reason this matters here — swapping the two arguments at + * a call site like this one compiled clean and left the full suite green, because every existing + * test exercised the method in both directions without ever exercising the pairing. + * + *
{@code #407}'s "keep the config invalid, assert on the log line before the throw" option + * does not apply to this call site: the five reporters #407 covers all run in {@code main} + * before {@code cfg.validateAll()} (line ~171), so an invalid config still exercises + * them. This call site is built during {@code FleetMcp} construction, which runs only after + * {@code UnixSocketHerdrClient.connect} has already opened a real herdr socket (line ~188) — + * reaching it at all means main already performed real I/O, which the no-socket constraint on + * this ticket rules out. So the pairing is pinned by extracting it to this directly-callable + * factory instead, the same shape {@link #capacitySource}/{@link #quarantineSource} already use. + * + *
Reads {@code config.get().health()} live (health.notifications is a {@code SPLIT_KEYS} + * entry — see {@link ConfigRef#SPLIT_KEYS}), so a hot-reloaded notifications block changes what + * {@code fleet_list} reports without a restart, exactly like {@link #capacitySource}'s maxLoad. + */ + static FleetMcp.HealthCoverageSource healthCoverageSource(ConfigRef config) { + return new FleetMcp.HealthCoverageSource(() -> { + var health = config.get().health(); + return FleetHealthMonitor.coverage(health != null && health.isEnabled(), + health != null && health.notifications() != null && health.notifications().configured()); + }); + } + /** * fleetd #248: package-private factory for the member worktree/branch lookup {@link * CompletionResolver} uses to name a fallback report's worktree and branch (fleetd#241). diff --git a/fleetd/src/test/java/dev/ltms/fleet/FleetdHealthCoverageSourceWiringTest.java b/fleetd/src/test/java/dev/ltms/fleet/FleetdHealthCoverageSourceWiringTest.java new file mode 100644 index 0000000..a9adf61 --- /dev/null +++ b/fleetd/src/test/java/dev/ltms/fleet/FleetdHealthCoverageSourceWiringTest.java @@ -0,0 +1,160 @@ +package dev.ltms.fleet; + +import dev.ltms.fleet.config.ConfigRef; +import dev.ltms.fleet.config.FleetConfig; +import dev.ltms.fleet.mcp.FleetMcp; +import org.junit.jupiter.api.DisplayName; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.io.TempDir; + +import java.nio.file.Files; +import java.nio.file.Path; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertTrue; + +/** + * fleetd #426: {@code FleetHealthMonitor.coverage} had zero references anywhere in the test + * tree — not the method, not either output string, not the field it populates. {@code + * FleetHealthMonitorCoverageTest} (package {@code dev.ltms.fleet.health}) pins the three-branch + * method itself; that is the easy half. + * + *
The half that actually matters is this one: {@link Fleetd#healthCoverageSource} is the exact + * call site {@code Fleetd.main} wires into {@code FleetMcp}'s constructor, and it is what feeds + * {@code fleet_list}'s {@code healthCoverage} field (see {@code FleetMcp#listFleet}'s {@code + * result.put("healthCoverage", healthCoverage.value().get())}). Measured precedent on fleetd #423 + * (for #415): swapping two arguments at a call site like this one — recreating #415's defect with + * the keys exchanged — compiled with 0 errors and ran the ENTIRE suite (1506 tests) green. A + * thoroughly-tested method proves nothing about whether the call site pairs its arguments correctly; + * only a test that drives the call site itself can catch that. + * + *
Why this is not driven through a real {@code Fleetd.main} the way #407 drives its five + * reporters (keep the config invalid, assert on the log line emitted before {@code + * cfg.validateAll()} throws): all five of #407's reporters run in {@code main} before {@code + * validateAll()} (line ~171). {@link Fleetd#healthCoverageSource} is built during {@code FleetMcp} + * construction, which happens only after {@code UnixSocketHerdrClient.connect} has already opened a + * real herdr socket (line ~188) and after {@code sessions}/{@code workers} are constructed. Reaching + * this call site by actually running {@code main} would require a real socket connect — banned by + * this ticket's hard constraints — so #407's option 1 does not apply here. Instead {@link + * Fleetd#healthCoverageSource} is extracted to a directly-callable package-private factory, the same + * shape {@link Fleetd#capacitySource} and {@link Fleetd#quarantineSource} already use for the same + * reason (see {@code FleetdCapacitySourceWiringTest}, the direct precedent this test follows). + * + *
Uses a real {@link FleetConfig#load} + {@link ConfigRef} (no socket, no port bind, no spawn, + * nothing written outside {@code @TempDir}) so the fixture goes through the actual YAML parser and + * {@code FleetConfig.Health}/{@code Notifications} records, not a hand-built stand-in that could + * silently drift from what the parser actually produces. + */ +class FleetdHealthCoverageSourceWiringTest { + + private static final String BASE = """ + bind: + host: 127.0.0.1 + port: 8765 + herdrSocket: ~/.config/herdr/herdr.sock + """; + + private static final String HEALTH_DISABLED_WITH_WEBHOOK = BASE + """ + health: + enabled: false + notifications: + mode: webhook + """; + + private static final String HEALTH_DETECTION_ONLY = BASE + """ + health: + enabled: true + """; + + private static final String HEALTH_FULL = BASE + """ + health: + enabled: true + notifications: + mode: webhook + """; + + private static final String HEALTH_ABSENT = BASE; + + @Test + @DisplayName("enabled: false reports off, even with a webhook configured") + void disabledHealthReportsOff(@TempDir Path dir) throws Exception { + FleetMcp.HealthCoverageSource source = sourceFor(dir, HEALTH_DISABLED_WITH_WEBHOOK); + + assertEquals("off", source.value().get(), + "health.enabled: false must report 'off' regardless of notifications — flipping " + + "the 'enabled' argument at the HealthCoverageSource call site would report " + + "'full' here instead"); + } + + @Test + @DisplayName("enabled with no notifications reports detection-only") + void enabledWithoutNotificationsReportsDetectionOnly(@TempDir Path dir) throws Exception { + FleetMcp.HealthCoverageSource source = sourceFor(dir, HEALTH_DETECTION_ONLY); + + assertEquals("detection-only", source.value().get()); + } + + @Test + @DisplayName("enabled with a webhook configured reports full") + void enabledWithNotificationsReportsFull(@TempDir Path dir) throws Exception { + FleetMcp.HealthCoverageSource source = sourceFor(dir, HEALTH_FULL); + + assertEquals("full", source.value().get(), + "health.enabled: true with notifications.mode: webhook must report 'full' — " + + "swapping 'full' and 'detection-only' at the call site, or breaking the " + + "enabled/notificationConfigured argument pairing, would report " + + "'detection-only' here instead"); + } + + @Test + @DisplayName("an absent health: block reports off") + void absentHealthBlockReportsOff(@TempDir Path dir) throws Exception { + FleetMcp.HealthCoverageSource source = sourceFor(dir, HEALTH_ABSENT); + + assertEquals("off", source.value().get()); + } + + /** + * fleetd #426, the live-wiring half: {@code health.notifications} is a {@code + * ConfigRef.SPLIT_KEYS} entry, and {@link Fleetd#healthCoverageSource} reads {@code + * config.get().health()} live (not the frozen startup {@code cfg}) — exactly like {@link + * Fleetd#capacitySource}'s {@code maxLoad} ({@code FleetdCapacitySourceWiringTest}'s {@code + * reloadedMaxLoadStillChangesWhatFleetListReports}). A hot-reloaded notifications block must + * change what {@code fleet_list} reports without a restart; a fix that froze the whole source + * against the startup snapshot would silently break that and every other test above would stay + * green, since none of them reload. + */ + @Test + @DisplayName("a hot notifications reload still changes what fleet_list reports") + void reloadedNotificationsStillChangeWhatFleetListReports(@TempDir Path dir) throws Exception { + Path file = dir.resolve("fleetd.yaml"); + Files.writeString(file, HEALTH_DETECTION_ONLY); + FleetConfig cfg = FleetConfig.load(file); + ConfigRef config = new ConfigRef(file, cfg); + + FleetMcp.HealthCoverageSource source = Fleetd.healthCoverageSource(config); + assertEquals("detection-only", source.value().get(), + "sanity: detection-only before any reload"); + + Files.writeString(file, HEALTH_FULL); + assertTrue(config.reload().applied()); + // The live snapshot now carries the webhook — proves the reload really happened and this + // test is not accidentally passing because nothing changed. + assertTrue(config.get().health().notifications() != null + && config.get().health().notifications().configured(), + "sanity: the reloaded config really carries a configured webhook"); + + assertEquals("full", source.value().get(), + "the SAME HealthCoverageSource instance must reflect a reloaded notifications " + + "block without a restart — health.notifications is read live off " + + "config.get(), exactly like capacitySource's maxLoad"); + } + + private static FleetMcp.HealthCoverageSource sourceFor(Path dir, String yaml) throws Exception { + Path file = dir.resolve("fleetd.yaml"); + Files.writeString(file, yaml); + FleetConfig cfg = FleetConfig.load(file); + ConfigRef config = new ConfigRef(file, cfg); + return Fleetd.healthCoverageSource(config); + } +} diff --git a/fleetd/src/test/java/dev/ltms/fleet/health/FleetHealthMonitorCoverageTest.java b/fleetd/src/test/java/dev/ltms/fleet/health/FleetHealthMonitorCoverageTest.java new file mode 100644 index 0000000..647698a --- /dev/null +++ b/fleetd/src/test/java/dev/ltms/fleet/health/FleetHealthMonitorCoverageTest.java @@ -0,0 +1,40 @@ +package dev.ltms.fleet.health; + +import org.junit.jupiter.api.Test; + +import static org.junit.jupiter.api.Assertions.assertEquals; + +/** + * fleetd #426: {@link FleetHealthMonitor#coverage} had zero references anywhere in the test tree — + * not the method, not either output string, not the field it populates. This pins the method's own + * three branches directly. + * + *
This is the easy half. It proves the method words each combination correctly, but it proves + * nothing about whether {@code Fleetd.java}'s two call sites pass the right argument in the right + * position — see {@code FleetdHealthCoverageSourceWiringTest} (package {@code dev.ltms.fleet}) for + * the half that actually guards the call site, following the measured fleetd #415 lesson that a + * thoroughly-tested method and an untested argument pairing at its call site are different risks. + * + *
The three output strings are load-bearing and must not change here. {@code + * "detection-only"} is read live off a running daemon's {@code fleet_list} today (measured + * 2026-09-12) — this test intentionally asserts the exact literal strings so a future edit to the + * wording trips it here first. + */ +class FleetHealthMonitorCoverageTest { + + @Test + void disabledIsOffRegardlessOfNotificationConfig() { + assertEquals("off", FleetHealthMonitor.coverage(false, false)); + assertEquals("off", FleetHealthMonitor.coverage(false, true)); + } + + @Test + void enabledWithoutNotificationsIsDetectionOnly() { + assertEquals("detection-only", FleetHealthMonitor.coverage(true, false)); + } + + @Test + void enabledWithNotificationsIsFull() { + assertEquals("full", FleetHealthMonitor.coverage(true, true)); + } +}