diff --git a/bridged/bridged.example.yaml b/bridged/bridged.example.yaml index 49a88dc..9e18d9f 100644 --- a/bridged/bridged.example.yaml +++ b/bridged/bridged.example.yaml @@ -223,6 +223,34 @@ profiles: # round-robin, or weighted. Omitting this key is a strict no-op for existing configs. placement: weighted +# Re-read this file without restarting the daemon (CB-559). Off unless you add this block, so an +# upgraded bridged keeps the old behaviour: the file is read once at boot and never again. +# enabled → turn the watch on. bridged checks the file's modified time on a timer and +# reloads when it moves. +# intervalSeconds → how often to check (default 10). One `stat` per tick, so this is cheap. +# +# Not every key can move under a running daemon, and the difference is about what already exists +# when the reload happens — not about how important the key is: +# HOT → takes effect on the next spawn: the whole `fleet:` block (every role pool and +# `tabLabel`), `placement:`, and an existing profile's weight / maxLoad / model / +# tabLabel. +# DEFERRED → accepted into the new config, but the wiring built at startup keeps the old value +# until you restart: `lifecycle:`, `leadHeartbeat:`, `guard:`, `worktreeRoot:`, +# `spawnReadyTimeoutMs` / `spawnReadyPollMs`, and ADDING or REMOVING a profile (a new +# backend needs its own launcher, and launchers are built once). The reload logs +# these by name rather than pretending they applied. +# COLD → cannot change at all: `bind:`, `herdrSocket:`, `broker:` and `auth:`. The socket is +# bound, the broker connection is open, and the auth mode decides who may reach the +# port that is already listening. +# +# A changed COLD key refuses the WHOLE reload — not the hot half applied and the cold half warned +# about. A half-applied reload would leave the daemon matching no file on disk, which is the worst +# thing a reload can do to an operator debugging one. A file that fails to parse or fails a startup +# validator is refused the same way, and the running config stays live. +# configReload: +# enabled: true +# intervalSeconds: 10 + # THE FLEET (CB-557) — who the daemon may run, and under which role. This one block replaced four # older keys: `leaders:`, `members:`, `leadScan:` and `defaultProfile:`. # diff --git a/bridged/src/main/java/dev/ltms/bridged/Bridged.java b/bridged/src/main/java/dev/ltms/bridged/Bridged.java index 31678a9..29ea236 100644 --- a/bridged/src/main/java/dev/ltms/bridged/Bridged.java +++ b/bridged/src/main/java/dev/ltms/bridged/Bridged.java @@ -1,6 +1,8 @@ package dev.ltms.bridged; import dev.ltms.bridged.config.BridgedConfig; +import dev.ltms.bridged.config.ConfigRef; +import dev.ltms.bridged.config.ConfigWatcher; import dev.ltms.bridged.guard.SubscriptionGuard; import dev.ltms.bridged.herdr.AgentControl; import dev.ltms.bridged.herdr.HerdrClient; @@ -37,7 +39,6 @@ import dev.ltms.bridged.session.SessionManager; import dev.ltms.bridged.peer.PeerLauncher; import dev.ltms.bridged.session.SessionReaper; import dev.ltms.bridged.member.ClaudeCodeLauncher; -import dev.ltms.bridged.placement.PlacementPolicies; import dev.ltms.bridged.member.CompositePeerLauncher; import dev.ltms.bridged.member.HerdrPeerLauncher; import dev.ltms.bridged.member.OpenCodeLauncher; @@ -79,6 +80,11 @@ public final class Bridged { static void main(String[] args) { Path configPath = Path.of(args.length > 0 ? args[0] : "bridged.yaml"); BridgedConfig cfg = BridgedConfig.load(configPath); + // CB-559: `cfg` stays the startup snapshot — every validation and every piece of one-time + // wiring below reads it, and must, because those decisions cannot be unmade. `config` is the + // live reference the hot paths read per use. Which keys can actually move is ConfigRef's + // contract; adding a reader here does not make a key reloadable by itself. + ConfigRef config = new ConfigRef(configPath, cfg); // The primary/host env that launched bridged must not be tainted. SubscriptionGuard guard = new SubscriptionGuard(cfg.guard().hostSet()); @@ -125,22 +131,20 @@ public final class Bridged { adapters.add(new ClaudeCodeLauncher(agents, spaces, guard, claudeProfiles, cfg.effectiveDefaultProfile(), System::getenv, cfg.spawnReadyTimeoutMs(), cfg.spawnReadyPollMs(), - cfg.fleet().tabLabel())); + () -> config.get().fleet().tabLabel())); } if (!opencodeProfiles.isEmpty()) { adapters.add(new OpenCodeLauncher(agents, spaces, opencodeProfiles, cfg.effectiveDefaultProfile(), System::getenv, cfg.spawnReadyTimeoutMs(), cfg.spawnReadyPollMs(), - cfg.fleet().tabLabel())); + () -> config.get().fleet().tabLabel())); } AtomicReference> liveCountRef = new AtomicReference<>(_ -> 0); PeerLauncher workers = new CompositePeerLauncher( adapters, cfg.effectiveDefaultProfile(), - cfg.profiles(), - PlacementPolicies.fromName(cfg.placement()), - profileName -> liveCountRef.get().apply(profileName), - cfg.fleet()); + config, + profileName -> liveCountRef.get().apply(profileName)); // CB-504: under supervision (launchd/systemd) bridged 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 @@ -384,6 +388,16 @@ public final class Bridged { BridgeMcp mcp = new BridgeMcp(messages, workers, sessions, identity, presence, primaryRegistry, callers, metrics); + // CB-559: opt-in config reload. With no `configReload:` block nothing is constructed, so an + // upgraded daemon behaves exactly as before — the file is read once at boot and never again. + final ConfigWatcher configWatcher; + if (cfg.configReload() != null && cfg.configReload().isEnabled()) { + configWatcher = new ConfigWatcher(config, cfg.configReload().intervalSeconds()); + configWatcher.start(); + } else { + configWatcher = null; + } + // CB-303 part 3: single ordered shutdown hook. Drain sessions first while herdr is still // open (so releases reach the daemon), then stop poller/message/mcp/reaper, and close herdr // last. This replaces the earlier independent hooks that could race and close herdr early. @@ -393,6 +407,7 @@ public final class Bridged { messages.close(); pushLoop.close(); if (heartbeat != null) heartbeat.close(); // CB-551: stop the idle-lead heartbeat scheduler + if (configWatcher != null) configWatcher.stop(); // CB-559: stop polling the config file mcp.close(); if (reaper != null) reaper.stop(); // Release the broker connection last among message resources (no-op for the in-memory inbox). diff --git a/bridged/src/main/java/dev/ltms/bridged/config/BridgedConfig.java b/bridged/src/main/java/dev/ltms/bridged/config/BridgedConfig.java index b513bc8..236c675 100644 --- a/bridged/src/main/java/dev/ltms/bridged/config/BridgedConfig.java +++ b/bridged/src/main/java/dev/ltms/bridged/config/BridgedConfig.java @@ -74,7 +74,17 @@ public record BridgedConfig( Fleet fleet, LeadHeartbeat leadHeartbeat, String placement, - Auth auth) { + Auth auth, + ConfigReload configReload) { + + /** Back-compat 14-arg form — no {@code configReload:} block, so file watching stays off. */ + public BridgedConfig(Bind bind, String herdrSocket, Map profiles, Guard guard, + String worktreeRoot, Lifecycle lifecycle, Integer spawnReadyTimeoutMs, + Integer spawnReadyPollMs, Broker broker, Primary primary, Fleet fleet, + LeadHeartbeat leadHeartbeat, String placement, Auth auth) { + this(bind, herdrSocket, profiles, guard, worktreeRoot, lifecycle, spawnReadyTimeoutMs, + spawnReadyPollMs, broker, primary, fleet, leadHeartbeat, placement, auth, null); + } /** * Normalize {@code profiles} once, at construction, so every reader sees the same map. @@ -622,6 +632,31 @@ public record BridgedConfig( } } + /** + * Watch {@code bridged.yaml} and re-read it when it changes (CB-559). + * + *

