diff --git a/fleetd/src/main/java/dev/ltms/fleet/Fleetd.java b/fleetd/src/main/java/dev/ltms/fleet/Fleetd.java index 00cf817..f9b42d6 100644 --- a/fleetd/src/main/java/dev/ltms/fleet/Fleetd.java +++ b/fleetd/src/main/java/dev/ltms/fleet/Fleetd.java @@ -53,6 +53,7 @@ import dev.ltms.fleet.rest.FleetApp; import dev.ltms.fleet.session.GitWorktrees; import dev.ltms.fleet.session.MemberSession; import dev.ltms.fleet.session.SessionManager; +import dev.ltms.fleet.peer.MemberRole; import dev.ltms.fleet.peer.PeerLauncher; import dev.ltms.fleet.session.SessionReaper; import dev.ltms.fleet.member.ClaudeCodeLauncher; @@ -176,6 +177,12 @@ public final class Fleetd { // and the Fleetd-startup tests actually pin — see FleetConfig#validateAll's javadoc for // why a name-by-name list here would have the same defect it replaces. cfg.validateAll(); + // fleetd #613: validateAll() (validateMembers() inside it) only refuses a slot that names a + // bad role or profile — it says nothing about a role that has NO pool or NO charter at all, + // because both are legitimate ("unconstrained") states, not errors. Report them here, right + // after validation passes, so an operator sees the gap once per restart instead of finding + // it later in a roster row (see reportRoleFallbackGaps' javadoc for the measured cause). + reportRoleFallbackGaps(cfg); // fleetd #469, follow-up to #464: validateAll() (and validateCharters() inside it) only // checks that a charter's KEY is a role wire name and its text is non-blank — it never // looks at what the text actually names. This is the separate check that does: it asks @@ -2175,6 +2182,58 @@ public final class Fleetd { } } + /** + * fleetd #613: {@code FleetConfig.candidateProfiles(MemberRole)} (FleetConfig.java:1827) and + * {@code CompositePeerLauncher.poolFor} (CompositePeerLauncher.java:598-603) both fall back to + * every configured profile when a role has no {@code fleet.s:} pool — a + * deliberate "unconstrained" behaviour, kept unchanged here, that lets a config with only + * {@code profiles:} and no {@code fleet:} block still spawn. That fallback is silent, and on + * the host that opened this ticket it widened an unqualified {@code hunter} spawn to all 8 + * configured profiles and picked {@code local} as the resolved first choice — a profile every + * other pool on that same config gives weight 0 to. Report it once at boot instead, naming both + * how many profiles the gap opens onto and the exact first choice, since the first choice (not + * the pool size) is what actually surprised the operator. + * + *

A missing {@code fleet.charters.:} entry is reported separately: the member still + * runs, but with only the launcher's own reply charter and no role contract. Unlike the pool + * gap this already has a per-spawn instrument ({@code HerdrPeerLauncher.logCharterReceipt}, + * {@code SessionManager}'s {@code charterSource} roster field) — this boot line is the same + * information surfaced once, up front, rather than discovered per member later. + * + *

