diff --git a/fleetd/src/main/java/dev/ltms/fleet/config/ConfigRef.java b/fleetd/src/main/java/dev/ltms/fleet/config/ConfigRef.java index f3e8b5c..7b490fb 100644 --- a/fleetd/src/main/java/dev/ltms/fleet/config/ConfigRef.java +++ b/fleetd/src/main/java/dev/ltms/fleet/config/ConfigRef.java @@ -42,7 +42,17 @@ import java.util.function.Supplier; * {@code guard:}, {@code worktreeRoot:} and {@code worktreeGroup:} (both baked once into the * {@code GitWorktrees} built at {@code Fleetd.java:251} and never rebuilt — fleetd #323 * instance 2 found {@code worktreeGroup} missing from this list and from - * {@link #changedDeferredKeys}), adding or removing a profile (a new backend needs its own launcher, + * {@link #changedDeferredKeys}), {@code primary:} (fleetd #326 — {@code Fleetd.java:506, 519, + * 520} read {@code cfg.primary()} only off the startup snapshot to build {@code + * PrimaryRegistry} and size {@code ReplyPushLoop}'s reminder cap/backoff, and neither is + * rebuilt on reload; a lead whose pinned terminal changed under a running daemon stays + * unresolved as primary until a restart), {@code configReload:} (fleetd #326 — {@code + * Fleetd.java:679-680} read it only at startup to decide whether to build a {@code + * ConfigWatcher} at all and with what interval; the watcher that would apply a later change is + * itself built once, so a running watcher keeps polling on its original enabled flag and + * interval regardless of what a reload changes it to, the same shape as {@code lifecycle} — + * not cold, because no already-open resource goes inconsistent with the new value, the watcher + * (if any) simply keeps its old settings), adding or removing a profile (a new backend needs its own launcher, * which is constructed once), and an existing profile's launch settings — * {@code model}, {@code baseUrl}, {@code argv}, {@code env}, {@code mcpUrl}, * {@code exhaustedPattern} (CB-578 stage A — compiled once into {@code Fleetd.main}'s @@ -234,6 +244,20 @@ public final class ConfigRef implements Supplier { if (!Objects.equals(old.worktreeGroup(), fresh.worktreeGroup())) { changed.add("worktreeGroup"); } + // fleetd #326: Fleetd.java:506, 519, 520 read cfg.primary() only off the startup snapshot + // (PrimaryRegistry's pinned terminal, ReplyPushLoop's reminder cap and backoff) — neither is + // rebuilt on reload, so a changed pin needs a restart before a lead resolves as primary again. + if (!Objects.equals(old.primary(), fresh.primary())) { + changed.add("primary"); + } + // fleetd #326: Fleetd.java:679-680 read cfg.configReload() only at startup to decide whether + // to build a ConfigWatcher at all and with what interval — the watcher that would apply a + // later change is itself built once, so a running watcher keeps its original enabled flag and + // interval regardless of what a reload changes it to. Not cold: no already-open resource goes + // inconsistent with the new value, a watcher (if any) simply keeps polling on the old settings. + if (!Objects.equals(old.configReload(), fresh.configReload())) { + changed.add("configReload"); + } if (!Objects.equals(old.spawnReadyTimeoutMs(), fresh.spawnReadyTimeoutMs()) || !Objects.equals(old.spawnReadyPollMs(), fresh.spawnReadyPollMs())) { changed.add("spawnReady*"); diff --git a/fleetd/src/test/java/dev/ltms/fleet/config/ConfigRefTest.java b/fleetd/src/test/java/dev/ltms/fleet/config/ConfigRefTest.java index 2de9750..853cb0e 100644 --- a/fleetd/src/test/java/dev/ltms/fleet/config/ConfigRefTest.java +++ b/fleetd/src/test/java/dev/ltms/fleet/config/ConfigRefTest.java @@ -506,6 +506,70 @@ class ConfigRefTest { assertEquals("devgroup2", ref.get().worktreeGroup()); } + /** + * fleetd #326: {@code primary} is read only off the startup snapshot — {@code Fleetd.java:506, + * 519, 520} feed {@code PrimaryRegistry} and {@code ReplyPushLoop} at construction and neither is + * rebuilt on reload — but it was missing from {@link ConfigRef#changedDeferredKeys}, so a reload + * that only changed the pinned primary terminal reported a bare "config reloaded" while a lead + * whose tab no longer matched stayed demoted to worker. + */ + @Test + void changingPrimaryIsReportedAsDeferred(@TempDir Path dir) throws Exception { + Path f = dir.resolve("fleetd.yaml"); + Files.writeString(f, yaml(""" + primary: + terminal: term-a + """)); + ConfigRef ref = refFor(f); + + Files.writeString(f, yaml(""" + primary: + terminal: term-b + """)); + ConfigRef.Outcome out = ref.reload(); + + assertTrue(out.applied()); + assertEquals(java.util.List.of("primary"), out.deferred()); + assertTrue(out.summary().contains("need") && out.summary().contains("restart"), out.summary()); + // The snapshot still carries the new value — a restart is what makes it take effect. + assertEquals("term-b", ref.get().primary().terminal()); + } + + /** + * fleetd #326: {@code configReload} itself is read only at startup ({@code Fleetd.java:679-680}) + * to decide whether to build a {@code ConfigWatcher} at all, and with what interval — the watcher + * that would apply a later change is itself built once, so it is deferred rather than cold (see + * {@link ConfigRef}'s class doc: cold means an already-open resource would go inconsistent with + * the new value, and there is no such resource here — a running watcher just keeps polling on its + * original enabled/interval until a restart, exactly like {@code lifecycle} or {@code guard}). + * Before this fix, turning reload off (or changing its interval) through a reload reported a bare + * "config reloaded" — the obvious joke the issue names. + */ + @Test + void changingConfigReloadIsReportedAsDeferred(@TempDir Path dir) throws Exception { + Path f = dir.resolve("fleetd.yaml"); + Files.writeString(f, yaml(""" + configReload: + enabled: true + intervalSeconds: 10 + """)); + ConfigRef ref = refFor(f); + + Files.writeString(f, yaml(""" + configReload: + enabled: false + intervalSeconds: 30 + """)); + ConfigRef.Outcome out = ref.reload(); + + assertTrue(out.applied()); + assertEquals(java.util.List.of("configReload"), out.deferred()); + assertTrue(out.summary().contains("need") && out.summary().contains("restart"), out.summary()); + // The snapshot still carries the new value — a restart is what makes it take effect. + assertFalse(ref.get().configReload().isEnabled()); + assertEquals(30, ref.get().configReload().intervalSeconds()); + } + @Test void aFixedRefHasNoFileAndRefusesToReload() { FleetConfig cfg = new FleetConfig(null, null, null, null, null, null,