Merge #451: make PeerLauncher.spawn(req, decision) abstract (fleetd #450)
CI / contract (push) Successful in 1m17s
CI / build (push) Successful in 1m33s

Verified by the lead on head cfebc57 (base 822327e is current main).

  FULL BUILD  Tests run: 1575, Failures: 0, Errors: 0, Skipped: 0  BUILD SUCCESS
              compile errors: 0

  M1  delete HerdrPeerLauncher's new override
      -> BUILD FAILURE, 1 COMPILATION ERROR block, naming exactly
         ClaudeCodeLauncher.java:[49,14] and OpenCodeLauncher.java:[59,14]

  M2  CONTROL for M1: put the interface method back to a `default` AND delete
      the override
      -> BUILD SUCCESS, 0 compile errors
      This is the row that makes M1 mean something. Without it, M1 only shows
      that the build broke; with it, the break is attributable to the method
      being abstract rather than to anything else the edit disturbed.

  M3  CompositePeerLauncher's override reverts to the re-entering form
      -> KILLED, Errors: 1
      CompositePeerLauncherTest
        .spawnHonorsAPlacementDecisionEvenAfterItsProfileIsQuarantinedInTheWindowAfterPlace
      So #447's guarantee survives this refactor of the interface it rests on.

Tree restored clean after each mutation (git status --porcelain empty).

The worker corrected my ticket, and it was right. My #450 body listed five
src/main implementers of PeerLauncher and quoted `grep -rln 'implements
PeerLauncher'` as the source; that command returns two files. I had run a wider
pattern that also matched a comment in ConfigRef and the `extends
HerdrPeerLauncher` line in two subclasses, then quoted the narrow command beside
the wide command's output. Ground truth: ConfigRef implements
Supplier<FleetConfig>; ClaudeCodeLauncher and OpenCodeLauncher extend
HerdrPeerLauncher. So one override in that parent serves both, which is what the
worker built. The ticket body is corrected.

