Verified by the lead on headcfebc57(base822327eis 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:
@@ -17,6 +17,7 @@ import dev.ltms.fleet.peer.PeerHandle;
|
|||||||
import dev.ltms.fleet.peer.PeerLauncher;
|
import dev.ltms.fleet.peer.PeerLauncher;
|
||||||
import dev.ltms.fleet.peer.PeerUnreachableException;
|
import dev.ltms.fleet.peer.PeerUnreachableException;
|
||||||
import dev.ltms.fleet.peer.SpawnRequest;
|
import dev.ltms.fleet.peer.SpawnRequest;
|
||||||
|
import dev.ltms.fleet.placement.PlacementDecision;
|
||||||
import org.slf4j.Logger;
|
import org.slf4j.Logger;
|
||||||
import org.slf4j.LoggerFactory;
|
import org.slf4j.LoggerFactory;
|
||||||
|
|
||||||
@@ -594,6 +595,23 @@ public abstract class HerdrPeerLauncher implements PeerLauncher {
|
|||||||
req.sessionName(), spawned.agentSessionId(), spawned.receipt());
|
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. */
|
/** The herdr daemon that owns this launcher's pane coordinates. */
|
||||||
public HerdrClient herdr() {
|
public HerdrClient herdr() {
|
||||||
return agents.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
|
* 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.
|
* 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
|
* <p>No default implementation (fleetd #450): the two correct bodies disagree on purpose, so an
|
||||||
* {@link #spawn(SpawnRequest)} with the decision's profile named explicitly — its only spawn
|
* implementer must choose one rather than silently inherit whichever this interface happened to
|
||||||
* contract, since there is no separate routing path to honor. This default is correct ONLY for
|
* provide. An implementer with no placement concept of its own — spawns a single profile, e.g.
|
||||||
* a launcher that spawns a single profile of its own (e.g. {@code HerdrPeerLauncher}), where
|
* {@code HerdrPeerLauncher} — should delegate to {@link #spawn(SpawnRequest)} with the decision's
|
||||||
* the explicit-profile branch it re-enters and the routing branch {@link #place} would have
|
* profile named explicitly, since there is no separate routing path to honor there: the
|
||||||
* used are the same thing. <strong>A launcher that routes across more than one profile — the
|
* explicit-profile branch it re-enters and the routing branch {@link #place} would have used are
|
||||||
* way {@code CompositePeerLauncher} routes across every configured adapter — MUST override
|
* the same thing. <strong>A launcher that routes across more than one profile — the way {@code
|
||||||
* this method instead of inheriting this default.</strong> Re-entering {@link
|
* CompositePeerLauncher} routes across every configured adapter — MUST NOT re-enter {@link
|
||||||
* #spawn(SpawnRequest)} re-applies that single-argument method's explicit-profile checks
|
* #spawn(SpawnRequest)}.</strong> Doing so re-applies that single-argument method's
|
||||||
* ({@code enforceNotQuarantined}, {@code enforceNotCoolingOff}, {@code enforceMaxLoad}, {@code
|
* explicit-profile checks ({@code enforceNotQuarantined}, {@code enforceNotCoolingOff}, {@code
|
||||||
* enforceModelEnabled} in {@code CompositePeerLauncher}), which can refuse the very profile
|
* enforceMaxLoad}, {@code enforceModelEnabled} in {@code CompositePeerLauncher}), which can
|
||||||
* {@link #place} just chose, if the underlying placement state moved in the window between the
|
* refuse the very profile {@link #place} just chose, if the underlying placement state moved in
|
||||||
* {@link #place} call and this one — the exact window this method and {@link PlacementDecision}
|
* the window between the {@link #place} call and this one — the exact window this method and
|
||||||
* exist to close (fleetd #444).
|
* {@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
|
* @throws IllegalArgumentException if the decision names an unknown profile
|
||||||
*/
|
*/
|
||||||
default PeerHandle spawn(SpawnRequest req, PlacementDecision decision) {
|
PeerHandle spawn(SpawnRequest req, PlacementDecision decision);
|
||||||
return spawn(req.withProfile(decision.profile()));
|
|
||||||
}
|
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Resolve the effective working directory for a spawn {@code req} without actually spawning.
|
* 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.peer.SpawnRequest;
|
||||||
import dev.ltms.fleet.placement.BackendOutagePolicy;
|
import dev.ltms.fleet.placement.BackendOutagePolicy;
|
||||||
import dev.ltms.fleet.placement.BackendQuarantine;
|
import dev.ltms.fleet.placement.BackendQuarantine;
|
||||||
|
import dev.ltms.fleet.placement.PlacementDecision;
|
||||||
import dev.ltms.fleet.placement.PlacementPolicies;
|
import dev.ltms.fleet.placement.PlacementPolicies;
|
||||||
import dev.ltms.fleet.session.MemberSession;
|
import dev.ltms.fleet.session.MemberSession;
|
||||||
import dev.ltms.fleet.session.SessionManager;
|
import dev.ltms.fleet.session.SessionManager;
|
||||||
@@ -123,6 +124,11 @@ class FleetdBackendErrorSinkTest {
|
|||||||
throw new UnsupportedOperationException("not reachable — this test never acquires a session");
|
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
|
@Override
|
||||||
public Set<String> profiles() {
|
public Set<String> profiles() {
|
||||||
return Set.of();
|
return Set.of();
|
||||||
|
|||||||
@@ -23,6 +23,7 @@ import dev.ltms.fleet.peer.PeerLauncher;
|
|||||||
import dev.ltms.fleet.peer.PeerUnreachableException;
|
import dev.ltms.fleet.peer.PeerUnreachableException;
|
||||||
import dev.ltms.fleet.peer.SpawnRequest;
|
import dev.ltms.fleet.peer.SpawnRequest;
|
||||||
import dev.ltms.fleet.placement.BackendQuarantine;
|
import dev.ltms.fleet.placement.BackendQuarantine;
|
||||||
|
import dev.ltms.fleet.placement.PlacementDecision;
|
||||||
import dev.ltms.fleet.placement.PlacementPolicies;
|
import dev.ltms.fleet.placement.PlacementPolicies;
|
||||||
import org.junit.jupiter.api.Test;
|
import org.junit.jupiter.api.Test;
|
||||||
import org.junit.jupiter.api.io.TempDir;
|
import org.junit.jupiter.api.io.TempDir;
|
||||||
@@ -1036,6 +1037,11 @@ class SessionManagerTest {
|
|||||||
return delegate.spawn(req);
|
return delegate.spawn(req);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@Override
|
||||||
|
public PeerHandle spawn(SpawnRequest req, PlacementDecision decision) {
|
||||||
|
return delegate.spawn(req, decision);
|
||||||
|
}
|
||||||
|
|
||||||
@Override
|
@Override
|
||||||
public Set<String> profiles() {
|
public Set<String> profiles() {
|
||||||
return delegate.profiles();
|
return delegate.profiles();
|
||||||
@@ -1597,6 +1603,11 @@ class SessionManagerTest {
|
|||||||
throw new UnsupportedOperationException("not reachable — the capability check refuses first");
|
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
|
@Override
|
||||||
public Set<String> profiles() {
|
public Set<String> profiles() {
|
||||||
return Set.of("stub-profile");
|
return Set.of("stub-profile");
|
||||||
@@ -1661,6 +1672,11 @@ class SessionManagerTest {
|
|||||||
return delegate.spawn(req);
|
return delegate.spawn(req);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@Override
|
||||||
|
public PeerHandle spawn(SpawnRequest req, PlacementDecision decision) {
|
||||||
|
return delegate.spawn(req, decision);
|
||||||
|
}
|
||||||
|
|
||||||
@Override
|
@Override
|
||||||
public Set<String> profiles() {
|
public Set<String> profiles() {
|
||||||
return delegate.profiles();
|
return delegate.profiles();
|
||||||
@@ -1867,6 +1883,11 @@ class SessionManagerTest {
|
|||||||
return handle;
|
return handle;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@Override
|
||||||
|
public PeerHandle spawn(SpawnRequest req, PlacementDecision decision) {
|
||||||
|
return spawn(req.withProfile(decision.profile()));
|
||||||
|
}
|
||||||
|
|
||||||
@Override
|
@Override
|
||||||
public Set<String> profiles() {
|
public Set<String> profiles() {
|
||||||
return Set.of("lazy");
|
return Set.of("lazy");
|
||||||
|
|||||||
Reference in New Issue
Block a user