fleet/fleetd
3
0
Fork 0
Code Issues 73 Pull Requests Actions Packages Projects Releases 1 Wiki Activity

CB-559: a profile's launch settings are deferred, not hot

Browse Source 
The shipped docs and javadoc said a profile's `model` and `tabLabel` take effect on
the next spawn. They do not, and ConfigRef did not detect the change either, so a
reload logged a clean "config reloaded" and silently did nothing. That is the worst
outcome a reload can produce: the operator has no reason to doubt it.

What makes a key hot is who reads it and when, not that it is config. Placement
reads weight and maxLoad through a supplier on CompositePeerLauncher, so those are
genuinely hot. HerdrPeerLauncher takes Map.copyOf(profiles) at construction and
resolves every spawn out of that copy, so model, baseUrl, argv, env and the rest
cannot move until the daemon restarts.

changedDeferredKeys now compares every launch component of an existing profile,
excluding weight and maxLoad, and names the profiles that need a restart. The
javadoc and bridged.example.yaml say the same thing. Two tests pin the pair:
weight/maxLoad reports nothing deferred, a changed model reports the profile by name.
This commit is contained in:
Dai Ha
2026-08-14 20:37:42 +02:00
parent c18572ea9d
commit e18e002d2f
3 changed files with 114 additions and 21 deletions
Download Patch File Download Diff File Expand all files Collapse all files
bridged/bridged.example.yaml
+9 -5
View File
@@ -232,13 +232,17 @@ placement: weighted
# 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.
# `tabLabel`), `placement:`, and an existing profile's weight / maxLoad. Those are
# hot because the placement policy reads them through a supplier — being config is
# not by itself enough to make a key hot.
# 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.
# `spawnReadyTimeoutMs` / `spawnReadyPollMs`, ADDING or REMOVING a profile (a new
# backend needs its own launcher, and launchers are built once), AND an existing
# profile's launch settings — model, baseUrl, argv, env, configDir, mcpUrl, tabLabel.
# The launcher takes a copy of `profiles:` at startup and resolves every spawn out of
# that copy, so those never reach a launch until you restart. The reload logs them 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.
bridged/src/main/java/dev/ltms/bridged/config/ConfigRef.java
+62 -12
View File
@@ -7,6 +7,7 @@ import java.nio.file.Path;
import java.util.ArrayList;
import java.util.LinkedHashSet;
import java.util.List;
import java.util.Map;
import java.util.Objects;
import java.util.Set;
import java.util.concurrent.atomic.AtomicReference;
@@ -27,13 +28,18 @@ import java.util.function.Supplier;
* <ul>
* <li><strong>Hot</strong> — re-read per use, so a reload takes effect on the next spawn:
* {@code fleet:} (every role pool and {@code tabLabel}), {@code placement:}, and an existing
* profile's {@code weight} / {@code maxLoad} / {@code model} / {@code tabLabel}.</li>
* profile's {@code weight} / {@code maxLoad}. Those three are read through a supplier on
* {@code CompositePeerLauncher}, which is what makes them hot — not the fact that they are
* config.</li>
* <li><strong>Deferred</strong> — accepted into the new snapshot, but the wiring built at startup
* keeps the old value until a restart: {@code lifecycle:}, {@code leadHeartbeat:},
* {@code spawnReadyTimeoutMs} / {@code spawnReadyPollMs}, {@code guard:},
* {@code worktreeRoot:}, and <em>adding or removing</em> a profile (a new backend needs its
* own launcher, which is constructed once). A reload logs these rather than pretending they
* applied.</li>
* {@code worktreeRoot:}, adding or removing a profile (a new backend needs its own launcher,
* which is constructed once), <em>and an existing profile's launch settings</em> —
* {@code model}, {@code baseUrl}, {@code argv}, {@code env}, {@code mcpUrl} and the rest.
* {@code HerdrPeerLauncher} takes {@code Map.copyOf(profiles)} at construction and resolves
* each spawn out of that copy, so those never reach a launch until the daemon restarts. A
* reload logs these rather than pretending they applied.</li>
* <li><strong>Cold</strong> — cannot change at all under a running daemon: {@code bind:},
* {@code herdrSocket:}, {@code broker:} and {@code auth:}. The socket is bound, the broker
* connection is open, and the auth mode decides who may reach the port that is already
@@ -204,16 +210,60 @@ public final class ConfigRef implements Supplier<BridgedConfig> {
|| !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<String> before = old.profiles() == null ? Set.of() : old.profiles().keySet();
Set<String> after = fresh.profiles() == null ? Set.of() : fresh.profiles().keySet();
if (!before.equals(after)) {
Set<String> diff = new LinkedHashSet<>(before);
diff.addAll(after);
diff.removeIf(p -> before.contains(p) && after.contains(p));
Map<String, BridgedConfig.Profile> before =
old.profiles() == null ? Map.of() : old.profiles();
Map<String, BridgedConfig.Profile> after =
fresh.profiles() == null ? Map.of() : fresh.profiles();
// Adding or removing a profile is deferred: a new backend needs its own launcher, and
// launchers are built once at startup.
if (!before.keySet().equals(after.keySet())) {
Set<String> diff = new LinkedHashSet<>(before.keySet());
diff.addAll(after.keySet());
diff.removeIf(p -> before.containsKey(p) && after.containsKey(p));
changed.add("profiles (added/removed: " + String.join(", ", diff) + ")");
}
// An EXISTING profile's launch settings are deferred too, and this is easy to get wrong:
// `HerdrPeerLauncher` takes `Map.copyOf(profiles)` at construction and `spawn` resolves the
// profile out of that snapshot, so a reloaded model/baseUrl/argv/env never reaches a launch.
// Only weight and maxLoad are genuinely hot, because placement reads them through the
// supplier on the composite rather than from the adapter's copy. Without this check a
// changed model would report "config reloaded" and silently do nothing — the worst outcome
// a reload can produce, because the operator has no reason to doubt it.
List<String> relaunch = new ArrayList<>();
before.forEach((name, was) -> {
BridgedConfig.Profile now = after.get(name);
if (now != null && !sameLaunchSettings(was, now)) {
relaunch.add(name);
}
});
if (!relaunch.isEmpty()) {
changed.add("profiles." + String.join("/", relaunch) + " launch settings "
+ "(model, baseUrl, argv, env, …) — the launcher holds a startup snapshot");
}
return changed;
}
/**
* Whether two versions of a profile would launch a peer identically. Compares every component
* the launcher reads at spawn; {@code weight} and {@code maxLoad} are excluded because those are
* read live by the placement policy and really do take effect on the next spawn.
*/
private static boolean sameLaunchSettings(BridgedConfig.Profile a, BridgedConfig.Profile b) {
return Objects.equals(a.baseUrl(), b.baseUrl())
&& Objects.equals(a.model(), b.model())
&& Objects.equals(a.configDir(), b.configDir())
&& Objects.equals(a.tokenEnv(), b.tokenEnv())
&& Objects.equals(a.argv(), b.argv())
&& Objects.equals(a.placement(), b.placement())
&& Objects.equals(a.workspace(), b.workspace())
&& Objects.equals(a.tabLabel(), b.tabLabel())
&& Objects.equals(a.mcpUrl(), b.mcpUrl())
&& Objects.equals(a.cwd(), b.cwd())
&& Objects.equals(a.parityOverlay(), b.parityOverlay())
&& Objects.equals(a.gitTokenEnv(), b.gitTokenEnv())
&& Objects.equals(a.gitHostEnv(), b.gitHostEnv())
&& Objects.equals(a.kind(), b.kind())
&& Objects.equals(a.env(), b.env())
&& Objects.equals(a.subscription(), b.subscription());
}
}
bridged/src/test/java/dev/ltms/bridged/config/ConfigRefTest.java
+43 -4
View File
@@ -213,9 +213,12 @@ class ConfigRefTest {
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. */
/**
* Changing an existing profile's weight or maxLoad IS hot: placement reads those live through
* the supplier on the composite, so the next spawn already sees them.
*/
@Test
void changingAnExistingProfilesFieldsIsHot(@TempDir Path dir) throws Exception {
void changingAProfilesWeightOrMaxLoadIsHot(@TempDir Path dir) throws Exception {
Path f = dir.resolve("bridged.yaml");
Files.writeString(f, yaml(""));
ConfigRef ref = refFor(f);
@@ -228,8 +231,9 @@ class ConfigRefTest {
profiles:
sonnet:
baseUrl: http://gx00.gw:8000
model: sonnet-4-5
model: sonnet
maxLoad: 7
weight: 3.0
guard:
offSubscriptionHosts:
- gx00.gw
@@ -238,7 +242,42 @@ class ConfigRefTest {
assertTrue(out.applied());
assertTrue(out.deferred().isEmpty(), out.deferred().toString());
assertEquals("sonnet-4-5", ref.get().profiles().get("sonnet").model());
assertEquals(7, ref.get().profiles().get("sonnet").maxLoad());
}
/**
* Changing an existing profile's MODEL is deferred, not hot — and saying so is the whole point.
* HerdrPeerLauncher takes Map.copyOf(profiles) at construction and resolves every spawn out of
* that copy, so a reloaded model never reaches a launch. Reporting it as applied would be the
* worst outcome a reload can produce: the operator has no reason to doubt a clean "reloaded".
*/
@Test
void changingAProfilesLaunchSettingsIsReportedAsDeferred(@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: deepseek-v4-flash
guard:
offSubscriptionHosts:
- gx00.gw
""");
ConfigRef.Outcome out = ref.reload();
assertTrue(out.applied());
assertEquals(1, out.deferred().size(), out.deferred().toString());
assertTrue(out.deferred().getFirst().contains("sonnet"), out.deferred().toString());
assertTrue(out.deferred().getFirst().contains("launch settings"), out.deferred().toString());
// The snapshot still carries the new value — a restart is what makes it take effect.
assertEquals("deepseek-v4-flash", ref.get().profiles().get("sonnet").model());
}
@Test