fleetd #474: ConfigRef.reload() runs the charter tool-surface gate too

A charter naming an MCP tool the server does not register refused Fleetd.main
at startup but slipped through ConfigRef.reload(), because reload() only ran
FleetConfig.validateAll(), which never looks at what a charter's text names.

CharterToolSurface stays in the mcp package (config must not depend on it), so
ConfigRef now accepts the check as a Consumer<FleetConfig> extraValidation,
run inside reload()'s same try/catch as validateAll(). Fleetd.main wires a new
package-private adapter, Fleetd.assertChartersNameOnlyRegisteredTools, into
both the startup call site and ConfigRef's constructor, so the two call sites
can never check different things.

Tests: ConfigRefTest (reload refuses/accepts, via a locally-built equivalent
consumer since Fleetd's method is package-private to dev.ltms.fleet) and the
new FleetdConfigRefCharterToolSurfaceWiringTest (same proof through the exact
Fleetd::assertChartersNameOnlyRegisteredTools reference production uses).
Verified deleting the new extraValidation.accept(fresh) call site fails both
new "refuses" tests by name.

(cherry picked from commit 97e4c1d658)

Lead review. Cherry-picked, not merged: the worker branched from 435e022,
which is not an ancestor of main (I had reset and rewritten that commit's
message while the worker was already on it). Merging its branch would have
added a second merge commit for work already on main. Same content, no
duplicated history. Rule learned: once a worker is spawned, its base
commit is published.

My own mutation battery, five cells, each a full `mvn -B clean test` on
this commit's own tree:

  CONTROL 1, unmutated       1633 tests, 0 failures, BUILD SUCCESS
  M1 delete accept(fresh)    KILLED  2 by name
  M3 3-arg ctor stores no-op KILLED  2 by name
  M4 set(fresh) before valid KILLED  6
  M5 accept outside try      KILLED  2 errors
  M2 main uses the 2-arg ctor SURVIVED

M2 is a real gap and it is a test gap, not a defect: the production code
here is correct. Reverting Fleetd.java:154 to `new ConfigRef(configPath,
cfg)` turns the live reload gate off and leaves all 1633 tests green,
because both new tests build their own ConfigRef with the method
reference rather than reading what main wires.

That is the fourth Fleetd.main call site to survive a battery (#446 M5
and M7, #466 M1, now this). Extracting a check into a well-tested helper
moves the untested surface UP, into the line that chooses to call it. The
repo already has the answer in three source-text wiring tests
(FleetdBackendQuarantineWiringTest, FleetdLeadSeatWiringTest,
FleetdCompletionResolverWiringTest); the worker followed the other
sibling pattern, which builds the wiring itself and so cannot pin main.
Delegated as a follow-up, with the surviving mutation as its acceptance
criterion.

Also mine to correct: my M3 cell's annotation said the no-op store "must
be 2" and measured 1. The 2-arg constructor delegates rather than
assigning, so my pattern only ever matched the mutant. The cell still
stands on its other proof (requireNonNull left = 0). Third battery in a
row with a wrong CONTROL annotation of my own.

fleetd/src/main/java/dev/ltms/fleet/Fleetd.java

@@ -148,7 +148,10 @@ public final class Fleetd {
         // 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);
+        // fleetd #474: pass the charter/tool-surface check in as ConfigRef's extraValidation, so
+        // ConfigRef#reload() runs the same gate main() runs below, without dev.ltms.fleet.config
+        // gaining a dependency on dev.ltms.fleet.mcp — Fleetd is the seam that already holds both.
+        ConfigRef config = new ConfigRef(configPath, cfg, Fleetd::assertChartersNameOnlyRegisteredTools);
 
         // The primary/host env that launched fleetd must not be tainted.
         SubscriptionGuard guard = new SubscriptionGuard(cfg.guard().hostSet());
@@ -172,9 +175,9 @@ public final class Fleetd {
         // inside FleetConfig#validateCharters() — config loads before the MCP server exists, and
         // must not gain a dependency on the mcp package — so it runs here instead, at the one seam
         // that already holds both a loaded FleetConfig and the mcp package, before anything below
-        // opens a socket or spawns a member.
+        // opens a socket or spawns a member. fleetd #474: the same check is also wired into `config`
-        CharterToolSurface.assertChartersNameOnlyRegisteredTools(
+        // above as ConfigRef's extraValidation, so a reload refuses what this line refuses at startup.
-                cfg.fleet() == null ? Map.of() : cfg.fleet().charters());
+        assertChartersNameOnlyRegisteredTools(cfg);
 
         Path socket = cfg.herdrSocket() != null && !cfg.herdrSocket().isBlank()
                 ? Path.of(cfg.herdrSocket())
@@ -1594,6 +1597,28 @@ public final class Fleetd {
         }
     }
 
+    /**
+     * 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,
+     * via a method reference to this method) go through, so the two can never drift into checking
+     * different things. Extracted only to give {@link ConfigRef}'s {@code Consumer<FleetConfig>}
+     * hook a {@code FleetConfig -> void} shape to bind to — {@link CharterToolSurface} itself still
+     * takes the raw charter map and knows nothing about {@code ConfigRef} or {@code Fleetd}.
+     *
+     * <p>Package-private so a test can call it directly the same way the other startup-report
+     * helpers above are tested, without needing to drive {@link #main} for a unit-level check;
+     * {@code FleetdStartupValidationTest} proves the startup call site, and {@code
+     * FleetdConfigRefCharterToolSurfaceWiringTest} — by constructing {@code ConfigRef} with this
+     * exact method reference, the same way {@code main} does above — proves the reload call site.
+     * {@code dev.ltms.fleet.config.ConfigRefTest} pins the same reload behaviour too, through an
+     * equivalent {@code Consumer<FleetConfig>} it builds locally (it cannot see this package-private
+     * method from {@code dev.ltms.fleet.config}).
+     */
+    static void assertChartersNameOnlyRegisteredTools(FleetConfig cfg) {
+        CharterToolSurface.assertChartersNameOnlyRegisteredTools(
+                cfg.fleet() == null ? Map.of() : cfg.fleet().charters());
+    }
+
     /**
      * Poll herdr's {@code ping} until it answers or {@link #HERDR_WAIT_SECONDS} elapses (CB-504).
      *

fleetd/src/main/java/dev/ltms/fleet/config/ConfigRef.java

@@ -11,6 +11,7 @@ import java.util.Map;
 import java.util.Objects;
 import java.util.Set;
 import java.util.concurrent.atomic.AtomicReference;
+import java.util.function.Consumer;
 import java.util.function.Supplier;
 
 /**
@@ -211,6 +212,24 @@ import java.util.function.Supplier;
  * <p>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.
+ *
+ * <p><strong>fleetd #474</strong> — {@link FleetConfig#validateAll()} is not the only gate startup
+ * runs before a config takes effect: {@code Fleetd.main} also calls {@code
+ * dev.ltms.fleet.mcp.CharterToolSurface#assertChartersNameOnlyRegisteredTools}, right after {@code
+ * cfg.validateAll()}, to refuse a charter that names an MCP tool the server does not register. That
+ * check cannot live inside {@link FleetConfig} — {@code CharterToolSurface} lives in the {@code mcp}
+ * package because the canonical tool set ({@code FleetTool}) does, and config is loaded before the
+ * MCP server exists, so {@code FleetConfig} must not gain a dependency on {@code mcp}. {@link
+ * #reload} cannot import {@code mcp} either, for the same reason applied one layer up: {@code
+ * dev.ltms.fleet.config} is loaded before {@code dev.ltms.fleet.mcp} exists, same as {@code
+ * FleetConfig}. So this class accepts the check as a {@code Consumer<FleetConfig>} —
+ * {@link #extraValidation} — supplied by whichever caller already sits at the seam that holds both
+ * a loaded {@code FleetConfig} and the {@code mcp} package: {@code Fleetd.main}. It is invoked
+ * inside the same try/catch as {@code fresh.validateAll()}, so a charter that would have refused to
+ * boot refuses a reload too, and keeps the running config exactly like any other {@code
+ * validateAll()} failure. A ref built through the two-argument constructor (every test fixture that
+ * does not care about this check, and {@link #fixed}) gets a no-op consumer, so nothing outside
+ * {@code Fleetd.main} needs to know this hook exists.
  */
 public final class ConfigRef implements Supplier<FleetConfig> {
 
@@ -255,10 +274,27 @@ public final class ConfigRef implements Supplier<FleetConfig> {
 
     private final Path path;
     private final AtomicReference<FleetConfig> current;
+    private final Consumer<FleetConfig> extraValidation;
 
+    /** Equivalent to the three-argument constructor with a no-op {@code extraValidation}. */
     public ConfigRef(Path path, FleetConfig initial) {
+        this(path, initial, cfg -> { });
+    }
+
+    /**
+     * @param extraValidation run on every {@link #reload} candidate, inside the same try/catch as
+     *                        {@code fresh.validateAll()} — see the class doc's fleetd #474 note.
+     *                        {@code Fleetd.main} passes {@code
+     *                        Fleetd::assertChartersNameOnlyRegisteredTools} (a package-private
+     *                        {@code FleetConfig -> void} adapter over {@code
+     *                        CharterToolSurface#assertChartersNameOnlyRegisteredTools}), so a reload
+     *                        runs the same gate startup does without this class depending on the
+     *                        {@code mcp} package.
+     */
+    public ConfigRef(Path path, FleetConfig initial, Consumer<FleetConfig> extraValidation) {
         this.path = path;
         this.current = new AtomicReference<>(Objects.requireNonNull(initial, "initial config"));
+        this.extraValidation = Objects.requireNonNull(extraValidation, "extraValidation");
     }
 
     /** A fixed reference that never reloads — for tests and for wiring built from a config in code. */
@@ -360,6 +396,12 @@ public final class ConfigRef implements Supplier<FleetConfig> {
             // at all — see FleetConfig#validateAll's javadoc for why the fix is one reflective call,
             // not a longer hand-maintained list.
             fresh.validateAll();
+            // fleetd #474: validateAll() does not cover everything startup refuses on — the charter
+            // tool-surface check (Fleetd.main, right after cfg.validateAll()) lives outside
+            // FleetConfig on purpose (see this class's doc) and is supplied here as extraValidation.
+            // Same try/catch as validateAll() above, on purpose: either failure must refuse the whole
+            // reload and keep the running config the same way.
+            extraValidation.accept(fresh);
         } catch (RuntimeException e) {
             String msg = e.getMessage() == null ? e.toString() : e.getMessage();
             log.warn("config reload from {} refused, keeping the running config: {}", path, msg);

fleetd/src/main/java/dev/ltms/fleet/mcp/CharterToolSurface.java

@@ -25,6 +25,20 @@ import java.util.regex.Pattern;
  * {@code fleetd.yaml} could ever fail it. This class is what a real charter is actually checked
  * against at boot; {@code FleetdStartupValidationTest} exercises it through {@code Fleetd.main}
  * itself, the same way it proves every other {@code validateXxx()} still runs there.
+ *
+ * <p><strong>fleetd #474</strong> — startup was not the only door: {@code
+ * dev.ltms.fleet.config.ConfigRef#reload()} used to run {@code FleetConfig#validateAll()} alone,
+ * which does not look at what a charter's text names, so a charter naming an unregistered tool
+ * that could not have booted the daemon could still be installed into a running one through a
+ * reload. This class still knows nothing about {@code ConfigRef} — {@code Fleetd.main} wires a
+ * small {@code FleetConfig -> void} adapter over {@link #assertChartersNameOnlyRegisteredTools}
+ * ({@code Fleetd::assertChartersNameOnlyRegisteredTools}) into {@code ConfigRef}'s constructor as
+ * its {@code Consumer<FleetConfig>} {@code extraValidation}, run inside {@code reload()}'s same
+ * try/catch as {@code validateAll()}, so both call sites — {@code Fleetd.main} at startup and
+ * {@code ConfigRef#reload()} afterwards — go through this one method and can never check different
+ * things. {@code dev.ltms.fleet.config.ConfigRefTest} and {@code
+ * FleetdConfigRefCharterToolSurfaceWiringTest} are what prove the reload call site, the same way
+ * {@code FleetdStartupValidationTest} proves the startup one.
  */
 public final class CharterToolSurface {
 

fleetd/src/test/java/dev/ltms/fleet/FleetdConfigRefCharterToolSurfaceWiringTest.java

@@ -0,0 +1,106 @@
+package dev.ltms.fleet;
+
+import dev.ltms.fleet.config.ConfigRef;
+import dev.ltms.fleet.config.FleetConfig;
+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.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertNotNull;
+import static org.junit.jupiter.api.Assertions.assertSame;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+
+/**
+ * fleetd #474: proves the exact wiring {@code Fleetd.main} uses to construct its live {@code
+ * ConfigRef} — {@code new ConfigRef(configPath, cfg, Fleetd::assertChartersNameOnlyRegisteredTools)}
+ * — actually makes {@link ConfigRef#reload()} refuse a charter that names an MCP tool the server
+ * does not register, the same way {@code Fleetd.main} itself refuses one at startup (see {@code
+ * FleetdStartupValidationTest#mainRefusesACharterNamingAnUnregisteredTool}).
+ *
+ * <p>{@code dev.ltms.fleet.config.ConfigRefTest} pins the same behaviour through a locally-built
+ * {@code Consumer<FleetConfig>} adapter that calls the same production {@code CharterToolSurface}
+ * method, because that test lives in {@code dev.ltms.fleet.config} and cannot see {@code
+ * Fleetd#assertChartersNameOnlyRegisteredTools} (package-private to {@code dev.ltms.fleet}). This
+ * class is the companion proof that lives where the real method reference is visible, so the literal
+ * expression {@code Fleetd::assertChartersNameOnlyRegisteredTools} — not just an equivalent — is
+ * what gets exercised. {@code Fleetd.main} itself cannot be driven this far in a unit test: every
+ * fixture in {@code FleetdStartupValidationTest} is deliberately invalid so {@code main} throws
+ * before opening a socket, binding Javalin, or doing anything else with a real side effect, so a
+ * test cannot get {@code main} far enough to hold a running daemon it could then reload — this test
+ * builds the {@code ConfigRef} the same way {@code main} does and drives {@link ConfigRef#reload()}
+ * directly instead, the same shape {@code FleetdExhaustionDetectionArmedWiringTest} and its
+ * siblings already use for the rest of {@code Fleetd.main}'s wiring.
+ */
+class FleetdConfigRefCharterToolSurfaceWiringTest {
+
+    private static final String BASE = """
+            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
+            """;
+
+    @Test
+    void reloadRefusesACharterNamingAnUnregisteredToolThroughFleetdsOwnWiring(@TempDir Path dir)
+            throws Exception {
+        Path f = dir.resolve("fleetd.yaml");
+        Files.writeString(f, BASE + """
+                fleet:
+                  charters:
+                    dev: |
+                      Send the final handoff through fleet_reply.
+                """);
+        ConfigRef config = new ConfigRef(f, FleetConfig.load(f),
+                Fleetd::assertChartersNameOnlyRegisteredTools);
+        FleetConfig before = config.get();
+
+        Files.writeString(f, BASE + """
+                fleet:
+                  charters:
+                    dev: |
+                      Send the final handoff through bridge_send.
+                """);
+        ConfigRef.Outcome out = config.reload();
+
+        assertFalse(out.applied());
+        assertNotNull(out.error());
+        assertTrue(out.error().contains("dev"), out.error());
+        assertTrue(out.error().contains("bridge_send"), out.error());
+        assertSame(before, config.get());
+    }
+
+    @Test
+    void reloadAcceptsACharterNamingOnlyRegisteredToolsThroughFleetdsOwnWiring(@TempDir Path dir)
+            throws Exception {
+        Path f = dir.resolve("fleetd.yaml");
+        Files.writeString(f, BASE + """
+                fleet:
+                  charters:
+                    dev: old charter
+                """);
+        ConfigRef config = new ConfigRef(f, FleetConfig.load(f),
+                Fleetd::assertChartersNameOnlyRegisteredTools);
+
+        Files.writeString(f, BASE + """
+                fleet:
+                  charters:
+                    dev: |
+                      Send the final handoff through fleet_reply.
+                """);
+        ConfigRef.Outcome out = config.reload();
+
+        assertTrue(out.applied());
+        assertEquals("config reloaded", out.summary());
+    }
+}

fleetd/src/test/java/dev/ltms/fleet/config/ConfigRefTest.java

@@ -1,10 +1,13 @@
 package dev.ltms.fleet.config;
 
+import dev.ltms.fleet.mcp.CharterToolSurface;
 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.util.Map;
+import java.util.function.Consumer;
 
 import static org.junit.jupiter.api.Assertions.*;
 
@@ -38,6 +41,23 @@ class ConfigRefTest {
         return new ConfigRef(f, FleetConfig.load(f));
     }
 
+    /**
+     * The exact {@code Consumer<FleetConfig>} {@code Fleetd.main} wires into {@code ConfigRef}'s
+     * constructor as {@code extraValidation} (fleetd #474) — an adapter from {@code FleetConfig} to
+     * the raw charter map {@link CharterToolSurface#assertChartersNameOnlyRegisteredTools} takes.
+     * Built here rather than referencing {@code dev.ltms.fleet.Fleetd} directly, because that method
+     * is package-private to {@code dev.ltms.fleet} and this test lives in {@code
+     * dev.ltms.fleet.config} — but it calls the SAME production {@link CharterToolSurface} method
+     * {@code Fleetd} calls, so this proves the real check runs on reload, not a stand-in for it.
+     */
+    private static final Consumer<FleetConfig> CHARTER_TOOL_SURFACE = cfg ->
+            CharterToolSurface.assertChartersNameOnlyRegisteredTools(
+                    cfg.fleet() == null ? Map.of() : cfg.fleet().charters());
+
+    private static ConfigRef refForWithCharterToolSurface(Path f) {
+        return new ConfigRef(f, FleetConfig.load(f), CHARTER_TOOL_SURFACE);
+    }
+
     @Test
     void aHotChangeIsAppliedAndReadThroughGet(@TempDir Path dir) throws Exception {
         Path f = dir.resolve("fleetd.yaml");
@@ -124,6 +144,87 @@ class ConfigRefTest {
         assertSame(before, ref.get());
     }
 
+    /**
+     * fleetd #474: {@code validateAll()} (via {@code validateCharters()}) 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
+     * names, so a charter naming {@code bridge_send} (the pre-CB-634 name, removed from the tool
+     * surface — #469's own motivating example) passes {@code validateAll()} and used to be applied
+     * on reload with nothing refusing it, even though the identical charter refuses {@code
+     * Fleetd.main} at startup ({@code FleetdStartupValidationTest
+     * #mainRefusesACharterNamingAnUnregisteredTool}). This is the reload-path proof: it drives
+     * {@link ConfigRef#reload()} itself (not a direct call to {@link
+     * CharterToolSurface#assertChartersNameOnlyRegisteredTools}), through the exact {@code
+     * extraValidation} wiring {@code Fleetd.main} uses, and the failure message must name both the
+     * charter key and the unknown tool — the same information the startup failure gives (ticket
+     * acceptance criterion 1).
+     */
+    @Test
+    void aReloadRefusesACharterNamingAnUnregisteredTool(@TempDir Path dir) throws Exception {
+        Path f = dir.resolve("fleetd.yaml");
+        Files.writeString(f, yaml("""
+                fleet:
+                  charters:
+                    dev: |
+                      Send the final handoff through fleet_reply.
+                """));
+        ConfigRef ref = refForWithCharterToolSurface(f);
+        FleetConfig before = ref.get();
+
+        Files.writeString(f, yaml("""
+                fleet:
+                  charters:
+                    dev: |
+                      Send the final handoff through bridge_send.
+                """));
+        ConfigRef.Outcome out = ref.reload();
+
+        assertFalse(out.applied());
+        assertNotNull(out.error());
+        assertTrue(out.error().contains("dev"),
+                "expected the charter key 'dev' in the refusal, got: " + out.error());
+        assertTrue(out.error().contains("bridge_send"),
+                "expected the unknown tool 'bridge_send' in the refusal, got: " + out.error());
+        assertTrue(out.summary().startsWith("config reload refused"), out.summary());
+        // The running config must not move at all — half-applying this would leave the daemon in a
+        // state that could never have booted, exactly the outcome ConfigRef.java's class doc warns
+        // a cold-key refusal must avoid, and this check must avoid the same way.
+        assertSame(before, ref.get());
+        assertEquals("Send the final handoff through fleet_reply.\n",
+                ref.get().fleet().charterFor(dev.ltms.fleet.peer.MemberRole.DEV));
+    }
+
+    /**
+     * fleetd #474 acceptance criterion 3, the positive case: a reload whose charter names only
+     * registered tools must still be ACCEPTED. An inverted filter (one that refuses every charter,
+     * or refuses on any {@code fleet_*}/{@code bridge_*} token regardless of registration) would pass
+     * the refusal test above alone — #469's own M5 mutation cell showed exactly that shape surviving
+     * a negative-only suite. This is the test that catches it.
+     */
+    @Test
+    void aReloadAcceptsACharterNamingOnlyRegisteredTools(@TempDir Path dir) throws Exception {
+        Path f = dir.resolve("fleetd.yaml");
+        Files.writeString(f, yaml("""
+                fleet:
+                  charters:
+                    dev: old charter, no tool names
+                """));
+        ConfigRef ref = refForWithCharterToolSurface(f);
+
+        Files.writeString(f, yaml("""
+                fleet:
+                  charters:
+                    dev: |
+                      Send the final handoff through fleet_reply, using fleet_send to delegate.
+                """));
+        ConfigRef.Outcome out = ref.reload();
+
+        assertTrue(out.applied());
+        assertTrue(out.deferred().isEmpty(), out.deferred().toString());
+        assertEquals("config reloaded", out.summary());
+        assertEquals("Send the final handoff through fleet_reply, using fleet_send to delegate.\n",
+                ref.get().fleet().charterFor(dev.ltms.fleet.peer.MemberRole.DEV));
+    }
+
     /**
      * 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.