Merge #315: the fixed placement policy honours the retry loop's unreachable set

fleetd/src/main/java/dev/ltms/fleet/member/CompositePeerLauncher.java

@@ -385,9 +385,12 @@ public final class CompositePeerLauncher implements PeerLauncher {
             }
         }
 
+        // unreachable.size() counts DISTINCT profiles, not attempts (a HashSet dedupes a profile
+        // added twice) — say "distinct" so the count matches the sentence and the profile list that
+        // follows, rather than reading as a count of attempts made (fleetd #315).
         throw new PeerUnreachableException(
                 "no reachable worker profile available after trying " + unreachable.size()
-                        + " candidate(s): " + String.join(", ", unreachable));
+                        + " distinct candidate(s): " + String.join(", ", unreachable));
     }
 
     /**

fleetd/src/main/java/dev/ltms/fleet/placement/FixedPlacementPolicy.java

@@ -5,10 +5,14 @@ import java.util.List;
 
 /**
  * Backward-compatible placement: an unqualified spawn always resolves to the configured default
- * profile, exactly as {@code CompositePeerLauncher} did before CB-518. This ignores caps and
- * reachability so that a pre-existing config behaves identically after upgrade.
+ * profile, exactly as {@code CompositePeerLauncher} did before CB-518. This ignores caps
+ * ({@code maxLoad}) so that a pre-existing config behaves identically after upgrade — capacity
+ * gating for automatic placement is deliberately out of scope for {@code fixed}, exactly as it
+ * always has been. Reachability is a narrower exception (fleetd #315, below): a profile is never
+ * checked for reachability up front, only skipped once it has already failed in <em>this same</em>
+ * spawn call's retry loop — see the unreachable case below.
  *
- * <p>Three exceptions walk past the default instead of returning it unconditionally:
+ * <p>Four exceptions walk past the default instead of returning it unconditionally:
  * <ul>
  *   <li>Quarantine (CB-578 stage B): a quarantined default is a credential that just refused on
  *       a usage limit, not a transient capacity or reachability concern.
@@ -16,13 +20,21 @@ import java.util.List;
  *       ({@code BackendOutagePolicy}) — a separate, shorter-lived source from quarantine. When a
  *       profile is both quarantined and cooling off, only the quarantine reason is reported
  *       (exhaustion takes priority), matching {@code CompositePeerLauncher}'s explicit-spawn order.
+ *   <li>Unreachable (fleetd #315): {@code CompositePeerLauncher.spawn} retries a failed candidate
+ *       on the next one and rebuilds the {@link PlacementContext} so {@code ctx.unreachable()}
+ *       names every profile that already failed with {@code PeerUnreachableException} in this same
+ *       call. Without this check {@code select} kept handing back the same dead default forever —
+ *       the retry loop's own comment says "so the policy excludes this profile", and this is what
+ *       makes that true for {@code fixed} too, matching {@code weighted}/{@code round-robin}
+ *       (both filter on {@code ctx.unreachable()} via {@link PlacementPolicyUtil#available}).
  *   <li>Weight 0 (CB-554): {@code fixed} is still automatic selection, so a profile the operator
  *       marked "never auto-select me" ({@code weight <= 0}) must be skipped here exactly as
  *       {@code weighted}/{@code round-robin} skip it — an explicit {@code fleet_spawn} naming
  *       the profile is unaffected, only this automatic fallback walk.
  * </ul>
- * A fleet where nothing is ever quarantined, cooling off, or weight-0 never exercises any of these
- * paths, so today's behaviour is unchanged.
+ * A fleet where nothing is ever quarantined, cooling off, unreachable, or weight-0 never exercises
+ * any of these paths, so today's behaviour is unchanged — in particular, the very first selection
+ * of a spawn call always sees an empty {@code unreachable} set, so the first choice is untouched.
  */
 final class FixedPlacementPolicy implements PlacementPolicy {
 
@@ -30,12 +42,12 @@ final class FixedPlacementPolicy implements PlacementPolicy {
     public PlacementCandidate select(PlacementContext ctx) {
         String d = ctx.defaultProfile();
         if (d != null && !d.isBlank() && !ctx.quarantined().contains(d) && !ctx.coolingOff().contains(d)
-                && !weightExcluded(ctx, d)) {
+                && !ctx.unreachable().contains(d) && !weightExcluded(ctx, d)) {
             return new PlacementCandidate(d, null, 1.0f, null);
         }
         for (PlacementCandidate c : ctx.candidates()) {
             if (!ctx.quarantined().contains(c.profile()) && !ctx.coolingOff().contains(c.profile())
-                    && !c.excluded()) {
+                    && !ctx.unreachable().contains(c.profile()) && !c.excluded()) {
                 return new PlacementCandidate(c.profile(), null, c.weight(), c.maxLoad());
             }
         }
@@ -44,8 +56,9 @@ final class FixedPlacementPolicy implements PlacementPolicy {
             // Exhaustion quarantine takes priority: reported only when quarantine is absent, so the
             // message never claims "cooling off" for a profile that is really backend-exhausted.
             boolean dCoolingOff = !dQuarantined && ctx.coolingOff().contains(d);
+            boolean dUnreachable = ctx.unreachable().contains(d);
             boolean dWeightExcluded = weightExcluded(ctx, d);
-            if (dQuarantined || dCoolingOff || dWeightExcluded) {
+            if (dQuarantined || dCoolingOff || dUnreachable || dWeightExcluded) {
                 List<String> reasons = new ArrayList<>();
                 if (dQuarantined) {
                     reasons.add("is quarantined (backend exhausted)");
@@ -53,6 +66,9 @@ final class FixedPlacementPolicy implements PlacementPolicy {
                 if (dCoolingOff) {
                     reasons.add("is cooling off after repeated backend errors");
                 }
+                if (dUnreachable) {
+                    reasons.add("is unreachable");
+                }
                 if (dWeightExcluded) {
                     reasons.add("has weight 0 (excluded from automatic selection)");
                 }
@@ -62,7 +78,7 @@ final class FixedPlacementPolicy implements PlacementPolicy {
         }
         if (!ctx.candidates().isEmpty()) {
             throw new PlacementException("all worker profiles are excluded from automatic "
-                    + "selection (quarantined, cooling off, or weight-0)");
+                    + "selection (quarantined, cooling off, unreachable, or weight-0)");
         }
         throw new PlacementException("no worker profiles configured");
     }

fleetd/src/test/java/dev/ltms/fleet/member/CompositePeerLauncherTest.java

@@ -615,6 +615,67 @@ class CompositePeerLauncherTest {
         assertEquals(1, adapter.spawnCount("b"));
     }
 
+    /**
+     * fleetd #315: {@code CompositePeerLauncher.spawn} rebuilds the {@link PlacementContext} after
+     * every failed attempt "so the policy excludes this profile" (see the comment at the retry call
+     * site) — but {@code FixedPlacementPolicy} never read {@code ctx.unreachable()}, so under the
+     * default {@code fixed} placement every retry re-picked the same dead default and a second,
+     * healthy, configured profile was never tried. This is the same scenario as
+     * {@link #failoverRetriesNextCandidateWhenProfileIsUnreachable}, but pinned to {@code fixed()}
+     * instead of {@code weighted()} — the three existing failover tests all use {@code weighted()},
+     * which is exactly why nobody caught this: the retry loop's contract has no coverage under its
+     * own default policy.
+     */
+    @Test
+    void failoverRetriesNextCandidateUnderFixedPlacementWhenProfileIsUnreachable() {
+        FakeHerdr herdr = new FakeHerdr();
+        Map<String, FleetConfig.Profile> profiles = ordered(
+                "a", stubWorker("a"),
+                "b", stubWorker("b"));
+        StubLauncher adapter = new StubLauncher("claude", herdr, profiles, "a", Set.of("a"));
+        CompositePeerLauncher composite = new CompositePeerLauncher(
+                List.of(adapter), "a", profiles, PlacementPolicies.fixed(), _ -> 0);
+
+        PeerHandle h = composite.spawn(new SpawnRequest(null, null, null));
+        assertEquals("b", h.profile(),
+                "fixed placement must fail over from the unreachable default a to the healthy b");
+        assertEquals(1, adapter.spawnCount("a"), "a was tried once and failed");
+        assertEquals(1, adapter.spawnCount("b"), "b was tried once and succeeded");
+    }
+
+    /**
+     * fleetd #315: the same fix — {@code FixedPlacementPolicy} consulting {@code ctx.unreachable()}
+     * — also covers the wiring-bug branch in {@code CompositePeerLauncher.spawn}: a profile that
+     * placement is allowed to choose (it is in the configured candidate list) but that no delegate
+     * declares ({@code byProfile.get(chosen.profile()) == null}). That branch adds the profile to
+     * {@code unreachable} and {@code continue}s without ever calling a launcher, so before this fix
+     * {@code fixed} handed back the same adapterless profile on every remaining attempt too.
+     */
+    @Test
+    void failoverSkipsAConfiguredProfileNoAdapterDeclaresUnderFixedPlacement() {
+        FakeHerdr herdr = new FakeHerdr();
+        // Placement's candidate list has three profiles, in this order (LinkedHashMap preserves it,
+        // and the fixed default resolves to the first — see the `ordered` helper's own javadoc).
+        Map<String, FleetConfig.Profile> profiles = new LinkedHashMap<>();
+        profiles.put("c", stubWorker("c"));
+        profiles.put("a", stubWorker("a"));
+        profiles.put("b", stubWorker("b"));
+        // The adapter only declares a and b — c is a configured profile with no owning adapter,
+        // the "wiring bug" the comment in CompositePeerLauncher.spawn calls out.
+        Map<String, FleetConfig.Profile> adapterProfiles = new LinkedHashMap<>();
+        adapterProfiles.put("a", stubWorker("a"));
+        adapterProfiles.put("b", stubWorker("b"));
+        StubLauncher adapter = new StubLauncher("claude", herdr, adapterProfiles, "a", Set.of());
+        CompositePeerLauncher composite = new CompositePeerLauncher(
+                List.of(adapter), "a", profiles, PlacementPolicies.fixed(), _ -> 0);
+
+        PeerHandle h = composite.spawn(new SpawnRequest(null, null, null));
+        assertEquals("a", h.profile(),
+                "c has no adapter, so fixed placement must skip it and land on the next candidate, a");
+        assertEquals(0, adapter.spawnCount("c"), "c is never spawned — no adapter owns it");
+        assertEquals(1, adapter.spawnCount("a"));
+    }
+
     @Test
     void explicitSpawnAtMaxLoadThrowsPlacementExceptionNamingProfileLiveAndCap() {
         FakeHerdr herdr = new FakeHerdr();