Opt-in, like every other block that acts on its own initiative. A daemon that reloads + * whenever a file is saved would apply a half-finished edit the moment an editor writes it, and + * an operator who did not ask for that has no reason to expect it. Absent block = off, and the + * config is read exactly once at startup as it always was. + * + *

Which keys a reload can actually change — and which refuse it — is + * {@link ConfigRef}'s contract, not this block's. This only decides when to look. + * + * @param enabled false (or an absent block) leaves the startup-only behaviour + * @param intervalSeconds how often the file's modified time is checked; defaults to 10 + */ + public record ConfigReload(Boolean enabled, Integer intervalSeconds) { + public ConfigReload { + enabled = enabled != null && enabled; + intervalSeconds = (intervalSeconds == null || intervalSeconds <= 0) ? 10 : intervalSeconds; + } + + public boolean isEnabled() { + return Boolean.TRUE.equals(enabled); + } + } + /** * The terminal → lead-name map that {@link dev.ltms.bridged.auth.CallerResolver} resolves * against, merging the {@code leaders:} registry with the legacy singular {@code primary:} pin. @@ -749,7 +784,7 @@ public record BridgedConfig( private static final Set KNOWN_TOP_LEVEL_KEYS = Set.of( "bind", "herdrSocket", "profiles", "guard", "worktreeRoot", "lifecycle", "spawnReadyTimeoutMs", "spawnReadyPollMs", "broker", "primary", "fleet", - "leadHeartbeat", "placement", "auth"); + "leadHeartbeat", "placement", "auth", "configReload"); /** Load and validate config from {@code path}. */ public static BridgedConfig load(Path path) { @@ -1015,8 +1050,11 @@ public record BridgedConfig( // leadHeartbeat is left as-is (CB-551): null is "off", and LeadHeartbeat's own compact // constructor defaults the fields of a block that IS present. Defaulting it here would // switch the feature on for every config that never mentioned it. + // configReload is left as-is: null is "off", and ConfigReload's own compact constructor + // defaults the fields of a block that IS present. Defaulting it here would start watching + // the file for every config that never asked to be watched. return new BridgedConfig(b, herdrSocket, profiles, g, worktreeRoot, l, timeout, pollMs, - broker, primary, f, leadHeartbeat, placementOrDefault, a); + broker, primary, f, leadHeartbeat, placementOrDefault, a, configReload); } /** diff --git a/bridged/src/main/java/dev/ltms/bridged/config/ConfigRef.java b/bridged/src/main/java/dev/ltms/bridged/config/ConfigRef.java new file mode 100644 index 0000000..1750234 --- /dev/null +++ b/bridged/src/main/java/dev/ltms/bridged/config/ConfigRef.java @@ -0,0 +1,219 @@ +package dev.ltms.bridged.config; + +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +import java.nio.file.Path; +import java.util.ArrayList; +import java.util.LinkedHashSet; +import java.util.List; +import java.util.Objects; +import java.util.Set; +import java.util.concurrent.atomic.AtomicReference; +import java.util.function.Supplier; + +/** + * The daemon's live configuration, re-readable without a restart (CB-559). + * + *

