fleet/fleetd
3
0
Fork 0
Code Issues 73 Pull Requests Actions Packages Projects Releases 1 Wiki Activity

fleetd #450: make PeerLauncher.spawn(SpawnRequest, PlacementDecision) abstract #451

Merged
ltms merged 1 commits from worker/450-abstract-spawn-599e1c-5 into main 2026-09-10 12:16:47 +02:00
Conversation 0 Commits 1 Files Changed 4
+63 -17
agent commented 2026-09-10 12:11:14 +02:00
Member
Copy Link
Copy Source

fleetd #450: make PeerLauncher.spawn(SpawnRequest, PlacementDecision) abstract.

Why

The default method re-entered the single-argument spawn(SpawnRequest), which
re-runs checks (enforceNotQuarantined, enforceNotCoolingOff, enforceMaxLoad,
enforceModelEnabled in CompositePeerLauncher) that can refuse the exact profile
place() just chose — the window PlacementDecision exists to close (#444). Only
CompositePeerLauncher overrode it; nothing forced a future placement-doing
launcher to override it too, so it could inherit the wrong body silently.

What changed

  • PeerLauncher.spawn(SpawnRequest, PlacementDecision) is now abstract (javadoc
    rewritten to describe the two legal bodies instead of "the default").
  • HerdrPeerLauncher (the base class of ClaudeCodeLauncher and OpenCodeLauncher
    — re-measured in this tree at 822327e, neither of those two classes implements
    PeerLauncher directly    , they extend this abstract class, and neither overrides
    spawn(SpawnRequest) or place() with placement filtering of its own) gets the
    re-entering form, with a comment saying why it's correct there.
  • CompositePeerLauncher's existing routing-form override is untouched.
  • 5 test-fake PeerLauncher implementers needed overrides to keep compiling
    (FleetdBackendErrorSinkTest.NeverSpawnsLauncher;
    SessionManagerTest.RaceLauncher, .NoResumeLauncher,
    .ClearContextSpyLauncher, .LazyIdLauncher) — each mirrors its own existing
    spawn(SpawnRequest) shape: a delegating wrapper delegates, an "unreachable"
    stub throws the same UnsupportedOperationException, the single-profile fake
    re-enters.

Deviation from the brief: the ticket/brief listed ConfigRef, ClaudeCodeLauncher,
CompositePeerLauncher, OpenCodeLauncher, HerdrPeerLauncher as the five
src/main implementers to give explicit overrides. Re-measured in this worktree at
822327e: ConfigRef (package config, implements Supplier<FleetConfig>) does
not implement PeerLauncher at all — only mentioned in comments — so it needed
no change. ClaudeCodeLauncher/OpenCodeLauncher do not implement PeerLauncher
directly; they extends HerdrPeerLauncher, which is abstract (only because of
its own buildLaunch adapter hook, unrelated to placement) and already owns the
entire spawn/placement contract. So the override belongs once, in HerdrPeerLauncher,
where both subclasses inherit it — duplicating it into the two leaf classes would
copy the exact same body for no reason. This was a decision within the brief's
delegated authority ("decide from what the class actually does, not from what the
others got"; "ConfigRef... check what kind of implementer it even is before
assuming").

Evidence

Full mvn -B clean install (green, before deleting anything):

[INFO] Tests run: 1575, Failures: 0, Errors: 0, Skipped: 0
[INFO] BUILD SUCCESS

Deleted HerdrPeerLauncher's new override, rebuilt — fails, naming both classes
that would otherwise have silently inherited nothing:

[ERROR] COMPILATION ERROR :
[ERROR] .../ClaudeCodeLauncher.java:[49,14] dev.ltms.fleet.member.ClaudeCodeLauncher is not abstract and does not override abstract method spawn(dev.ltms.fleet.peer.SpawnRequest,dev.ltms.fleet.placement.PlacementDecision) in dev.ltms.fleet.peer.PeerLauncher
[ERROR] .../OpenCodeLauncher.java:[59,14] dev.ltms.fleet.member.OpenCodeLauncher is not abstract and does not override abstract method spawn(dev.ltms.fleet.peer.SpawnRequest,dev.ltms.fleet.placement.PlacementDecision) in dev.ltms.fleet.peer.PeerLauncher

(exit=1, BUILD FAILURE)

Restored the override, rebuilt — green again:

[INFO] Tests run: 1575, Failures: 0, Errors: 0, Skipped: 0
[INFO] BUILD SUCCESS

Scope

Only #450's scope. Did not touch enforce* checks, the routing branch's
fall-through behavior, the #447 test, or revisit #425/#435/#444.

Out-of-scope note (not investigated further): PeerLauncher has two more
default methods with the same "only some implementers may safely inherit"
shape — defaultProfileFor(MemberRole) and place(MemberRole) (both wrap
defaultProfile() and are documented as correct only for a no-placement
launcher). Flagging per the brief's scope rule; not fixed here.

fleetd #450: make `PeerLauncher.spawn(SpawnRequest, PlacementDecision)` abstract. ## Why The `default` method re-entered the single-argument `spawn(SpawnRequest)`, which re-runs checks (`enforceNotQuarantined`, `enforceNotCoolingOff`, `enforceMaxLoad`, `enforceModelEnabled` in `CompositePeerLauncher`) that can refuse the exact profile `place()` just chose — the window `PlacementDecision` exists to close (#444). Only `CompositePeerLauncher` overrode it; nothing forced a future placement-doing launcher to override it too, so it could inherit the wrong body silently. ## What changed - `PeerLauncher.spawn(SpawnRequest, PlacementDecision)` is now abstract (javadoc rewritten to describe the two legal bodies instead of "the default"). - `HerdrPeerLauncher` (the base class of `ClaudeCodeLauncher` and `OpenCodeLauncher` — re-measured in this tree at 822327e, **neither of those two classes implements `PeerLauncher` directly**, they extend this abstract class, and neither overrides `spawn(SpawnRequest)` or `place()` with placement filtering of its own) gets the re-entering form, with a comment saying why it's correct there. - `CompositePeerLauncher`'s existing routing-form override is untouched. - 5 test-fake `PeerLauncher` implementers needed overrides to keep compiling (`FleetdBackendErrorSinkTest.NeverSpawnsLauncher`; `SessionManagerTest.RaceLauncher`, `.NoResumeLauncher`, `.ClearContextSpyLauncher`, `.LazyIdLauncher`) — each mirrors its own existing `spawn(SpawnRequest)` shape: a delegating wrapper delegates, an "unreachable" stub throws the same `UnsupportedOperationException`, the single-profile fake re-enters. **Deviation from the brief:** the ticket/brief listed `ConfigRef`, `ClaudeCodeLauncher`, `CompositePeerLauncher`, `OpenCodeLauncher`, `HerdrPeerLauncher` as the five `src/main` implementers to give explicit overrides. Re-measured in this worktree at 822327e: `ConfigRef` (package `config`, implements `Supplier<FleetConfig>`) does **not** implement `PeerLauncher` at all — only mentioned in comments — so it needed no change. `ClaudeCodeLauncher`/`OpenCodeLauncher` do not implement `PeerLauncher` directly; they `extends HerdrPeerLauncher`, which is `abstract` (only because of its own `buildLaunch` adapter hook, unrelated to placement) and already owns the entire spawn/placement contract. So the override belongs once, in `HerdrPeerLauncher`, where both subclasses inherit it — duplicating it into the two leaf classes would copy the exact same body for no reason. This was a decision within the brief's delegated authority ("decide from what the class actually does, not from what the others got"; "ConfigRef... check what kind of implementer it even is before assuming"). ## Evidence Full `mvn -B clean install` (green, before deleting anything): ``` [INFO] Tests run: 1575, Failures: 0, Errors: 0, Skipped: 0 [INFO] BUILD SUCCESS ``` Deleted `HerdrPeerLauncher`'s new override, rebuilt — fails, naming both classes that would otherwise have silently inherited nothing: ``` [ERROR] COMPILATION ERROR : [ERROR] .../ClaudeCodeLauncher.java:[49,14] dev.ltms.fleet.member.ClaudeCodeLauncher is not abstract and does not override abstract method spawn(dev.ltms.fleet.peer.SpawnRequest,dev.ltms.fleet.placement.PlacementDecision) in dev.ltms.fleet.peer.PeerLauncher [ERROR] .../OpenCodeLauncher.java:[59,14] dev.ltms.fleet.member.OpenCodeLauncher is not abstract and does not override abstract method spawn(dev.ltms.fleet.peer.SpawnRequest,dev.ltms.fleet.placement.PlacementDecision) in dev.ltms.fleet.peer.PeerLauncher ``` (exit=1, `BUILD FAILURE`) Restored the override, rebuilt — green again: ``` [INFO] Tests run: 1575, Failures: 0, Errors: 0, Skipped: 0 [INFO] BUILD SUCCESS ``` ## Scope Only #450's scope. Did not touch `enforce*` checks, the routing branch's fall-through behavior, the #447 test, or revisit #425/#435/#444. **Out-of-scope note (not investigated further):** `PeerLauncher` has two more `default` methods with the same "only some implementers may safely inherit" shape — `defaultProfileFor(MemberRole)` and `place(MemberRole)` (both wrap `defaultProfile()` and are documented as correct only for a no-placement launcher). Flagging per the brief's scope rule; not fixed here.
agent added 1 commit 2026-09-10 12:11:14 +02:00
fleetd #450: make PeerLauncher.spawn(SpawnRequest, PlacementDecision) abstract
CI / contract (pull_request) Successful in 47s
Details
CI / build (pull_request) Successful in 2m5s
Details
cfebc575ea 
The default re-entered the single-argument spawn(SpawnRequest), which re-runs
checks that can refuse the profile place() just chose (#444's window). Only
CompositePeerLauncher overrode it; a future placement-doing launcher could
have inherited the wrong body silently.

Give every current implementer an explicit override, chosen by what it does:
- HerdrPeerLauncher (base of ClaudeCodeLauncher/OpenCodeLauncher, neither of
  which overrides spawn(req) or place()) does no placement filtering of its
  own, so it gets the re-entering form.
- CompositePeerLauncher's routing-form override is untouched.
- 5 test-fake PeerLauncher implementers (SessionManagerTest, FleetdBackendErrorSinkTest)
  get overrides matching their existing spawn(SpawnRequest) shape: delegating
  wrappers delegate, unreachable stubs throw, the single-profile fake re-enters.

ConfigRef does not implement PeerLauncher at all (confirmed in this tree at
822327e) despite the ticket listing it as a src/main implementer.
ltms merged commit bdcf285265 into main 2026-09-10 12:16:47 +02:00
Sign in to join this conversation.
Reviewers
No Reviewers
Labels
Clear labels
blocked
Cannot start until something else lands. The body says what.
 needs-live-proof
Merged and green, but never shown working on the running daemon. Not the same as done.
 ready-to-delegate
Scope, files and acceptance criteria are written. A worker can be briefed from the body alone.
 silent-default
A feature that compiles, passes tests, and ships turned off. Nine recurrences and counting.
No Label
Milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
agent kevin ltms
No Assignees
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: fleet/fleetd#451
Delete Branch "worker/450-abstract-spawn-599e1c-5"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?