Never refuses to start over either gap — both are legitimate configurations, and this is a + * report, not a validation. Package-private so a test can call it directly and capture the log + * via a {@link ch.qos.logback.core.read.ListAppender}, the same pattern {@link + * #reportExhaustedPatternGap} and {@link #reportMemberCredentialsGap} already use. + */ + static void reportRoleFallbackGaps(FleetConfig cfg) { + List poolGaps = new ArrayList<>(); + List charterGaps = new ArrayList<>(); + int profileCount = cfg.profiles().size(); + for (MemberRole role : MemberRole.values()) { + boolean hasPool = cfg.fleet() != null && !cfg.fleet().profilesFor(role).isEmpty(); + if (!hasPool) { + String firstChoice = cfg.defaultProfileFor(role); + poolGaps.add(role.wireName() + " (may land on any of " + profileCount + + " profile(s), first choice " + + (firstChoice == null ? "none — no profiles configured" : "'" + firstChoice + "'") + + ")"); + } + String charter = cfg.fleet() == null ? null : cfg.fleet().charterFor(role); + if (charter == null || charter.isBlank()) { + charterGaps.add(role.wireName()); + } + } + if (!poolGaps.isEmpty()) { + log.info("role fallback: no fleet.s: pool for {} — an unqualified spawn of that " + + "role falls back to every configured profile (deliberate; see " + + "FleetConfig#candidateProfiles)", poolGaps); + } + if (!charterGaps.isEmpty()) { + log.info("role fallback: no fleet.charters: entry for {} — that role runs with only " + + "the launcher's reply charter, no role contract", charterGaps); + } + } + /** * fleetd #474: the one place both the startup call (right after {@code cfg.validateAll()} in * {@link #main}) and the reload call (wired into {@code config}'s {@code extraValidation} above, diff --git a/fleetd/src/main/java/dev/ltms/fleet/msg/LeadHeartbeatLoop.java b/fleetd/src/main/java/dev/ltms/fleet/msg/LeadHeartbeatLoop.java index 2636d69..c40f31d 100644 --- a/fleetd/src/main/java/dev/ltms/fleet/msg/LeadHeartbeatLoop.java +++ b/fleetd/src/main/java/dev/ltms/fleet/msg/LeadHeartbeatLoop.java @@ -271,8 +271,13 @@ public final class LeadHeartbeatLoop { * does not evaluate the lead's idle state before the fleet has settled. */ public void start() { - log.info("idle-lead heartbeat: on — nudge lead after {}s idle (recheck {}ms, quiet cap {})", - TimeUnit.NANOSECONDS.toSeconds(idleAfterNanos), backoffMs, quietNudgeCap); + // fleetd #613: contextHighNudge added alongside the three settings already here — an + // operator otherwise cannot tell from the boot log whether the #609 handover notice is + // armed, and had to load the deployed jar's config to confirm it. + log.info("idle-lead heartbeat: on — nudge lead after {}s idle (recheck {}ms, quiet cap {}, " + + "context-high nudge {})", + TimeUnit.NANOSECONDS.toSeconds(idleAfterNanos), backoffMs, quietNudgeCap, + contextHighNudge); scheduler.schedule(this::tick, backoffMs, TimeUnit.MILLISECONDS); } diff --git a/fleetd/src/test/java/dev/ltms/fleet/RoleFallbackGapReportTest.java b/fleetd/src/test/java/dev/ltms/fleet/RoleFallbackGapReportTest.java new file mode 100644 index 0000000..021ffa8 --- /dev/null +++ b/fleetd/src/test/java/dev/ltms/fleet/RoleFallbackGapReportTest.java @@ -0,0 +1,252 @@ +package dev.ltms.fleet; + +import ch.qos.logback.classic.Level; +import ch.qos.logback.classic.Logger; +import ch.qos.logback.classic.spi.ILoggingEvent; +import ch.qos.logback.core.read.ListAppender; +import dev.ltms.fleet.config.FleetConfig; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.io.TempDir; +import org.slf4j.LoggerFactory; + +import java.nio.file.Files; +import java.nio.file.Path; +import java.util.List; + +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertTrue; + +/** + * fleetd #613: a {@code MemberRole} with no {@code fleet.s:} pool falls back to + * every configured profile ({@code FleetConfig#candidateProfiles}), and one with no + * {@code fleet.charters.:} entry runs with only the launcher's reply charter. Both are + * deliberate, legitimate states — neither is refused by {@code validateMembers()} — but both were + * silent at boot. On the host that opened this ticket, an unqualified {@code hunter} spawn silently + * widened to all 8 configured profiles and its resolved first choice was {@code local}, a profile + * every other pool on that same config gave weight 0 to. + * + *

{@link Fleetd#reportRoleFallbackGaps} must name every gapped role, and for a pool gap, the + * exact resolved first-choice profile — that number, not the pool size, is what actually surprised + * the operator. Mirrors {@link ExhaustedPatternGapReportTest}'s pattern: capture the real log via a + * {@link ListAppender} rather than asserting on the call site's source text. + */ +class RoleFallbackGapReportTest { + + private static FleetConfig load(Path dir, String yaml) throws Exception { + Path f = dir.resolve("fleetd.yaml"); + Files.writeString(f, yaml); + return FleetConfig.load(f); + } + + /** + * The level this logger had before {@link #attach()} raised it, so {@link #detach} can put it + * back. {@code null} means "inherit from the parent" — the state this logger starts in. + */ + private static Level originalLevel; + + /** + * {@code reportRoleFallbackGaps} logs at INFO, and {@code logback-test.xml} sets + * {@code dev.ltms.fleet} to WARN — so INFO events are dropped by the level check before any + * appender sees them. Raise the level for the duration of the test, exactly like {@code + * GitHostShapeReportTest#attach}. + */ + private static ListAppender attach() { + Logger logger = (Logger) LoggerFactory.getLogger(Fleetd.class); + originalLevel = logger.getLevel(); + logger.setLevel(Level.INFO); + ListAppender appender = new ListAppender<>(); + appender.start(); + logger.addAppender(appender); + return appender; + } + + private static void detach(ListAppender appender) { + Logger logger = (Logger) LoggerFactory.getLogger(Fleetd.class); + logger.detachAppender(appender); + logger.setLevel(originalLevel); + } + + private static List infoMessages(ListAppender appender) { + return appender.list.stream() + .filter(e -> e.getLevel() == Level.INFO) + .map(ILoggingEvent::getFormattedMessage) + .toList(); + } + + /** + * Reproduces the shape measured in the ticket: {@code dev}, {@code reviewer} and + * {@code architect} each have a pool and a charter; {@code hunter} has neither. The pool-gap + * line must name {@code hunter}, the profile count (3), and the resolved first choice + * ({@code local}, the first profile in definition order) — and must not name the three healthy + * roles. The charter-gap line must separately name only {@code hunter}. + */ + @Test + void hunterWithNoPoolOrCharterIsNamedWithItsResolvedFirstChoice(@TempDir Path dir) throws Exception { + FleetConfig cfg = load(dir, """ + profiles: + local: + baseUrl: http://gx00.gw:8000 + sonnet: + baseUrl: https://llm.ltms.dev/v1 + terra: + baseUrl: https://llm.ltms.dev/v1 + fleet: + developers: + a: + profile: sonnet + reviewers: + b: + profile: terra + architects: + c: + profile: sonnet + charters: + dev: "dev charter text" + reviewer: "reviewer charter text" + architect: "architect charter text" + """); + + ListAppender appender = attach(); + try { + Fleetd.reportRoleFallbackGaps(cfg); + } finally { + detach(appender); + } + + List infos = infoMessages(appender); + + String poolLine = infos.stream() + .filter(m -> m.contains("no fleet.s: pool")) + .findFirst() + .orElseThrow(() -> new AssertionError("expected a pool-gap INFO line: " + infos)); + assertTrue(poolLine.contains("hunter"), poolLine); + assertTrue(poolLine.contains("3"), "must name the profile count the gap opens onto: " + poolLine); + assertTrue(poolLine.contains("'local'"), + "must name the resolved first-choice profile, the number that actually surprised " + + "the operator: " + poolLine); + for (String healthy : List.of("dev", "reviewer", "architect")) { + assertFalse(poolLine.contains(healthy), + "pool-gap line must not name a role that has a pool: " + poolLine); + } + + String charterLine = infos.stream() + .filter(m -> m.contains("no fleet.charters: entry")) + .findFirst() + .orElseThrow(() -> new AssertionError("expected a charter-gap INFO line: " + infos)); + assertTrue(charterLine.contains("hunter"), charterLine); + for (String healthy : List.of("dev", "reviewer", "architect")) { + assertFalse(charterLine.contains(healthy), + "charter-gap line must not name a role that has a charter: " + charterLine); + } + } + + /** + * The resolved first choice must be genuinely computed from definition order, not hardcoded — + * reordering {@code profiles:} so a different entry comes first changes the reported choice. + */ + @Test + void theResolvedFirstChoiceFollowsProfileDefinitionOrder(@TempDir Path dir) throws Exception { + FleetConfig cfg = load(dir, """ + profiles: + sonnet: + baseUrl: https://llm.ltms.dev/v1 + local: + baseUrl: http://gx00.gw:8000 + fleet: + developers: + a: + profile: sonnet + charters: + dev: "dev charter text" + """); + + ListAppender appender = attach(); + try { + Fleetd.reportRoleFallbackGaps(cfg); + } finally { + detach(appender); + } + + String poolLine = infoMessages(appender).stream() + .filter(m -> m.contains("no fleet.s: pool")) + .findFirst() + .orElseThrow(); + // hunter, reviewer and architect all lack a pool here; each falls back to the full 2-profile + // set and the first-choice is 'sonnet' because it is first in profiles: definition order. + assertTrue(poolLine.contains("'sonnet'"), poolLine); + assertFalse(poolLine.contains("'local'"), poolLine); + } + + /** A config with a pool and a charter for every role produces no role-fallback log at all. */ + @Test + void everyRoleWithAPoolAndACharterProducesNoLogAtAll(@TempDir Path dir) throws Exception { + FleetConfig cfg = load(dir, """ + profiles: + sonnet: + baseUrl: https://llm.ltms.dev/v1 + fleet: + developers: + a: + profile: sonnet + reviewers: + b: + profile: sonnet + hunters: + c: + profile: sonnet + architects: + d: + profile: sonnet + charters: + dev: "dev charter text" + reviewer: "reviewer charter text" + hunter: "hunter charter text" + architect: "architect charter text" + """); + + ListAppender appender = attach(); + try { + Fleetd.reportRoleFallbackGaps(cfg); + } finally { + detach(appender); + } + + assertTrue(appender.list.isEmpty(), + "a config with no gaps must not print a per-role block: " + infoMessages(appender)); + } + + /** + * A config with only {@code profiles:} and no {@code fleet:} block at all must still be + * reported (every role is gapped, both pool and charter) rather than throwing — this is the + * exact shape {@code candidateProfiles}' fallback exists to keep starting. + */ + @Test + void aConfigWithNoFleetBlockAtAllReportsEveryRoleGapped(@TempDir Path dir) throws Exception { + FleetConfig cfg = load(dir, """ + profiles: + sonnet: + baseUrl: https://llm.ltms.dev/v1 + """); + + ListAppender appender = attach(); + try { + Fleetd.reportRoleFallbackGaps(cfg); + } finally { + detach(appender); + } + + List infos = infoMessages(appender); + String poolLine = infos.stream() + .filter(m -> m.contains("no fleet.s: pool")) + .findFirst() + .orElseThrow(() -> new AssertionError("expected a pool-gap INFO line: " + infos)); + String charterLine = infos.stream() + .filter(m -> m.contains("no fleet.charters: entry")) + .findFirst() + .orElseThrow(() -> new AssertionError("expected a charter-gap INFO line: " + infos)); + for (String role : List.of("dev", "hunter", "reviewer", "architect")) { + assertTrue(poolLine.contains(role), poolLine); + assertTrue(charterLine.contains(role), charterLine); + } + } +} diff --git a/fleetd/src/test/java/dev/ltms/fleet/msg/LeadHeartbeatLoopTest.java b/fleetd/src/test/java/dev/ltms/fleet/msg/LeadHeartbeatLoopTest.java index d327861..9f48af9 100644 --- a/fleetd/src/test/java/dev/ltms/fleet/msg/LeadHeartbeatLoopTest.java +++ b/fleetd/src/test/java/dev/ltms/fleet/msg/LeadHeartbeatLoopTest.java @@ -1,5 +1,9 @@ package dev.ltms.fleet.msg; +import ch.qos.logback.classic.Level; +import ch.qos.logback.classic.Logger; +import ch.qos.logback.classic.spi.ILoggingEvent; +import ch.qos.logback.core.read.ListAppender; import com.fasterxml.jackson.databind.JsonNode; import com.fasterxml.jackson.databind.ObjectMapper; import dev.ltms.fleet.herdr.AgentControl; @@ -12,6 +16,7 @@ import dev.ltms.fleet.session.MemberSession; import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; +import org.slf4j.LoggerFactory; import java.util.ArrayList; import java.util.List; @@ -107,6 +112,58 @@ class LeadHeartbeatLoopTest { "never inject into a state the loop cannot read"); } + // ── fleetd #613: the boot line names all four heartbeat settings ────────────────────────── + + /** + * fleetd #613: {@link LeadHeartbeatLoop#start()}'s boot line named only 3 of the 4 constructor + * settings — {@code contextHighNudge} (fleetd #609) was missing, so an operator could not tell + * from the log whether the handover notice was armed. Captures the real log via a + * {@link ListAppender}, raising the logger's level past {@code logback-test.xml}'s + * {@code dev.ltms.fleet -> WARN} override for the duration of the call — the same seam {@code + * GitHostShapeReportTest#attach} uses for its own INFO-level boot line. + */ + private static String heartbeatBootLine(boolean contextHighNudge, ScheduledExecutorService scheduler) { + Logger logger = (Logger) LoggerFactory.getLogger(LeadHeartbeatLoop.class); + Level originalLevel = logger.getLevel(); + logger.setLevel(Level.INFO); + ListAppender appender = new ListAppender<>(); + appender.start(); + logger.addAppender(appender); + try { + LeadHeartbeatLoop l = new LeadHeartbeatLoop( + new PrimaryRegistry("term_lead"), null /*agents*/, null /*inbox*/, List::of, + null /*pushLoop*/, scheduler, () -> 0L, IDLE_AFTER_NANOS, 1_000L, 3, null, + LeadHeartbeatLoop.LeadContextSource.none(), contextHighNudge); + l.start(); + } finally { + logger.detachAppender(appender); + logger.setLevel(originalLevel); + } + return appender.list.stream() + .filter(e -> e.getLevel() == Level.INFO) + .map(ILoggingEvent::getFormattedMessage) + .filter(m -> m.startsWith("idle-lead heartbeat:")) + .findFirst() + .orElseThrow(() -> new AssertionError("expected the heartbeat boot line to be logged")); + } + + @Test + void theBootLineNamesContextHighNudgeWhenArmed() { + String line = heartbeatBootLine(true, scheduler); + assertTrue(line.contains("300s idle"), line); + assertTrue(line.contains("recheck 1000ms"), line); + assertTrue(line.contains("quiet cap 3"), line); + assertTrue(line.contains("context-high nudge true"), + "the boot line must name the 4th setting, contextHighNudge, alongside the other " + + "three: " + line); + } + + @Test + void theBootLineNamesContextHighNudgeWhenOff() { + String line = heartbeatBootLine(false, scheduler); + assertTrue(line.contains("context-high nudge false"), line); + } + // ── (b) an idle lead within the quiet period is not yet injected ─────────────────────────── @Test