Out-of-scope note carried forward from the worker: defaultProfileFor(MemberRole)
and place(MemberRole) are two more default methods with the same shape. Noted,
not fixed here.
This commit was merged in pull request #451.
This commit is contained in:
2026-09-10 12:16:47 +02:00
4 changed files with 63 additions and 17 deletions
@@ -17,6 +17,7 @@ import dev.ltms.fleet.peer.PeerHandle;
import dev.ltms.fleet.peer.PeerLauncher;
import dev.ltms.fleet.peer.PeerUnreachableException;
import dev.ltms.fleet.peer.SpawnRequest;
import dev.ltms.fleet.placement.PlacementDecision;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
@@ -594,6 +595,23 @@ public abstract class HerdrPeerLauncher implements PeerLauncher {
req.sessionName(), spawned.agentSessionId(), spawned.receipt());
}
/**
* {@inheritDoc}
*
* <p>fleetd #450: re-enters {@link #spawn(SpawnRequest)} with {@code decision}'s profile named
* explicitly. This is the re-entering form the interface javadoc describes for a launcher with
* no placement concept of its own — an instance of this class spawns a single adapter's own
* profile set by explicit name only ({@link #place}/{@link #defaultProfileFor} are unoverridden
* here and just wrap {@link #defaultProfile()}); it does no quarantine/cool-off/maxLoad/model-off
* filtering of its own to re-apply. That filtering lives one layer up, in {@code
* CompositePeerLauncher}, which is the launcher that routes across more than one profile and
* therefore overrides this method with the routing form instead.
*/
@Override
public PeerHandle spawn(SpawnRequest req, PlacementDecision decision) {
return spawn(req.withProfile(decision.profile()));
}
/** The herdr daemon that owns this launcher's pane coordinates. */
public HerdrClient herdr() {
return agents.herdr();
@@ -253,26 +253,27 @@ public interface PeerLauncher {
* matching profile; passing a request that names a <em>different</em>, explicit profile than
* the decision it is paired with is a caller bug this method does not attempt to detect.
*
* <p>Default implementation for a launcher with no placement concept of its own: delegates to
* {@link #spawn(SpawnRequest)} with the decision's profile named explicitly — its only spawn
* contract, since there is no separate routing path to honor. This default is correct ONLY for
* a launcher that spawns a single profile of its own (e.g. {@code HerdrPeerLauncher}), where
* the explicit-profile branch it re-enters and the routing branch {@link #place} would have
* used are the same thing. <strong>A launcher that routes across more than one profile — the
* way {@code CompositePeerLauncher} routes across every configured adapter — MUST override
* this method instead of inheriting this default.</strong> Re-entering {@link
* #spawn(SpawnRequest)} re-applies that single-argument method's explicit-profile checks
* ({@code enforceNotQuarantined}, {@code enforceNotCoolingOff}, {@code enforceMaxLoad}, {@code
* enforceModelEnabled} in {@code CompositePeerLauncher}), which can refuse the very profile
* {@link #place} just chose, if the underlying placement state moved in the window between the
* {@link #place} call and this one — the exact window this method and {@link PlacementDecision}
* exist to close (fleetd #444).
* <p>No default implementation (fleetd #450): the two correct bodies disagree on purpose, so an
* implementer must choose one rather than silently inherit whichever this interface happened to
* provide. An implementer with no placement concept of its own — spawns a single profile, e.g.
* {@code HerdrPeerLauncher} — should delegate to {@link #spawn(SpawnRequest)} with the decision's
* profile named explicitly, since there is no separate routing path to honor there: the
* explicit-profile branch it re-enters and the routing branch {@link #place} would have used are
* the same thing. <strong>A launcher that routes across more than one profile — the way {@code
* CompositePeerLauncher} routes across every configured adapter — MUST NOT re-enter {@link
* #spawn(SpawnRequest)}.</strong> Doing so re-applies that single-argument method's
* explicit-profile checks ({@code enforceNotQuarantined}, {@code enforceNotCoolingOff}, {@code
* enforceMaxLoad}, {@code enforceModelEnabled} in {@code CompositePeerLauncher}), which can
* refuse the very profile {@link #place} just chose, if the underlying placement state moved in
* the window between the {@link #place} call and this one — the exact window this method and
* {@link PlacementDecision} exist to close (fleetd #444). Before #450 this was a {@code default}
* method that only {@code CompositePeerLauncher} overrode; a future placement-doing launcher
* could have inherited the re-entering body silently and never known. Making it abstract turns
* that silent inheritance into a compile error.
*
* @throws IllegalArgumentException if the decision names an unknown profile
*/
default PeerHandle spawn(SpawnRequest req, PlacementDecision decision) {
return spawn(req.withProfile(decision.profile()));
}
PeerHandle spawn(SpawnRequest req, PlacementDecision decision);
/**
* Resolve the effective working directory for a spawn {@code req} without actually spawning.
@@ -20,6 +20,7 @@ import dev.ltms.fleet.peer.PeerLauncher;
import dev.ltms.fleet.peer.SpawnRequest;
import dev.ltms.fleet.placement.BackendOutagePolicy;
import dev.ltms.fleet.placement.BackendQuarantine;
import dev.ltms.fleet.placement.PlacementDecision;
import dev.ltms.fleet.placement.PlacementPolicies;
import dev.ltms.fleet.session.MemberSession;
import dev.ltms.fleet.session.SessionManager;
@@ -123,6 +124,11 @@ class FleetdBackendErrorSinkTest {
throw new UnsupportedOperationException("not reachable — this test never acquires a session");
}
@Override
public PeerHandle spawn(SpawnRequest req, PlacementDecision decision) {
throw new UnsupportedOperationException("not reachable — this test never acquires a session");
}
@Override
public Set<String> profiles() {
return Set.of();
@@ -23,6 +23,7 @@ import dev.ltms.fleet.peer.PeerLauncher;
import dev.ltms.fleet.peer.PeerUnreachableException;
import dev.ltms.fleet.peer.SpawnRequest;
import dev.ltms.fleet.placement.BackendQuarantine;
import dev.ltms.fleet.placement.PlacementDecision;
import dev.ltms.fleet.placement.PlacementPolicies;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.io.TempDir;
@@ -1036,6 +1037,11 @@ class SessionManagerTest {
return delegate.spawn(req);
}
@Override
public PeerHandle spawn(SpawnRequest req, PlacementDecision decision) {
return delegate.spawn(req, decision);
}
@Override
public Set<String> profiles() {
return delegate.profiles();
@@ -1597,6 +1603,11 @@ class SessionManagerTest {
throw new UnsupportedOperationException("not reachable — the capability check refuses first");
}
@Override
public PeerHandle spawn(SpawnRequest req, PlacementDecision decision) {
throw new UnsupportedOperationException("not reachable — the capability check refuses first");
}
@Override
public Set<String> profiles() {
return Set.of("stub-profile");
@@ -1661,6 +1672,11 @@ class SessionManagerTest {
return delegate.spawn(req);
}
@Override
public PeerHandle spawn(SpawnRequest req, PlacementDecision decision) {
return delegate.spawn(req, decision);
}
@Override
public Set<String> profiles() {
return delegate.profiles();
@@ -1867,6 +1883,11 @@ class SessionManagerTest {
return handle;
}
@Override
public PeerHandle spawn(SpawnRequest req, PlacementDecision decision) {
return spawn(req.withProfile(decision.profile()));
}
@Override
public Set<String> profiles() {
return Set.of("lazy");