fleetd #518: make the FleetMcp caller-resolution wiring an explicit choice, and test it once for real

FleetMcp's contextExtractor picked its principal-resolution path off `callers == null`, so
"authorization off" also silently swapped in a second, untested identity heuristic
(legacyPrincipal). Nothing drove that closure through a real MCP request, so the whole wiring
was an unexercised claim.

- callers (CallerResolver) is now required, never null.
- A new AuthorizationMode enum (ENFORCED/UNENFORCED) is a required constructor parameter with
  no default, replacing the null-means-legacy idiom for whether denyFor enforces at all.
- legacyPrincipal is deleted: there is exactly one resolution path now
  (callers.resolve(...)), so the mutation that swapped it for an unconditional legacy call no
  longer compiles ("cannot find symbol: method legacyPrincipal").
- FleetMcpContextExtractorTest boots the real transport on a real Jetty server and drives it
  with a real MCP client, proving fleet_whoami's resolved role comes from CallerResolver's
  token check.
- Adapted FleetMcpAuthzTest/FleetMcpHandoverTest call sites; theLegacyConstructorLeavesTheGateOpen
  keeps its meaning under the new AuthorizationMode.UNENFORCED value.

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

@@ -694,7 +694,7 @@ public final class Fleetd {
         }, outagePolicy);
 
         FleetMcp mcp = new FleetMcp(messages, workers, sessions, identity, presence,
-                primaryRegistry, callers, metrics,
+                primaryRegistry, callers, FleetMcp.AuthorizationMode.ENFORCED, metrics,
                 capacitySource(config, cfg, profile -> liveCountRef.get().apply(profile)),
                 new FleetMcp.HealthCoverageSource(() -> {
                     var health = config.get().health();

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

@@ -93,7 +93,18 @@ public final class FleetMcp {
 
     private final HttpServletStreamableServerTransportProvider transport;
     private final McpSyncServer server;
-    private final CallerResolver authz; // CB-501: null → authorization not enforced (legacy)
+    /**
+     * fleetd #518: whether {@link #denyFor} enforces the CB-505 policy table at all. Replaces the
+     * old {@code CallerResolver authz} field, whose null-ness used to decide BOTH this AND which
+     * principal-resolution code path {@link #contextExtractor} ran — reaching "authorization off"
+     * by simply not passing a {@link CallerResolver} also meant the resolved {@link Principal}
+     * came from a second, separately-maintained heuristic ({@code legacyPrincipal}, now deleted)
+     * that nothing ever exercised. There is now exactly one resolution path ({@code callers},
+     * required and non-null below) and a separate, explicitly-chosen {@link AuthorizationMode}
+     * for this flag — so a caller can turn enforcement off without silently swapping in a second,
+     * untested identity heuristic.
+     */
+    private final boolean authorizationEnforced;
     private final Metrics metrics;      // CB-502: null → auth failures not counted
     private final CapacitySource capacity;
     private final HealthCoverageSource healthCoverage;
@@ -250,6 +261,17 @@ public final class FleetMcp {
         public static CoordinationSource none() { return new CoordinationSource(null, List.of()); }
     }
 
+    /**
+     * fleetd #518: whether {@link #denyFor} enforces the CB-505 policy table. A required
+     * constructor parameter with no default, so "authorization is off" can only be reached by a
+     * caller explicitly saying so — never by omitting a {@link CallerResolver} the way the old
+     * {@code callers == null} idiom allowed. {@code callers} itself is required either way: even
+     * under {@link #UNENFORCED}, the one real {@link CallerResolver} still resolves every caller's
+     * {@link Principal} (so {@code markSpawnedMemberPresent}/{@code recordPrimarySingleton} see a
+     * real identity), and {@link #denyFor} is the only thing that changes.
+     */
+    public enum AuthorizationMode { ENFORCED, UNENFORCED }
+
     /**
      * The only constructor (fleetd #480 Unit C correction round). Every field below used to have
      * its own defaulting overload — {@code leadChannel}/{@code outage}/{@code leadSeats}/
@@ -268,10 +290,16 @@ public final class FleetMcp {
      * {@link OutageSource#none()}, {@link LeadSeatSource#none()}, {@code List.of()} are all still
      * perfectly fine values, just never an implicit default reached by omission.
      *
-     * @param callers      resolves each call's {@link Principal}; {@code null} disables
-     *                     authorization. This surface needs its own enforcement: {@code /mcp} is a
-     *                     raw servlet on Jetty's context handler and never passes through
-     *                     Javalin's {@code before} filter, so the REST guard does not cover it.
+     * @param callers      resolves each call's {@link Principal}. Required, never {@code null} —
+     *                     fleetd #518: use {@link AuthorizationMode#UNENFORCED} to disable
+     *                     enforcement, not a missing resolver. This surface needs its own
+     *                     enforcement: {@code /mcp} is a raw servlet on Jetty's context handler and
+     *                     never passes through Javalin's {@code before} filter, so the REST guard
+     *                     does not cover it.
+     * @param authorizationMode fleetd #518: whether {@link #denyFor} enforces the CB-505 policy
+     *                     table ({@link AuthorizationMode#ENFORCED}) or leaves the gate open
+     *                     ({@link AuthorizationMode#UNENFORCED}, for the pre-CB-513 test suite that
+     *                     does not exercise authorization). Required, with no default.
      * @param metrics      registry for auth-failure counting; may be {@code null}
      * @param quarantine   CB-578 stage B facts for {@code fleet_profiles}; pass
      *                     {@link QuarantineSource#none()} for a caller that does not want the
@@ -301,9 +329,13 @@ public final class FleetMcp {
      */
     public FleetMcp(MessageService messages, PeerLauncher workers, SessionManager sessions,
                      ConnectionIdentity identity, MemberPresence presence, PrimaryRegistry primaryRegistry,
-                      CallerResolver callers, Metrics metrics, CapacitySource capacity, HealthCoverageSource healthCoverage,
+                      CallerResolver callers, AuthorizationMode authorizationMode, Metrics metrics,
+                      CapacitySource capacity, HealthCoverageSource healthCoverage,
                       QuarantineSource quarantine, LeadChannel leadChannel, OutageSource outage,
                       LeadSeatSource leadSeats, List<String> peers, LeadRollover leadRollover) {
+        Objects.requireNonNull(callers, "callers");
+        this.authorizationEnforced = Objects.requireNonNull(authorizationMode, "authorizationMode")
+                == AuthorizationMode.ENFORCED;
         this.leadChannel = leadChannel;
         this.peers = peers == null ? List.of() : List.copyOf(peers);
         this.capacity = capacity;
@@ -322,11 +354,11 @@ public final class FleetMcp {
                 // (CB-113) — its MCP initialize is the reliable "the agent is up" signal.
                 .contextExtractor(req -> {
                     // One resolution per call, shared with the REST surface via CallerResolver so
-                    // the two paths cannot drift on who a caller is.
-                    Principal p = callers != null
-                            ? callers.resolve(req.getRemoteAddr(), req.getRemotePort(),
-                                    req.getHeader("Authorization"))
-                            : legacyPrincipal(identity, req.getRemoteAddr(), req.getRemotePort());
+                    // the two paths cannot drift on who a caller is. fleetd #518: callers is
+                    // required (never null) so there is no second, untested resolution path to
+                    // fall back to here — AuthorizationMode governs enforcement, not identity.
+                    Principal p = callers.resolve(req.getRemoteAddr(), req.getRemotePort(),
+                            req.getHeader("Authorization"));
                     // CB-532: guard on the ROLE, not on the terminal being null. This excludes a
                     // lead, which carries its pane too, while including every spawned member role.
                     // Enrolling a lead would count it as an available member in the roster.
@@ -443,7 +475,7 @@ public final class FleetMcp {
                     McpSchema.CallToolResult denied = deny(exchange, toolAction("fleet_list", Map.of()), null);
                     if (denied != null) return denied;
                     return listFleet(workers, sessions, messages, capacity, healthCoverage, quarantine, outage,
-                            leadSeats, callers == null ? Map.of() : callers.leads(),
+                            leadSeats, callers.leads(),
                             callerTerminal(exchange),
                             new CoordinationSource(leadChannel, peers),
                             coordinatorVisibleTo(principal(exchange)));
@@ -522,35 +554,9 @@ public final class FleetMcp {
                 .toolCall(fleetWhoami, whoamiHandler)
                 .toolCall(fleetHandover, handoverHandler)
                 .build();
-        this.authz = callers;
         this.metrics = metrics;
     }
 
-    /**
-     * Pre-CB-501 identity: worker if the connection maps to a pane, otherwise anonymous. Used
-     * only by the legacy constructor ({@code callers == null}), where authorization is not
-     * enforced anyway — but the resolved {@link Principal} still reaches non-authz logic (e.g.
-     * {@code markSpawnedMemberPresent}, {@code recordPrimarySingleton}), so it must not be trusted
-     * with a role it did not earn.
-     *
-     * <p>fleetd #509: this used to fall back to {@link Principal#primary}, unconditionally, for
-     * every caller the connection did not resolve to a worker pane — with none of
-     * {@code CallerResolver.java:254}'s two guards ({@code isLoopback}, {@code scanComplete}).
-     * That is the exact shape #317 and #505 each closed on the enforced path; this branch was the
-     * same trap, left open on the legacy one. It now returns {@link Principal#anonymous} instead,
-     * so an unresolved legacy caller earns no authority rather than the primary's.
-     *
-     * <p>Package-private (was {@code private}) so this is unit-testable directly, the same reason
-     * {@link #denyFor} was split out — it runs inside a contextExtractor closure that only fires on
-     * a real MCP request, so nothing else could pin this behaviour.
-     */
-    static Principal legacyPrincipal(ConnectionIdentity identity, String addr, int port) {
-        ConnectionIdentity.Caller c = identity.resolve(addr, port);
-        return c.terminal() != null
-                ? Principal.worker(c.terminal(), c.pid())
-                : Principal.anonymous();
-    }
-
     /** The caller reconstructed from the transport context. */
     private static Principal principal(McpSyncServerExchange exchange) {
         return principalFrom(exchange.transportContext().get(CALLER_ROLE),
@@ -605,8 +611,8 @@ public final class FleetMcp {
     McpSchema.CallToolResult denyFor(Principal caller, Authz.Action action, String target) {
         // The enforcement switch lives HERE rather than in the exchange-facing wrapper: any future
         // tool that calls this directly must not be able to skip the gate by accident.
-        if (authz == null) {
-            return null; // legacy constructor: authorization not enforced
+        if (!authorizationEnforced) {
+            return null; // AuthorizationMode.UNENFORCED: authorization not enforced (fleetd #518)
         }
         if (Authz.permits(caller, action, target)) {
             if (action != Authz.Action.READ) {

fleetd/src/test/java/dev/ltms/fleet/mcp/FleetMcpAuthzTest.java

@@ -78,10 +78,15 @@ class FleetMcpAuthzTest {
         // fleetd #480 correction round: FleetMcp has one constructor now (no defaulting
         // overloads — see its javadoc), so every feature this test does not exercise is passed
         // its explicit "off" value here rather than being omitted.
+        //
+        // fleetd #518: callers is now required (never null) either way — the resolver that used
+        // to be omitted to reach "legacy" is now always real, and AuthorizationMode is the
+        // separate, explicit choice that governs enforcement.
         mcp = new FleetMcp(messages, workers, sessions, identity, sessions.asPresence(),
                 new PrimaryRegistry(null),
-                enforce ? CallerResolver.withLeadsAndMembers(identity, false, null,
-                Map::of, new MemberRegistry(null)) : null,
+                CallerResolver.withLeadsAndMembers(identity, false, null,
+                Map::of, new MemberRegistry(null)),
+                enforce ? FleetMcp.AuthorizationMode.ENFORCED : FleetMcp.AuthorizationMode.UNENFORCED,
                 metrics, FleetMcp.CapacitySource.none(), new FleetMcp.HealthCoverageSource(() -> "off"),
                 FleetMcp.QuarantineSource.none(), null, FleetMcp.OutageSource.none(),
                 FleetMcp.LeadSeatSource.none(), List.of(), null);
@@ -187,28 +192,31 @@ class FleetMcpAuthzTest {
         // The 22 pre-existing FleetMcpTest cases rely on no authorization being enforced.
         FleetMcp m = mcp(false);
         assertNull(m.denyFor(ANON, Authz.Action.SPAWN, null),
-                "no CallerResolver supplied ⇒ authorization not enforced (legacy behaviour)");
+                "AuthorizationMode.UNENFORCED chosen explicitly ⇒ authorization not enforced "
+                        + "(legacy behaviour) — fleetd #518 replaced the old callers == null idiom");
     }
 
     /**
-     * fleetd #509: {@code legacyPrincipal} (used only when {@code callers == null}, i.e. the
-     * legacy constructor above) used to fall back to {@link Principal#primary} for ANY caller the
-     * connection did not resolve to a worker pane — no {@code isLoopback} check, no
-     * {@code scanComplete} check, unlike the enforced path's {@code CallerResolver.java:254}. A
-     * non-loopback caller (an off-host client) is exactly the case that must never earn the
-     * primary's authority, and authorization being disabled in legacy mode does not make that
-     * safe: the resolved {@link Principal} still reaches non-authz logic such as
-     * {@code markSpawnedMemberPresent} and {@code recordPrimarySingleton}.
+     * fleetd #509 was originally proven against {@code FleetMcp.legacyPrincipal} — a second,
+     * separately-maintained principal-resolution heuristic that only ran when {@code callers} was
+     * omitted (null). fleetd #518 deleted that whole heuristic: {@code callers} is now required
+     * and non-null under every {@link FleetMcp.AuthorizationMode}, so the ONE real
+     * {@link CallerResolver} resolves every caller, enforced or not, and #509's property (a
+     * non-loopback / unresolved caller must never earn the primary's authority) is exactly what
+     * {@code CallerResolverTest.aNonLoopbackCallerIsNeverThePrimaryUnderLoopbackTrust} already
+     * proves on that one real path. There is no longer a second heuristic here to test.
      */
     @Test
-    void legacyPrincipalIsAnonymousNotPrimaryForAnUnresolvedCaller() {
+    void anUnresolvedNonLoopbackCallerIsAnonymousUnderTheOneRealResolver() {
         ConnectionIdentity identity = new ConnectionIdentity(new PaneLocator(herdr), _ -> 999_999);
+        CallerResolver resolver = CallerResolver.withLeadsAndMembers(identity, false, null,
+                Map::of, new MemberRegistry(null));
         // A non-loopback address never even reaches the pane scan — resolve() short-circuits it
         // to Caller(null, -1, true), the same "no terminal" shape a genuine primary's connection
-        // produces. legacyPrincipal must not conflate the two.
-        Principal p = FleetMcp.legacyPrincipal(identity, "8.8.8.8", 1234);
+        // produces on loopback. The real resolver must not conflate the two.
+        Principal p = resolver.resolve("8.8.8.8", 1234, null);
         assertEquals(Principal.anonymous(), p,
-                "an unresolved legacy caller must earn no authority, not the primary's");
+                "an unresolved, non-loopback caller must earn no authority, not the primary's");
     }
 
     // --- fleetd #439: who may see fleet_list's coordinator row ----------------------------------

fleetd/src/test/java/dev/ltms/fleet/mcp/FleetMcpContextExtractorTest.java

@@ -0,0 +1,147 @@
+package dev.ltms.fleet.mcp;
+
+import dev.ltms.fleet.auth.CallerResolver;
+import dev.ltms.fleet.auth.MemberRegistry;
+import dev.ltms.fleet.config.FleetConfig;
+import dev.ltms.fleet.guard.SubscriptionGuard;
+import dev.ltms.fleet.herdr.AgentControl;
+import dev.ltms.fleet.herdr.FakeHerdr;
+import dev.ltms.fleet.herdr.PaneLocator;
+import dev.ltms.fleet.herdr.WorkspaceControl;
+import dev.ltms.fleet.inject.Injector;
+import dev.ltms.fleet.member.ClaudeCodeLauncher;
+import dev.ltms.fleet.msg.InMemoryReplyInbox;
+import dev.ltms.fleet.msg.MessageService;
+import dev.ltms.fleet.msg.Rendezvous;
+import dev.ltms.fleet.session.FakeWorktrees;
+import dev.ltms.fleet.session.SessionManager;
+import io.modelcontextprotocol.client.McpClient;
+import io.modelcontextprotocol.client.McpSyncClient;
+import io.modelcontextprotocol.client.transport.HttpClientStreamableHttpTransport;
+import io.modelcontextprotocol.spec.McpClientTransport;
+import io.modelcontextprotocol.spec.McpSchema;
+import org.eclipse.jetty.server.Server;
+import org.eclipse.jetty.server.ServerConnector;
+import org.eclipse.jetty.servlet.ServletContextHandler;
+import org.eclipse.jetty.servlet.ServletHolder;
+import org.junit.jupiter.api.AfterEach;
+import org.junit.jupiter.api.Test;
+
+import java.net.http.HttpRequest;
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
+
+import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+
+/**
+ * fleetd #518 — Part 2: drive the {@code contextExtractor} closure for real.
+ *
+ * <p>{@code FleetMcp.deny()}/{@code denyFor()} has a full policy table of tests
+ * ({@code FleetMcpAuthzTest}), and {@code CallerResolver.resolve()} has its own full suite
+ * ({@code CallerResolverTest}). Neither one ever exercises the closure that WIRES them together
+ * inside {@code FleetMcp}'s constructor: it is built once, handed to the MCP SDK's transport, and
+ * only ever runs when a real MCP client makes a real HTTP request. Every existing test either
+ * calls {@code denyFor(Principal, ...)} with a hand-built {@link dev.ltms.fleet.auth.Principal}
+ * (never asking who the transport would actually have resolved) or drives a static handler method
+ * directly. A mutation that swapped the whole resolution decision for an unconditional fallback —
+ * bypassing {@link CallerResolver} entirely — passed the full suite, including every
+ * {@code FleetMcpAuthzTest} case, because none of them go through the transport at all.
+ *
+ * <p>This test boots the real {@code HttpServletStreamableServerTransportProvider} on a real
+ * Jetty server, drives it with a real MCP client over HTTP, and checks a result that only the
+ * real {@link CallerResolver} can produce: token-mode inspects the {@code Authorization} header
+ * and grants {@code PRIMARY} only for the right bearer token. The connection never resolves to a
+ * worker pane (the fake peer-pid lookup always misses), so the ONLY way {@code fleet_whoami} can
+ * come back as {@code primary} is if the closure actually called {@code callers.resolve(...)} and
+ * read that header — a behaviour the deleted {@code legacyPrincipal} heuristic never had at all.
+ */
+class FleetMcpContextExtractorTest {
+
+    private static final String TOKEN = "s3cret-mcp-token";
+
+    private final FakeHerdr herdr = new FakeHerdr();
+    private final AgentControl agents = new AgentControl(herdr);
+    private FleetMcp mcp;
+    private Server server;
+
+    @AfterEach
+    void tearDown() throws Exception {
+        if (server != null) {
+            server.stop();
+        }
+        if (mcp != null) {
+            mcp.close();
+        }
+    }
+
+    @Test
+    void aRealMcpRequestIsResolvedByTheRealCallerResolverNotAFallback() throws Exception {
+        FleetConfig.Profile cfg = new FleetConfig.Profile(
+                "ltms-local", "http://gx00.gw:8000", "coder", null, "FLEETD_WORKER_TOKEN", null,
+                "tab", "fleetd-workers", "worker: {profile} #{n}", null, null, null);
+        ClaudeCodeLauncher workers = new ClaudeCodeLauncher(agents, new WorkspaceControl(herdr),
+                new SubscriptionGuard(Set.of("gx00.gw")), Map.of(cfg.profile(), cfg), cfg.profile(),
+                _ -> "tok");
+        SessionManager sessions = new SessionManager(workers, new FakeWorktrees());
+        MessageService messages = new MessageService(agents, new Injector(agents), new Rendezvous(),
+                new InMemoryReplyInbox());
+        // The peer-pid lookup always misses (-1), so no connection here is ever resolved to a
+        // worker pane — every call falls through to CallerResolver's token check, the one branch
+        // that is unreachable through the deleted legacy heuristic.
+        ConnectionIdentity identity = new ConnectionIdentity(new PaneLocator(herdr), _ -> -1);
+        CallerResolver callers = CallerResolver.withLeadsAndMembers(identity, true, TOKEN,
+                Map::of, new MemberRegistry(null));
+
+        mcp = new FleetMcp(messages, workers, sessions, identity, sessions.asPresence(),
+                new PrimaryRegistry(null), callers, FleetMcp.AuthorizationMode.ENFORCED,
+                null, FleetMcp.CapacitySource.none(), new FleetMcp.HealthCoverageSource(() -> "off"),
+                FleetMcp.QuarantineSource.none(), null, FleetMcp.OutageSource.none(),
+                FleetMcp.LeadSeatSource.none(), List.of(), null);
+
+        ServletContextHandler handler = new ServletContextHandler();
+        handler.setContextPath("/");
+        handler.addServlet(new ServletHolder(mcp.servlet()), "/mcp");
+        server = new Server(0);
+        server.setHandler(handler);
+        server.start();
+        String baseUrl = "http://127.0.0.1:"
+                + ((ServerConnector) server.getConnectors()[0]).getLocalPort();
+
+        // The right bearer token: the real CallerResolver grants PRIMARY, which fleet_whoami's
+        // READ gate lets through.
+        McpSchema.CallToolResult authorized = callWhoami(baseUrl, "Bearer " + TOKEN);
+        assertFalse(authorized.isError(), "a valid bearer token must resolve as PRIMARY and pass "
+                + "fleet_whoami's READ gate: " + textOf(authorized));
+        assertTrue(textOf(authorized).contains("\"role\":\"primary\""),
+                "fleet_whoami must report the role the real CallerResolver resolved over this "
+                        + "connection, not a fallback: " + textOf(authorized));
+
+        // No credential at all, over the SAME wiring: the real resolver refuses it as ANONYMOUS.
+        // legacyPrincipal never looked at the Authorization header, so it could not have told
+        // these two calls apart at all -- this is the assertion the deleted mutation would fail.
+        McpSchema.CallToolResult unauthorized = callWhoami(baseUrl, null);
+        assertTrue(unauthorized.isError(), "no credential must be refused, not silently let "
+                + "through: " + textOf(unauthorized));
+    }
+
+    private static McpSchema.CallToolResult callWhoami(String baseUrl, String authorizationHeader) {
+        HttpRequest.Builder requestTemplate = HttpRequest.newBuilder();
+        if (authorizationHeader != null) {
+            requestTemplate.header("Authorization", authorizationHeader);
+        }
+        McpClientTransport transport = HttpClientStreamableHttpTransport.builder(baseUrl)
+                .endpoint("/mcp")
+                .requestBuilder(requestTemplate)
+                .build();
+        try (McpSyncClient client = McpClient.sync(transport).build()) {
+            client.initialize();
+            return client.callTool(McpSchema.CallToolRequest.builder("fleet_whoami").arguments(Map.of()).build());
+        }
+    }
+
+    private static String textOf(McpSchema.CallToolResult r) {
+        return ((McpSchema.TextContent) r.content().getFirst()).text();
+    }
+}

fleetd/src/test/java/dev/ltms/fleet/mcp/FleetMcpHandoverTest.java

@@ -89,6 +89,7 @@ class FleetMcpHandoverTest {
         mcp = new FleetMcp(messages, workers, sessions, identity, sessions.asPresence(),
                 new PrimaryRegistry(null),
                 CallerResolver.withLeadsAndMembers(identity, false, null, Map::of, new MemberRegistry(null)),
+                FleetMcp.AuthorizationMode.ENFORCED,
                 null, FleetMcp.CapacitySource.none(), new FleetMcp.HealthCoverageSource(() -> "off"),
                 FleetMcp.QuarantineSource.none(), null, FleetMcp.OutageSource.none(),
                 FleetMcp.LeadSeatSource.none(), List.of(), leadRollover);

scripts/probe-member-credentials.sh

@@ -65,6 +65,27 @@
 # credential the member holds in full.
 #
 set -uo pipefail
+# `pipefail` is not what catches the parser failure handled below (fleetd #500): in
+# `printf '%s' "$POLICY_JSON" | jq -r '...'`, jq is the LAST element of the pipe, so the pipeline's
+# own exit status is already jq's status, with or without pipefail. It is kept as insurance for if
+# a post-processing stage is ever appended after the parser (e.g. `| tail -n +2`) — at that point
+# the parser would sit upstream and pipefail becomes the only thing that still reports its status.
+
+# --- refuse on an interpreter that cannot run this script (fleetd #500) -------------------------
+#
+# mapfile, used below to parse the policy response, was added in bash 4.0. macOS ships bash 3.2.57
+# at /bin/bash, which predates it. This script's own `set -uo pipefail` does not catch a missing
+# mapfile: the builtin just fails with "command not found" on stderr, and every line below that
+# reads the array it would have filled uses a `:-` default or a slice, neither of which `set -u`
+# catches on an unset array. Left unguarded, that chain ends in the "0 known names" refusal further
+# down — a claim about the POLICY, for a failure that is actually about the INTERPRETER. So the
+# interpreter is checked once, explicitly, before it is asked to do anything mapfile depends on.
+if (( ${BASH_VERSINFO[0]} < 4 )); then
+  echo "refusing to run: this script uses mapfile, which needs bash 4 or newer. This shell is bash" \
+       "${BASH_VERSION:-<unknown, no \$BASH_VERSION>}. Re-run it under a newer bash, for example:" \
+       "\"\$(command -v bash)\" \"$0\"" "$@" >&2
+  exit 3
+fi
 
 FLEETD_HOST="${FLEETD_HOST:-http://127.0.0.1:8765}"
 POLICY_URL="${FLEETD_HOST%/}/member-credentials"
@@ -124,16 +145,32 @@ fi
 # One parse pass: line 1 = present (true/false/null), line 2 = policy mode (possibly blank),
 # lines 3-5 = knownCount/allowedCount/blockedCount, remaining lines = the known[] names. A single
 # pass avoids re-parsing (and re-risking a truthiness bug) five separate times.
+#
+# This used to feed the parser straight into `mapfile -t _FIELDS < <(producer)`. That form cannot
+# see the producer fail: `<` `<(...)` is a process substitution, not a pipeline, so `set -o
+# pipefail` does not reach inside it, and mapfile's own exit status reports whether the BUILTIN
+# ran, not whether the command substituted into it succeeded — a failing jq or python3 there still
+# leaves mapfile at rc=0 with an empty array, read as a parse that genuinely found nothing (fleetd
+# #500). Capturing the parser's output with command substitution first, and checking ITS exit
+# status, reports the producer's real failure while the fact still exists — before it is handed to
+# mapfile at all.
+#
+# mapfile then reads from that captured string with `<<<` (a herestring), not `< <(...)`: `<<<`
+# materialises the whole string in memory first, where `< <(...)` would stream it. That only
+# matters for a large producer; this one is a short credential-name policy response, so the
+# tradeoff is irrelevant here — noted because it would not be for every producer.
 if command -v jq >/dev/null 2>&1; then
-  mapfile -t _FIELDS < <(printf '%s' "$POLICY_JSON" | jq -r '
+  _FIELDS_RAW="$(printf '%s' "$POLICY_JSON" | jq -r '
       (.present | tostring),
       (.policy // ""),
       (.knownCount // 0 | tostring),
       (.allowedCount // 0 | tostring),
       (.blockedCount // 0 | tostring),
-      (.known[]? // empty)')
+      (.known[]? // empty)')"
+  _PARSE_STATUS=$?
+  _PARSER_NAME="jq"
 else
-  mapfile -t _FIELDS < <(printf '%s' "$POLICY_JSON" | python3 - <<'PY'
+  _FIELDS_RAW="$(printf '%s' "$POLICY_JSON" | python3 - <<'PY'
 import json, sys
 data = json.load(sys.stdin)
 print(str(data.get("present")))
@@ -144,7 +181,34 @@ print(data.get("blockedCount") if data.get("blockedCount") is not None else 0)
 for n in (data.get("known") or []):
     print(n)
 PY
-  )
+  )"
+  _PARSE_STATUS=$?
+  _PARSER_NAME="python3"
+fi
+
+if [ "$_PARSE_STATUS" -ne 0 ]; then
+  echo "refusing to run: could not parse the policy fetched from $POLICY_URL — $_PARSER_NAME exited" \
+       "non-zero (status $_PARSE_STATUS). That is a parser failure, not a claim about the policy" \
+       "itself; the policy response has not been read." >&2
+  exit 4
+fi
+
+mapfile -t _FIELDS <<< "$_FIELDS_RAW"
+
+# Arity check — the CORRECTNESS fix (fleetd #500). A parser that exits 0 can still return fewer
+# than the 5 fixed fields (present, policy mode, 3 counts) that every line below this expects,
+# whatever the reason: a producer that printed nothing, malformed JSON that jq/python3 still
+# accepted, or a schema change upstream. The slice just below this (`_FIELDS[@]:5`) does not fire
+# `set -u` on an unset OR a short array, and every fixed-field read above used a `:-` default, so
+# without this check a short `_FIELDS` reaches the "0 known names" guard further down with the
+# same look as a policy that genuinely has 0 names. Check the count here, at the one point the
+# fact is still present, before the slice consumes it.
+if (( ${#_FIELDS[@]} < 5 )); then
+  echo "refusing to run: the policy parser ($_PARSER_NAME) returned ${#_FIELDS[@]} field(s); at" \
+       "least 5 are required (present, policy mode, knownCount, allowedCount, blockedCount). The" \
+       "parse ran but its shape is wrong — this is not a claim about how many names the policy" \
+       "knows." >&2
+  exit 5
 fi
 
 PRESENT="${_FIELDS[0]:-null}"
@@ -164,19 +228,21 @@ case "$KNOWN_COUNT_REPORTED" in
     ;;
 esac
 
-# --- guard the denominator explicitly — never proceed on a zero/short count ---------------------
+# --- guard the denominator explicitly — never proceed on a zero count ---------------------------
 #
-# This is the exact trap named in the ticket: an empty (or truncated) NAMES array passes every
-# subsequent "is it set" check vacuously and prints a table that LOOKS complete. So this is checked
-# before anything else runs, with a message that says why, not just that it failed.
+# This is the exact trap named in the ticket: an empty NAMES array passes every subsequent "is it
+# set" check vacuously and prints a table that LOOKS complete. By this point the interpreter gate,
+# the parser-exit-status check, and the arity check above have already ruled out "the interpreter
+# couldn't run mapfile", "the parser failed", and "the parser returned the wrong shape" — so a zero
+# count reaching here really does mean the policy itself reports 0 known names, not a swallowed
+# failure upstream. That is still checked before anything else runs, with a message that says so.
 if [ "${#NAMES[@]}" -eq 0 ] || [ "$KNOWN_COUNT_REPORTED" -eq 0 ]; then
   cat >&2 <<EOF
 refusing to run: the policy fetched from $POLICY_URL contains 0 known names (present=${PRESENT:-unknown}).
 
-Either memberCredentials: is absent/empty on the running daemon (nothing is protected — see fleetd's
-own startup warning), or the response could not be parsed. Either way, checking zero names would
-print a clean-looking table for a policy that protects nothing, or for a probe that read nothing.
-This is refused rather than reported as a pass.
+memberCredentials: is absent or empty on the running daemon — nothing is protected (see fleetd's own
+startup warning). Checking zero names would print a clean-looking table for a policy that protects
+nothing. This is refused rather than reported as a pass.
 EOF
   exit 1
 fi

scripts/redeploy-fleetd.sh

@@ -615,8 +615,9 @@ if [ -n "$OLD_PID" ] && [ "$ASSUME_YES" = 0 ]; then
     # than before this run started, even though the running daemon itself was never touched.
     if [ "$DO_BUILD" = 1 ] && [ -f "$JAR_STAGED" ]; then
       die "aborted — the running daemon was NOT touched, but the freshly built jar is sitting at
-          $JAR_STAGED, not yet swapped into $JAR. Rerun (with or without --no-build) to finish the
-          restart, or remove $JAR_STAGED by hand if you want to discard this build."
+          $JAR_STAGED, not yet swapped into $JAR. Rerun WITHOUT --no-build to finish the restart —
+          the freshly built jar is no longer at the live path that --no-build requires — or
+          remove $JAR_STAGED by hand if you want to discard this build."
     fi
     die "aborted — nothing changed"
   fi

scripts/test-redeploy-fleetd.sh

@@ -206,6 +206,28 @@ test_assert_single_daemon_rejects_two_pids() {
   printf '%s' "$output" | grep -qF '4343' || fail "refusal message does not list the pids it found"
 }
 
+# fleetd #511 — jar_id()'s no-argument default was unpinned by any test: nothing proved it reports
+# $JAR (the live path) rather than $JAR_STAGED. Both halves matter, so this pins both: the bare call
+# must hash the live jar, and an explicit path argument must hash THAT file, not fall back to $JAR.
+# Two files with different content, so a default pointed at the wrong one reports the wrong hash
+# rather than accidentally matching.
+test_jar_id_defaults_to_live_and_reports_explicit_path() {
+  local dir saved_jar="$JAR" saved_staged="$JAR_STAGED"
+  local live_hash staged_hash default_result explicit_result
+  dir="$TMP/jar-id"; mkdir -p "$dir"
+  JAR="$dir/fleetd.jar"; JAR_STAGED="$dir/fleetd-new.jar"
+  printf 'live jar bytes' > "$JAR"
+  printf 'staged jar bytes, not the same content' > "$JAR_STAGED"
+  live_hash="$(shasum -a 256 "$JAR" | cut -c1-12)"
+  staged_hash="$(shasum -a 256 "$JAR_STAGED" | cut -c1-12)"
+  default_result="$(jar_id)"
+  explicit_result="$(jar_id "$JAR_STAGED")"
+  JAR="$saved_jar"; JAR_STAGED="$saved_staged"
+  [ "$live_hash" != "$staged_hash" ] || fail "test fixture error: live and staged jars hashed the same"
+  assert_equals "$live_hash" "$default_result" "jar_id with no arguments must report the hash of \$JAR"
+  assert_equals "$staged_hash" "$explicit_result" "jar_id \"\$JAR_STAGED\" must report the hash of the staged jar, not fall back to \$JAR"
+}
+
 # fleetd #493 — never build into the path a running process holds. stage_built_jar/swap_staged_jar
 # are exercised directly against real files on disk (not stubs), because the whole point is file
 # behavior (does the content move, does the source disappear, does a failure leave both sides
@@ -344,6 +366,26 @@ test_swap_ordered_after_wait_and_before_start() {
     || fail "swap_staged_jar (line $swap_line) is not before the start section (line $start_line)"
 }
 
+# fleetd #511: the drain-gate abort message (fired when a build has staged a jar but the operator
+# declines the drain confirmation) used to tell the operator to "Rerun (with or without --no-build)"
+# to finish the restart. That is wrong — by the time this message can fire, stage_built_jar has
+# already moved the jar off $JAR, so a rerun WITH --no-build hits require_no_build_jar's own refusal
+# ("no jar at $JAR — run without --no-build"). Like test_swap_ordered_after_wait_and_before_start
+# above, this code path is never reached by sourcing (the SOURCED guard stops before the main flow),
+# so the only way to pin its exact wording is to read the source.
+test_drain_gate_abort_message_says_no_no_build() {
+  local src="$ROOT/scripts/redeploy-fleetd.sh" msg
+  msg="$(grep -A3 -F 'aborted — the running daemon was NOT touched, but the freshly built jar is sitting at' "$src")"
+  [ -n "$msg" ] || fail "could not find the drain-gate staged-jar abort message in redeploy-fleetd.sh"
+  if printf '%s' "$msg" | grep -qF 'with or without --no-build'; then
+    fail "abort message still claims a rerun WITH --no-build can finish the restart"
+  fi
+  printf '%s' "$msg" | grep -qF 'WITHOUT --no-build' \
+    || fail "abort message does not tell the operator to rerun without --no-build"
+  printf '%s' "$msg" | grep -qF 'no longer at the live path' \
+    || fail "abort message does not say why --no-build cannot finish the restart"
+}
+
 test_no_errors() {
   cat > "$TMP/no-errors.log" <<'LOG'
 2026-09-05 12:00:00 INFO fleetd listening
@@ -555,6 +597,7 @@ test_require_drivable_supervisor_accepts_known_kinds
 test_count_daemon_pids
 test_assert_single_daemon_accepts_one_pid
 test_assert_single_daemon_rejects_two_pids
+test_jar_id_defaults_to_live_and_reports_explicit_path
 test_stage_built_jar_moves_off_live_path
 test_stage_built_jar_dies_when_build_produced_nothing
 test_swap_staged_jar_moves_staged_onto_live
@@ -565,6 +608,7 @@ test_require_no_build_jar_accepts_present_jar
 test_wait_for_daemon_exit_returns_true_once_pid_clears
 test_wait_for_daemon_exit_times_out_if_pid_never_clears
 test_swap_ordered_after_wait_and_before_start
+test_drain_gate_abort_message_says_no_no_build
 test_no_errors
 test_recovery_patterns_match_source
 test_attributed_recovered_connection_error