Consumers hold this, not a {@link BridgedConfig}, and read through {@link #get()} at the point + * of use. A component that captures {@code ref.get()} into a field at construction has opted out of + * reload — which is sometimes right (see deferred below), but it must then be a deliberate + * choice rather than an accident of where the field was initialised. + * + *

Not every key can change under a running daemon

+ * Keys fall into three classes, and the difference is about what already exists when the reload + * happens — not about how important the key is. + * + * + * + *

A cold change refuses the whole reload. Not the hot half applied and the cold + * half warned about: that would leave the running daemon in a state matching no file on disk, which + * is the worst thing a reload can do to an operator debugging one. Refusing keeps the invariant that + * the live config is always some version of the file, and the message names the keys that must + * change through a restart. + * + *

A reload that fails to parse or fails validation is also refused, and the previous config keeps + * running. A config file being edited is normally read once mid-save; degrading a working daemon + * because it caught a half-written file would be a bad trade. + */ +public final class ConfigRef implements Supplier { + + private static final Logger log = LoggerFactory.getLogger(ConfigRef.class); + + /** Keys that cannot change under a running daemon — see the class doc. */ + private static final Set COLD_KEYS = + Set.of("bind", "herdrSocket", "broker", "auth"); + + private final Path path; + private final AtomicReference current; + + public ConfigRef(Path path, BridgedConfig initial) { + this.path = path; + this.current = new AtomicReference<>(Objects.requireNonNull(initial, "initial config")); + } + + /** A fixed reference that never reloads — for tests and for wiring built from a config in code. */ + public static ConfigRef fixed(BridgedConfig cfg) { + return new ConfigRef(null, cfg); + } + + /** The live configuration. Read this per use; do not cache it in a field. */ + @Override + public BridgedConfig get() { + return current.get(); + } + + /** The file this ref reloads from, or {@code null} for a {@link #fixed} ref. */ + public Path path() { + return path; + } + + /** + * What a reload attempt did. + * + * @param applied true when the new config is now live + * @param coldKeys cold keys whose value changed, which is why an unapplied reload was refused + * @param deferred keys that changed and were accepted, but whose effect waits for a restart + * @param error the parse or validation failure that refused the reload, else {@code null} + */ + public record Outcome(boolean applied, List coldKeys, List deferred, + String error) { + + public Outcome { + coldKeys = List.copyOf(coldKeys); + deferred = List.copyOf(deferred); + } + + static Outcome refusedCold(List keys) { + return new Outcome(false, keys, List.of(), null); + } + + static Outcome failed(String error) { + return new Outcome(false, List.of(), List.of(), error); + } + + /** A one-line summary for the operator — the reason, not just the verdict. */ + public String summary() { + if (error != null) { + return "config reload refused — " + error; + } + if (!applied) { + return "config reload refused — these keys cannot change under a running daemon: " + + String.join(", ", coldKeys) + ". Restart bridged to apply them."; + } + if (!deferred.isEmpty()) { + return "config reloaded; these changes need a restart to take effect: " + + String.join(", ", deferred); + } + return "config reloaded"; + } + } + + /** + * Re-read the file, validate it, and swap it in when nothing cold changed. + * + *

Never throws: a reload is a best-effort operation on a daemon that is already serving, and + * a bad edit must not take it down. Every failure path leaves the previous config live and is + * reported through the returned {@link Outcome}. + */ + public Outcome reload() { + if (path == null) { + return Outcome.failed("this config was built in code and has no file to reload from"); + } + BridgedConfig old = current.get(); + BridgedConfig fresh; + try { + fresh = BridgedConfig.load(path); + // The same gate startup runs. A config that would have refused to boot must not be able + // to slip in through a reload — that is how a daemon ends up in a state it could never + // have started in, which is the hardest kind to debug. + fresh.validateAuthExposure(); + fresh.validateLeadTabPrefixes(); + fresh.validateSubscriptionProfiles(); + fresh.validateMembers(); + } catch (RuntimeException e) { + String msg = e.getMessage() == null ? e.toString() : e.getMessage(); + log.warn("config reload from {} refused, keeping the running config: {}", path, msg); + return Outcome.failed(msg); + } + + List cold = changedColdKeys(old, fresh); + if (!cold.isEmpty()) { + Outcome out = Outcome.refusedCold(cold); + log.warn(out.summary()); + return out; + } + + List deferred = changedDeferredKeys(old, fresh); + current.set(fresh); + Outcome out = new Outcome(true, List.of(), deferred, null); + log.info(out.summary()); + return out; + } + + /** Cold keys whose value differs between the running config and the candidate. */ + private static List changedColdKeys(BridgedConfig old, BridgedConfig fresh) { + List changed = new ArrayList<>(); + if (!Objects.equals(old.bind(), fresh.bind())) { + changed.add("bind"); + } + if (!Objects.equals(old.herdrSocket(), fresh.herdrSocket())) { + changed.add("herdrSocket"); + } + if (!Objects.equals(old.broker(), fresh.broker())) { + changed.add("broker"); + } + if (!Objects.equals(old.auth(), fresh.auth())) { + changed.add("auth"); + } + // Kept in step with COLD_KEYS so the doc and the code cannot drift apart silently. + assert COLD_KEYS.containsAll(changed) : "a cold key was reported that COLD_KEYS omits"; + return changed; + } + + /** Changed keys that were accepted but whose effect waits for a restart. */ + private static List changedDeferredKeys(BridgedConfig old, BridgedConfig fresh) { + List changed = new ArrayList<>(); + if (!Objects.equals(old.lifecycle(), fresh.lifecycle())) { + changed.add("lifecycle"); + } + if (!Objects.equals(old.leadHeartbeat(), fresh.leadHeartbeat())) { + changed.add("leadHeartbeat"); + } + if (!Objects.equals(old.guard(), fresh.guard())) { + changed.add("guard"); + } + if (!Objects.equals(old.worktreeRoot(), fresh.worktreeRoot())) { + changed.add("worktreeRoot"); + } + if (!Objects.equals(old.spawnReadyTimeoutMs(), fresh.spawnReadyTimeoutMs()) + || !Objects.equals(old.spawnReadyPollMs(), fresh.spawnReadyPollMs())) { + changed.add("spawnReady*"); + } + // Only the profile SET is deferred: a new backend needs a launcher, and launchers are built + // once at startup. An existing profile's fields are read per spawn and so are hot. + Set before = old.profiles() == null ? Set.of() : old.profiles().keySet(); + Set after = fresh.profiles() == null ? Set.of() : fresh.profiles().keySet(); + if (!before.equals(after)) { + Set diff = new LinkedHashSet<>(before); + diff.addAll(after); + diff.removeIf(p -> before.contains(p) && after.contains(p)); + changed.add("profiles (added/removed: " + String.join(", ", diff) + ")"); + } + return changed; + } +} diff --git a/bridged/src/main/java/dev/ltms/bridged/config/ConfigWatcher.java b/bridged/src/main/java/dev/ltms/bridged/config/ConfigWatcher.java new file mode 100644 index 0000000..7cde2d2 --- /dev/null +++ b/bridged/src/main/java/dev/ltms/bridged/config/ConfigWatcher.java @@ -0,0 +1,96 @@ +package dev.ltms.bridged.config; + +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +import java.io.IOException; +import java.nio.file.Files; +import java.nio.file.Path; +import java.util.concurrent.Executors; +import java.util.concurrent.ScheduledExecutorService; +import java.util.concurrent.TimeUnit; + +/** + * Polls {@code bridged.yaml}'s modified time and asks {@link ConfigRef} to reload when it moves + * (CB-559). Opt-in through {@code configReload.enabled}. + * + *

Why polling and not a filesystem watch. {@code WatchService} on macOS has no + * native backend — it falls back to polling internally anyway, at an interval this code does not + * control — and editors save config files in ways that produce a different event mix per editor + * (write-in-place, write-and-rename, write-temp-and-swap). A modified-time check treats all of them + * the same and is a single {@code stat} per tick, which at a ten-second cadence costs nothing worth + * measuring. + * + *

A missing or unreadable file is not a reason to act. Many editors briefly + * unlink the file during a save. Reloading on "it vanished" would mean reloading from a file that no + * longer exists; reporting an error every tick would bury the log. So an unreadable file is skipped + * silently and the next tick tries again — the running config stays live, which is the correct + * outcome either way. + */ +public final class ConfigWatcher { + + private static final Logger log = LoggerFactory.getLogger(ConfigWatcher.class); + + private final ConfigRef ref; + private final long intervalSeconds; + private final ScheduledExecutorService scheduler; + + private volatile long lastSeenMillis; + + public ConfigWatcher(ConfigRef ref, long intervalSeconds) { + this.ref = ref; + this.intervalSeconds = intervalSeconds; + this.lastSeenMillis = modifiedMillis(ref.path()); + this.scheduler = Executors.newSingleThreadScheduledExecutor(r -> { + Thread t = new Thread(r, "config-watcher"); + // A daemon thread: an operator's config watch must never be the reason the JVM refuses + // to exit after everything else has shut down. + t.setDaemon(true); + return t; + }); + } + + /** Begin watching. A ref with no file (a fixed one) is a no-op rather than an error. */ + public void start() { + if (ref.path() == null) { + log.debug("config watch not started — this config has no file behind it"); + return; + } + scheduler.scheduleWithFixedDelay(this::tick, intervalSeconds, intervalSeconds, + TimeUnit.SECONDS); + log.info("config watch: {} re-read when it changes (every {}s)", ref.path(), intervalSeconds); + } + + /** One poll. Never throws — an exception here would silently cancel the schedule. */ + void tick() { + try { + long now = modifiedMillis(ref.path()); + if (now == 0 || now == lastSeenMillis) { + return; + } + // Stamp BEFORE reloading. A file whose reload is refused (a bad edit, or a cold key) + // must not be retried every tick — that would log the same refusal forever. The next + // save moves the timestamp again and earns a fresh attempt. + lastSeenMillis = now; + ref.reload(); + } catch (RuntimeException e) { + log.warn("config watch tick failed, still watching: {}", e.getMessage()); + } + } + + private static long modifiedMillis(Path path) { + if (path == null) { + return 0; + } + try { + return Files.getLastModifiedTime(path).toMillis(); + } catch (IOException e) { + return 0; // mid-save, or gone: say nothing and try again next tick + } + } + + /** Stop polling. Called from the daemon's ordered shutdown hook, alongside the other loops. */ + public void stop() { + scheduler.shutdownNow(); + } +} diff --git a/bridged/src/main/java/dev/ltms/bridged/member/ClaudeCodeLauncher.java b/bridged/src/main/java/dev/ltms/bridged/member/ClaudeCodeLauncher.java index 1d66286..5d68d7f 100644 --- a/bridged/src/main/java/dev/ltms/bridged/member/ClaudeCodeLauncher.java +++ b/bridged/src/main/java/dev/ltms/bridged/member/ClaudeCodeLauncher.java @@ -16,6 +16,7 @@ import java.util.Set; import java.util.UUID; import java.util.function.Function; import java.util.function.LongSupplier; +import java.util.function.Supplier; /** * The {@link HerdrPeerLauncher} adapter for Claude Code — the safe path from a @@ -92,7 +93,7 @@ public final class ClaudeCodeLauncher extends HerdrPeerLauncher { Map profiles, String defaultProfile, Function env, long spawnReadyTimeoutMs, long spawnReadyPollMs, - String tabLabelTemplate) { + Supplier tabLabelTemplate) { this(agents, spaces, guard, profiles, defaultProfile, env, spawnReadyTimeoutMs, System::currentTimeMillis, () -> sleepUninterruptibly(spawnReadyPollMs), @@ -136,7 +137,7 @@ public final class ClaudeCodeLauncher extends HerdrPeerLauncher { Function env, long spawnReadyTimeoutMs, LongSupplier nowMillis, Runnable sleeper, - String tabLabelTemplate) { + Supplier tabLabelTemplate) { super(NAME_PREFIX, agents, spaces, profiles, defaultProfile, env, spawnReadyTimeoutMs, nowMillis, sleeper, tabLabelTemplate); this.guard = guard; diff --git a/bridged/src/main/java/dev/ltms/bridged/member/CompositePeerLauncher.java b/bridged/src/main/java/dev/ltms/bridged/member/CompositePeerLauncher.java index a8866d5..605e0c2 100644 --- a/bridged/src/main/java/dev/ltms/bridged/member/CompositePeerLauncher.java +++ b/bridged/src/main/java/dev/ltms/bridged/member/CompositePeerLauncher.java @@ -26,6 +26,7 @@ import java.util.Map; import java.util.Set; import java.util.concurrent.ConcurrentHashMap; import java.util.function.Function; +import java.util.function.Supplier; /** * The {@link PeerLauncher} the core actually holds when more than one adapter is configured — a thin @@ -65,16 +66,24 @@ public final class CompositePeerLauncher implements PeerLauncher { /** paneId → the delegate that spawned it, so {@link #stop} tears down through the right adapter. */ private final Map spawnedBy = new ConcurrentHashMap<>(); - private final Map profileConfigs; - private final PlacementPolicy placementPolicy; private final Function liveCount; /** - * CB-557: the role pools an unqualified spawn draws its candidates from. Nullable, and an empty - * pool for a role means "no pool configured" — both fall back to every configured profile, which - * is the pre-CB-557 behaviour. + * CB-559: the placement inputs are read per spawn, not captured at construction, so a + * config reload changes where the next member lands without a restart. These are the hot keys — + * role pools, an existing profile's weight/maxLoad, and the placement policy. What cannot change + * this way is the set of adapters ({@link #byProfile}), because a new backend needs a launcher + * and launchers are built once; {@code ConfigRef} classifies that as deferred and says so. */ - private final BridgedConfig.Fleet fleet; + private final Supplier> profileConfigs; + private final Supplier placementPolicy; + + /** + * CB-557: the role pools an unqualified spawn draws its candidates from. A supplier that yields + * {@code null}, and an empty pool for a role, both fall back to every configured profile — the + * pre-CB-557 behaviour. + */ + private final Supplier fleet; /** * Backward-compatible constructor: fixed placement, no live-counting. Use this for tests and @@ -121,16 +130,45 @@ public final class CompositePeerLauncher implements PeerLauncher { PlacementPolicy placementPolicy, Function liveCount, BridgedConfig.Fleet fleet) { + // LinkedHashMap, not Map.copyOf: candidates() promises definition order and the weighted + // policy breaks exact-weight ties on it, so a salted iteration order would make placement + // differ from one JVM run to the next. + this(delegates, defaultProfile, + constant(Collections.unmodifiableMap(new LinkedHashMap<>(profileConfigs))), + constant(placementPolicy), liveCount, constant(fleet)); + } + + /** + * Production constructor that re-reads its placement inputs per spawn (CB-559), so a config + * reload retargets the next member without a restart. + * + * @param config the live configuration — read at every spawn, never captured + */ + public CompositePeerLauncher(List delegates, + String defaultProfile, + Supplier config, + Function liveCount) { + this(delegates, defaultProfile, + () -> config.get().profiles(), + () -> PlacementPolicies.fromName(config.get().placement()), + liveCount, + () -> config.get().fleet()); + } + + /** The all-suppliers form every other constructor funnels into. */ + private CompositePeerLauncher(List delegates, + String defaultProfile, + Supplier> profileConfigs, + Supplier placementPolicy, + Function liveCount, + Supplier fleet) { this.fleet = fleet; if (delegates.isEmpty()) { throw new IllegalArgumentException("at least one peer adapter must be configured"); } this.delegates = List.copyOf(delegates); this.defaultProfile = defaultProfile; - // LinkedHashMap, not Map.copyOf: candidates() promises definition order and the weighted - // policy breaks exact-weight ties on it, so a salted iteration order would make placement - // differ from one JVM run to the next. - this.profileConfigs = Collections.unmodifiableMap(new LinkedHashMap<>(profileConfigs)); + this.profileConfigs = profileConfigs; this.placementPolicy = placementPolicy; this.liveCount = liveCount; Map index = new LinkedHashMap<>(); @@ -147,6 +185,22 @@ public final class CompositePeerLauncher implements PeerLauncher { this.byProfile = Collections.unmodifiableMap(index); } + /** A supplier of a value fixed at construction — how the non-reloading constructors funnel in. */ + private static Supplier constant(T value) { + return () -> value; + } + + /** + * The currently-configured profiles, never null. + * + *

Read fresh on every call so a reload is visible; a caller that needs two consistent reads + * takes one local, as {@link #poolFor} does. + */ + private Map profiles0() { + Map m = profileConfigs.get(); + return m == null ? Map.of() : m; + } + /** The adapter owning {@code profileName} (null/blank → the default). Throws on an unknown profile. */ private HerdrPeerLauncher route(String profileName) { String resolved = (profileName == null || profileName.isBlank()) ? defaultProfile : profileName; @@ -191,7 +245,7 @@ public final class CompositePeerLauncher implements PeerLauncher { // Deliberately uncaught: when no candidate is left (all at cap, or all unreachable) the // policy already throws a clear message. Catching it to rethrow a generic // PeerUnreachableException would replace a precise diagnosis with a vague one. - PlacementCandidate chosen = placementPolicy.select(ctx); + PlacementCandidate chosen = placementPolicy.get().select(ctx); HerdrPeerLauncher d = byProfile.get(chosen.profile()); if (d == null) { @@ -247,7 +301,7 @@ public final class CompositePeerLauncher implements PeerLauncher { private void enforceMaxLoad(String profile) { // Absent config, or a config whose maxLoad normalized to null (non-positive ⇒ unlimited at // load), means no cap — never cap what wasn't configured. - BridgedConfig.Profile cfg = profileConfigs.get(profile); + BridgedConfig.Profile cfg = profiles0().get(profile); Integer cap = (cfg == null) ? null : cfg.maxLoad(); if (cap == null) { return; @@ -268,9 +322,11 @@ public final class CompositePeerLauncher implements PeerLauncher { * a pool entry with no profile, so a survivor is a profile this particular composite does not own. */ private List poolFor(MemberRole role) { - List pool = (fleet == null) ? List.of() : fleet.profilesFor(role); - List known = pool.stream().filter(profileConfigs::containsKey).toList(); - return known.isEmpty() ? List.copyOf(profileConfigs.keySet()) : known; + Map configured = profiles0(); + BridgedConfig.Fleet f = fleet.get(); + List pool = (f == null) ? List.of() : f.profilesFor(role); + List known = pool.stream().filter(configured::containsKey).toList(); + return known.isEmpty() ? List.copyOf(configured.keySet()) : known; } /** The profile an unqualified spawn for {@code role} falls back to under {@code fixed} placement. */ @@ -283,7 +339,7 @@ public final class CompositePeerLauncher implements PeerLauncher { private List candidates(MemberRole role) { List out = new ArrayList<>(); for (String name : poolFor(role)) { - BridgedConfig.Profile w = profileConfigs.get(name); + BridgedConfig.Profile w = profiles0().get(name); if (w != null) { out.add(new PlacementCandidate(name, null, w.weight(), w.maxLoad())); } diff --git a/bridged/src/main/java/dev/ltms/bridged/member/HerdrPeerLauncher.java b/bridged/src/main/java/dev/ltms/bridged/member/HerdrPeerLauncher.java index 2ce7d9b..eb27d8a 100644 --- a/bridged/src/main/java/dev/ltms/bridged/member/HerdrPeerLauncher.java +++ b/bridged/src/main/java/dev/ltms/bridged/member/HerdrPeerLauncher.java @@ -29,6 +29,7 @@ import java.util.concurrent.atomic.AtomicBoolean; import java.util.concurrent.atomic.AtomicLong; import java.util.function.Function; import java.util.function.LongSupplier; +import java.util.function.Supplier; import java.util.regex.Matcher; import java.util.regex.Pattern; @@ -79,10 +80,15 @@ public abstract class HerdrPeerLauncher implements PeerLauncher { private final AtomicLong nameSeq = new AtomicLong(); // per-peer counter (herdr agent names only) /** - * The {@code fleet.tabLabel} template; {@code null} ⇒ {@link BridgedConfig.Fleet#DEFAULT_TAB_LABEL}. - * A profile's own {@code tabLabel} still overrides it. + * The {@code fleet.tabLabel} template; a {@code null} supplier or a {@code null}/blank value ⇒ + * {@link BridgedConfig.Fleet#DEFAULT_TAB_LABEL}. A profile's own {@code tabLabel} still + * overrides it. + * + *

CB-559: a supplier rather than a String, so a config reload renames the next tab + * without a restart. Existing tabs keep the label they were given — bridged does not rewrite a + * label it already wrote. */ - private final String tabLabelTemplate; + private final Supplier tabLabelTemplate; /** * Tab numbers, counted per {@code role/profile} pair (CB-557). @@ -138,7 +144,8 @@ public abstract class HerdrPeerLauncher implements PeerLauncher { /** * As above, plus the {@code fleet.tabLabel} template (CB-557). * - * @param tabLabelTemplate fleet-wide tab-label template; {@code null}/blank ⇒ + * @param tabLabelTemplate fleet-wide tab-label template, read per spawn (CB-559); {@code null}, + * or a supplier yielding {@code null}/blank ⇒ * {@link BridgedConfig.Fleet#DEFAULT_TAB_LABEL}. A separate constructor * rather than a new parameter on the one above, so every existing call * site keeps the default without an edit. @@ -148,7 +155,7 @@ public abstract class HerdrPeerLauncher implements PeerLauncher { Function env, long spawnReadyTimeoutMs, LongSupplier nowMillis, Runnable sleeper, - String tabLabelTemplate) { + Supplier tabLabelTemplate) { this.tabLabelTemplate = tabLabelTemplate; this.namePrefix = namePrefix; this.agents = agents; @@ -407,7 +414,9 @@ public abstract class HerdrPeerLauncher implements PeerLauncher { // we log and still return it so the caller gets its paneId and can tear it down. tidy("label tab " + tab.tab().tabId(), () -> spaces.renameTab(tab.tab().tabId(), - cfg.renderTabLabel(tabLabelTemplate, role, nextLabelSeq(role, cfg.profile())))); + cfg.renderTabLabel( + tabLabelTemplate == null ? null : tabLabelTemplate.get(), + role, nextLabelSeq(role, cfg.profile())))); log.info("{} started pane={} tab={} terminal={}", namePrefix, started.agent().paneId(), started.agent().tabId(), started.agent().terminalId()); return started.agent(); diff --git a/bridged/src/main/java/dev/ltms/bridged/member/OpenCodeLauncher.java b/bridged/src/main/java/dev/ltms/bridged/member/OpenCodeLauncher.java index cef1d9f..50527ad 100644 --- a/bridged/src/main/java/dev/ltms/bridged/member/OpenCodeLauncher.java +++ b/bridged/src/main/java/dev/ltms/bridged/member/OpenCodeLauncher.java @@ -20,6 +20,7 @@ import java.util.Map; import java.util.Set; import java.util.function.Function; import java.util.function.LongSupplier; +import java.util.function.Supplier; /** * The {@link HerdrPeerLauncher} adapter for opencode — an open-source, @@ -125,7 +126,7 @@ public final class OpenCodeLauncher extends HerdrPeerLauncher { Map profiles, String defaultProfile, Function env, long spawnReadyTimeoutMs, long spawnReadyPollMs, - String tabLabelTemplate) { + Supplier tabLabelTemplate) { this(agents, spaces, profiles, defaultProfile, env, spawnReadyTimeoutMs, System::currentTimeMillis, () -> sleepUninterruptibly(spawnReadyPollMs), defaultConfigRoot(), defaultDiscoveryRoot(), tabLabelTemplate); @@ -172,7 +173,7 @@ public final class OpenCodeLauncher extends HerdrPeerLauncher { long spawnReadyTimeoutMs, LongSupplier nowMillis, Runnable sleeper, Path configRoot, Path discoveryRoot, - String tabLabelTemplate) { + Supplier tabLabelTemplate) { super(NAME_PREFIX, agents, spaces, profiles, defaultProfile, env, spawnReadyTimeoutMs, nowMillis, sleeper, tabLabelTemplate); this.configRoot = configRoot; diff --git a/bridged/src/test/java/dev/ltms/bridged/config/ConfigRefTest.java b/bridged/src/test/java/dev/ltms/bridged/config/ConfigRefTest.java new file mode 100644 index 0000000..abba835 --- /dev/null +++ b/bridged/src/test/java/dev/ltms/bridged/config/ConfigRefTest.java @@ -0,0 +1,256 @@ +package dev.ltms.bridged.config; + +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.*; + +/** + * CB-559: re-reading {@code bridged.yaml} under a running daemon. + * + *

The tests that matter here are the refusals. A reload that applies a good file is the easy + * half; the half that protects an operator is the one that keeps the running config when the new + * file is bad, and the one that refuses a change the running daemon cannot honour. + */ +class ConfigRefTest { + + /** A minimal file that loads and passes every startup validator. */ + private static String yaml(String extra) { + return """ + bind: + host: 127.0.0.1 + port: 8765 + herdrSocket: ~/.config/herdr/herdr.sock + profiles: + sonnet: + baseUrl: http://gx00.gw:8000 + model: sonnet + guard: + offSubscriptionHosts: + - gx00.gw + """ + extra; + } + + private static ConfigRef refFor(Path f) { + return new ConfigRef(f, BridgedConfig.load(f)); + } + + @Test + void aHotChangeIsAppliedAndReadThroughGet(@TempDir Path dir) throws Exception { + Path f = dir.resolve("bridged.yaml"); + Files.writeString(f, yaml(""" + fleet: + tabLabel: "{role}: {profile} #{n}" + developers: + a: + profile: sonnet + """)); + ConfigRef ref = refFor(f); + assertEquals("{role}: {profile} #{n}", ref.get().fleet().tabLabel()); + + Files.writeString(f, yaml(""" + fleet: + tabLabel: "[{profile}] {role}" + developers: + a: + profile: sonnet + """)); + ConfigRef.Outcome out = ref.reload(); + + assertTrue(out.applied()); + assertTrue(out.deferred().isEmpty()); + assertEquals("config reloaded", out.summary()); + assertEquals("[{profile}] {role}", ref.get().fleet().tabLabel()); + } + + /** + * The point of the whole class: a consumer holding the ref sees the new value without being + * rebuilt. A component that captured {@code get()} into a field would still show the old one. + */ + @Test + void aConsumerHoldingTheRefSeesTheNewValue(@TempDir Path dir) throws Exception { + Path f = dir.resolve("bridged.yaml"); + Files.writeString(f, yaml("placement: weighted\n")); + ConfigRef ref = refFor(f); + java.util.function.Supplier reader = () -> ref.get().placement(); + assertEquals("weighted", reader.get()); + + Files.writeString(f, yaml("placement: fixed\n")); + assertTrue(ref.reload().applied()); + + assertEquals("fixed", reader.get()); + } + + @Test + void aChangedColdKeyRefusesTheWholeReload(@TempDir Path dir) throws Exception { + Path f = dir.resolve("bridged.yaml"); + Files.writeString(f, yaml("placement: weighted\n")); + ConfigRef ref = refFor(f); + + // Two changes in one file: a cold one (the port) and a hot one (placement). + Files.writeString(f, yaml("placement: fixed\n").replace("port: 8765", "port: 9999")); + ConfigRef.Outcome out = ref.reload(); + + assertFalse(out.applied()); + assertEquals(java.util.List.of("bind"), out.coldKeys()); + assertTrue(out.summary().contains("Restart bridged"), out.summary()); + // The hot half must NOT have leaked in. A half-applied reload leaves the daemon matching no + // file on disk, which is worse for an operator than no reload at all. + assertEquals("weighted", ref.get().placement()); + assertEquals(8765, ref.get().bind().port()); + } + + @Test + void aFileThatNoLongerParsesKeepsTheRunningConfig(@TempDir Path dir) throws Exception { + Path f = dir.resolve("bridged.yaml"); + Files.writeString(f, yaml("placement: weighted\n")); + ConfigRef ref = refFor(f); + BridgedConfig before = ref.get(); + + Files.writeString(f, "profiles:\n sonnet:\n baseUrl: \"unclosed\n"); + ConfigRef.Outcome out = ref.reload(); + + assertFalse(out.applied()); + assertNotNull(out.error()); + assertTrue(out.summary().startsWith("config reload refused"), out.summary()); + assertSame(before, ref.get()); + } + + /** A file that would have refused to boot must not be able to slip in through a reload. */ + @Test + void aFileThatFailsAValidatorKeepsTheRunningConfig(@TempDir Path dir) throws Exception { + Path f = dir.resolve("bridged.yaml"); + Files.writeString(f, yaml("placement: weighted\n")); + ConfigRef ref = refFor(f); + BridgedConfig before = ref.get(); + + // A member slot naming a profile that does not exist — validateMembers refuses this at + // startup, so it must refuse it here too. + Files.writeString(f, yaml(""" + fleet: + developers: + a: + profile: no-such-profile + """)); + ConfigRef.Outcome out = ref.reload(); + + assertFalse(out.applied()); + assertNotNull(out.error()); + assertSame(before, ref.get()); + } + + @Test + void aDeletedFileIsRefusedRatherThanCrashing(@TempDir Path dir) throws Exception { + Path f = dir.resolve("bridged.yaml"); + Files.writeString(f, yaml("")); + ConfigRef ref = refFor(f); + BridgedConfig before = ref.get(); + + Files.delete(f); + ConfigRef.Outcome out = ref.reload(); + + assertFalse(out.applied()); + assertNotNull(out.error()); + assertSame(before, ref.get()); + } + + /** A deferred change applies to the snapshot but the operator is told it needs a restart. */ + @Test + void aDeferredChangeIsAppliedAndReported(@TempDir Path dir) throws Exception { + Path f = dir.resolve("bridged.yaml"); + Files.writeString(f, yaml(""" + lifecycle: + drainTimeoutSeconds: 30 + """)); + ConfigRef ref = refFor(f); + + Files.writeString(f, yaml(""" + lifecycle: + drainTimeoutSeconds: 60 + """)); + ConfigRef.Outcome out = ref.reload(); + + assertTrue(out.applied()); + assertEquals(java.util.List.of("lifecycle"), out.deferred()); + assertTrue(out.summary().contains("needs a restart") || out.summary().contains("need a restart"), + out.summary()); + assertEquals(60, ref.get().lifecycle().drainTimeoutSeconds()); + } + + /** + * Adding a profile is deferred, not hot: a new backend needs its own launcher, and launchers are + * built once at startup. The snapshot carries it so a restart picks it up. + */ + @Test + void addingAProfileIsReportedAsDeferred(@TempDir Path dir) throws Exception { + Path f = dir.resolve("bridged.yaml"); + Files.writeString(f, yaml("")); + ConfigRef ref = refFor(f); + + Files.writeString(f, """ + bind: + host: 127.0.0.1 + port: 8765 + herdrSocket: ~/.config/herdr/herdr.sock + profiles: + sonnet: + baseUrl: http://gx00.gw:8000 + model: sonnet + haiku: + baseUrl: http://gx00.gw:8000 + model: haiku + guard: + offSubscriptionHosts: + - gx00.gw + """); + ConfigRef.Outcome out = ref.reload(); + + assertTrue(out.applied()); + assertEquals(1, out.deferred().size()); + assertTrue(out.deferred().getFirst().contains("haiku"), out.deferred().toString()); + } + + /** Changing an existing profile's fields is hot — no launcher has to be rebuilt for it. */ + @Test + void changingAnExistingProfilesFieldsIsHot(@TempDir Path dir) throws Exception { + Path f = dir.resolve("bridged.yaml"); + Files.writeString(f, yaml("")); + ConfigRef ref = refFor(f); + + Files.writeString(f, """ + bind: + host: 127.0.0.1 + port: 8765 + herdrSocket: ~/.config/herdr/herdr.sock + profiles: + sonnet: + baseUrl: http://gx00.gw:8000 + model: sonnet-4-5 + maxLoad: 7 + guard: + offSubscriptionHosts: + - gx00.gw + """); + ConfigRef.Outcome out = ref.reload(); + + assertTrue(out.applied()); + assertTrue(out.deferred().isEmpty(), out.deferred().toString()); + assertEquals("sonnet-4-5", ref.get().profiles().get("sonnet").model()); + } + + @Test + void aFixedRefHasNoFileAndRefusesToReload() { + BridgedConfig cfg = new BridgedConfig(null, null, null, null, null, null, + null, null, null, null, null, null, null, null, null).withDefaults(); + ConfigRef ref = ConfigRef.fixed(cfg); + + assertNull(ref.path()); + assertSame(cfg, ref.get()); + ConfigRef.Outcome out = ref.reload(); + assertFalse(out.applied()); + assertNotNull(out.error()); + } +} diff --git a/bridged/src/test/java/dev/ltms/bridged/config/ConfigWatcherTest.java b/bridged/src/test/java/dev/ltms/bridged/config/ConfigWatcherTest.java new file mode 100644 index 0000000..753e0c4 --- /dev/null +++ b/bridged/src/test/java/dev/ltms/bridged/config/ConfigWatcherTest.java @@ -0,0 +1,162 @@ +package dev.ltms.bridged.config; + +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.io.TempDir; + +import java.nio.file.Files; +import java.nio.file.Path; +import java.nio.file.attribute.FileTime; + +import static org.junit.jupiter.api.Assertions.*; + +/** + * CB-559: the mtime poller behind {@code configReload:}. The tests drive {@link + * ConfigWatcher#tick()} directly rather than the scheduler, so nothing here sleeps. + */ +class ConfigWatcherTest { + + private static String yaml(String placement) { + return """ + bind: + host: 127.0.0.1 + port: 8765 + herdrSocket: ~/.config/herdr/herdr.sock + profiles: + sonnet: + baseUrl: http://gx00.gw:8000 + model: sonnet + guard: + offSubscriptionHosts: + - gx00.gw + """ + "placement: " + placement + "\n"; + } + + /** Write and stamp an mtime, so a test never depends on the filesystem's clock resolution. */ + private static void write(Path f, String content, long millis) throws Exception { + Files.writeString(f, content); + Files.setLastModifiedTime(f, FileTime.fromMillis(millis)); + } + + @Test + void aChangedMtimeTriggersAReload(@TempDir Path dir) throws Exception { + Path f = dir.resolve("bridged.yaml"); + write(f, yaml("weighted"), 1_000L); + ConfigRef ref = new ConfigRef(f, BridgedConfig.load(f)); + + ConfigWatcher watcher = new ConfigWatcher(ref, 10); + try { + write(f, yaml("fixed"), 2_000L); + watcher.tick(); + + assertEquals("fixed", ref.get().placement()); + } finally { + watcher.stop(); + } + } + + @Test + void anUnchangedFileIsNotReloaded(@TempDir Path dir) throws Exception { + Path f = dir.resolve("bridged.yaml"); + write(f, yaml("weighted"), 1_000L); + ConfigRef ref = new ConfigRef(f, BridgedConfig.load(f)); + BridgedConfig before = ref.get(); + + ConfigWatcher watcher = new ConfigWatcher(ref, 10); + try { + watcher.tick(); + watcher.tick(); + + // Same instance, so no reload happened — a reload always swaps in a fresh object. + assertSame(before, ref.get()); + } finally { + watcher.stop(); + } + } + + /** + * A refused reload must not be retried every tick. Without the stamp-before-reload order the + * same refusal would be logged forever, which buries the log an operator needs. + */ + @Test + void aRefusedReloadIsNotRetriedUntilTheFileChangesAgain(@TempDir Path dir) throws Exception { + Path f = dir.resolve("bridged.yaml"); + write(f, yaml("weighted"), 1_000L); + ConfigRef ref = new ConfigRef(f, BridgedConfig.load(f)); + BridgedConfig before = ref.get(); + + ConfigWatcher watcher = new ConfigWatcher(ref, 10); + try { + // A cold change: refused, and the running config stays. + write(f, yaml("fixed").replace("port: 8765", "port: 9999"), 2_000L); + watcher.tick(); + assertSame(before, ref.get()); + + // The next tick sees the same mtime, so it does nothing at all. + watcher.tick(); + assertSame(before, ref.get()); + + // A fresh save earns a fresh attempt — and this one is hot, so it applies. + write(f, yaml("fixed"), 3_000L); + watcher.tick(); + assertEquals("fixed", ref.get().placement()); + } finally { + watcher.stop(); + } + } + + /** + * Editors briefly unlink the file mid-save. A missing file is skipped, not an error and not a + * reload — the running config stays live, which is right either way. + */ + @Test + void aMissingFileIsSkippedAndTheNextTickTriesAgain(@TempDir Path dir) throws Exception { + Path f = dir.resolve("bridged.yaml"); + write(f, yaml("weighted"), 1_000L); + ConfigRef ref = new ConfigRef(f, BridgedConfig.load(f)); + BridgedConfig before = ref.get(); + + ConfigWatcher watcher = new ConfigWatcher(ref, 10); + try { + Files.delete(f); + assertDoesNotThrow(watcher::tick); + assertSame(before, ref.get()); + + write(f, yaml("fixed"), 2_000L); + watcher.tick(); + assertEquals("fixed", ref.get().placement()); + } finally { + watcher.stop(); + } + } + + /** A ref with no file behind it must not start a scheduler that could never do anything. */ + @Test + void aFixedRefStartsNoWatch() { + BridgedConfig cfg = new BridgedConfig(null, null, null, null, null, null, + null, null, null, null, null, null, null, null, null).withDefaults(); + ConfigWatcher watcher = new ConfigWatcher(ConfigRef.fixed(cfg), 10); + try { + assertDoesNotThrow(watcher::start); + assertDoesNotThrow(watcher::tick); + } finally { + watcher.stop(); + } + } + + @Test + void configReloadIsOffUnlessTheBlockSaysOtherwise(@TempDir Path dir) throws Exception { + Path f = dir.resolve("bridged.yaml"); + write(f, yaml("weighted"), 1_000L); + assertNull(BridgedConfig.load(f).configReload()); + + write(f, yaml("weighted") + "configReload:\n enabled: true\n", 2_000L); + BridgedConfig.ConfigReload on = BridgedConfig.load(f).configReload(); + assertTrue(on.isEnabled()); + assertEquals(10, on.intervalSeconds(), "an absent interval defaults to 10s"); + + write(f, yaml("weighted") + "configReload:\n intervalSeconds: 30\n", 3_000L); + BridgedConfig.ConfigReload off = BridgedConfig.load(f).configReload(); + assertFalse(off.isEnabled(), "a block that only sets the interval does not enable the watch"); + assertEquals(30, off.intervalSeconds()); + } +} diff --git a/bridged/src/test/java/dev/ltms/bridged/member/ClaudeCodeLauncherTest.java b/bridged/src/test/java/dev/ltms/bridged/member/ClaudeCodeLauncherTest.java index b421ffc..1dfa0ba 100644 --- a/bridged/src/test/java/dev/ltms/bridged/member/ClaudeCodeLauncherTest.java +++ b/bridged/src/test/java/dev/ltms/bridged/member/ClaudeCodeLauncherTest.java @@ -17,7 +17,9 @@ import java.util.List; import java.util.Map; import java.util.Set; import java.util.UUID; +import java.util.concurrent.atomic.AtomicReference; import java.util.function.Function; +import java.util.function.Supplier; import static org.junit.jupiter.api.Assertions.*; @@ -795,7 +797,7 @@ class ClaudeCodeLauncherTest { } /** A profile with no {@code tabLabel:} of its own — the fleet template decides. */ - private ClaudeCodeLauncher labelService(FakeHerdr herdr, String fleetTemplate) { + private ClaudeCodeLauncher labelService(FakeHerdr herdr, Supplier fleetTemplate) { BridgedConfig.Profile cfg = new BridgedConfig.Profile( "sonnet", "http://gx00.gw:8000", "sonnet", null, "BRIDGED_WORKER_TOKEN", List.of("claude"), "tab", "bridged-workers", null, null, null, null); @@ -812,7 +814,7 @@ class ClaudeCodeLauncherTest { @Test void theFleetTemplateNamesTheRoleTheMemberWasSpawnedFor() { FakeHerdr herdr = new FakeHerdr(); - ClaudeCodeLauncher svc = labelService(herdr, "{role}: {profile} #{n}"); + ClaudeCodeLauncher svc = labelService(herdr, () -> "{role}: {profile} #{n}"); svc.spawn(new SpawnRequest("sonnet", null, null, null, null, MemberRole.REVIEWER)); @@ -823,7 +825,7 @@ class ClaudeCodeLauncherTest { @Test void theCounterRunsPerRoleAndProfileNotPerFleet() { FakeHerdr herdr = new FakeHerdr(); - ClaudeCodeLauncher svc = labelService(herdr, "{role}: {profile} #{n}"); + ClaudeCodeLauncher svc = labelService(herdr, () -> "{role}: {profile} #{n}"); svc.spawn(new SpawnRequest("sonnet", null, null, null, null, MemberRole.DEV)); svc.spawn(new SpawnRequest("sonnet", null, null, null, null, MemberRole.REVIEWER)); @@ -837,7 +839,7 @@ class ClaudeCodeLauncherTest { @Test void aBlankFleetTemplateFallsBackToTheRoleFirstDefault() { FakeHerdr herdr = new FakeHerdr(); - labelService(herdr, null).spawn( + labelService(herdr, () -> null).spawn( new SpawnRequest("sonnet", null, null, null, null, MemberRole.ARCHITECT)); assertEquals(List.of("architect: sonnet #1"), tabLabels(herdr)); @@ -853,9 +855,27 @@ class ClaudeCodeLauncherTest { List.of("claude"), "tab", "bridged-workers", "pinned {profile}", null, null, null); new ClaudeCodeLauncher(new AgentControl(herdr), new WorkspaceControl(herdr), new SubscriptionGuard(Set.of("gx00.gw")), Map.of(cfg.profile(), cfg), cfg.profile(), - _ -> null, 0, 0L, "{role}: {profile} #{n}") + _ -> null, 0, 0L, () -> "{role}: {profile} #{n}") .spawn(new SpawnRequest("sonnet", null, null, null, null, MemberRole.REVIEWER)); assertEquals(List.of("pinned sonnet"), tabLabels(herdr)); } + + /** + * CB-559: the template is read per spawn, not captured at construction. This is what makes + * {@code fleet.tabLabel} a hot key — a launcher built at boot must see an edit made an hour later + * without being rebuilt. + */ + @Test + void theTemplateIsReadOnEverySpawnSoAnEditTakesEffect() { + FakeHerdr herdr = new FakeHerdr(); + AtomicReference template = new AtomicReference<>("{role}: {profile} #{n}"); + ClaudeCodeLauncher svc = labelService(herdr, template::get); + + svc.spawn(new SpawnRequest("sonnet", null, null, null, null, MemberRole.DEV)); + template.set("[{profile}] {role} {n}"); + svc.spawn(new SpawnRequest("sonnet", null, null, null, null, MemberRole.DEV)); + + assertEquals(List.of("dev: sonnet #1", "[sonnet] dev 2"), tabLabels(herdr)); + } }