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

fleetd #234: key opencode model check on resolved session id; make spawn-time quarantine actually happen #236

Closed
agent wants to merge 0 commits from worker/cb234-session-id-keyed-04e1fc-1 into main
pull from: worker/cb234-session-id-keyed-04e1fc-1
merge into: fleet:main
Conversation 4 Commits - Files Changed -
agent commented 2026-09-03 05:10:03 +02:00
Member
Copy Link
Copy Source

fleetd #234 — session-id-keyed model check + spawn-time quarantine that actually happens

Two coupled defects, one owner (per the ticket) to avoid a merge conflict.

Defect 1 — model read-back keyed on directory, not the resolved session id

OpenCodeSessionDiscovery.actualModelForDirectory queried WHERE directory = ?, the same heuristic sessionIdForDirectory uses. A default fleet_spawn (no worktree:) shares the lead's cwd with every other worker and every past session ever run there, so the model read-back could silently compare against a different session's row (this was measured live: a fresh terra member's check compared against a 3-day-old gx session in the same directory).

Fix: renamed to actualModelForSessionId(sessionId), keyed on the primary key id instead of directory. OpenCodeLauncher's SessionAwareHandle now caches the resolved id once it is first non-null (AtomicReference, compareAndSet(null, id)), so a later sibling row appearing in the same shared directory can never flip which session's evidence gets read. If the id has not resolved yet, that is UNKNOWN — never compared (fleetd #175's existing rule).

sessionIdForDirectory (fleetd #209) is left exactly as-is and still directory-based, per the ticket's explicit instruction — a comment now explains why the heuristic is unavoidable at that layer (nothing else disambiguates a session at this layer, and diffing before/after races under concurrent spawns).

Defect 2 — the quarantine the ERROR log announces never actually happened

The ERROR log said "quarantining this profile's credential" but Fleetd.java's ExhaustionSink lambda resolved target -> session (via sessions.roster()) -> profile -> credential. OpenCodeLauncher's model-mismatch check fires from SessionAwareHandle.agentSessionId(), called during SessionManager.acquire() before the new session is registered in the roster — the lookup found nothing and .ifPresent silently no-opped.

Fix: added a default 3-arg ExhaustionSink.onExhausted(target, reason, profile) overload (defaults to the existing 2-arg method, so CompletionResolver's two call sites — which always call with a target already live in the roster — are unchanged). OpenCodeLauncher already holds its own FleetConfig.Profile and now passes its profile name directly, bypassing the need for roster resolution at this call site entirely. Fleetd.java's sink (now an anonymous class, not a lambda, so it can actually override the 3-arg method) tries the roster first, falls back to the hint, and — if neither resolves — logs loudly at ERROR naming target/reason/profile-hint instead of silently no-opping.

Mutation-tested, not just asserted

Both fixes were proven load-bearing by reverting each independently and confirming its new test fails with a real message, then restoring and confirming it passes again:

Defect 1 — reverted OpenCodeLauncher.java + OpenCodeSessionDiscovery.java to HEAD, ran OpenCodeLauncherTest#modelCheckReadsTheResolvedSessionsOwnRowNotWhateverIsNewestInTheSharedDirectory:

org.opentest4j.AssertionFailedError: the session id, once resolved, must not flip to a sibling sharing the directory ==> expected: <ses_ours> but was: <ses_sibling>
[ERROR] Tests run: 1, Failures: 1, Errors: 0, Skipped: 0

Restored, re-ran: green.

Defect 2 — mutated the one call site in OpenCodeLauncher.java back to the 2-arg onExhausted(target, reason) call (the pre-fix shape), ran OpenCodeLauncherTest#aSpawnTimeModelMismatchActuallyQuarantinesTheCredentialThroughTheRealAcquirePath:

org.opentest4j.AssertionFailedError: the mismatch fires DURING acquire(), before roster registration, and must still reach the quarantine via the profile hint — not silently no-op ==> expected: <true> but was: <false>
[ERROR] Tests run: 1, Failures: 1, Errors: 0, Skipped: 0

Restored, re-ran: green. (A second new test, aRosterOnlySinkSilentlyDropsTheSpawnTimeQuarantine, pins the pre-fix roster-only-sink behavior directly, using a plain 2-arg lambda that cannot see the hint.)

Both new tests go through the real production call path (SessionManager.acquire() -> handle.agentSessionId() -> the sink), not a direct call into the sink or the discovery class.

Build

mvn clean install (unpiped, from fleetd/):

Tests run: 1127, Failures: 0, Errors: 0, Skipped: 0
BUILD SUCCESS

Files changed

  • src/main/java/dev/ltms/fleet/member/OpenCodeSessionDiscovery.java
  • src/main/java/dev/ltms/fleet/member/OpenCodeLauncher.java
  • src/main/java/dev/ltms/fleet/inject/ExhaustionSink.java
  • src/main/java/dev/ltms/fleet/Fleetd.java (confined to the ExhaustionSink lambda ~line 348 — #115's startup-warning work in the rest of that file is untouched)
  • src/test/java/dev/ltms/fleet/member/OpenCodeSessionDiscoveryTest.java
  • src/test/java/dev/ltms/fleet/member/OpenCodeLauncherTest.java

Caveats for review

  • sessionIdForDirectory (#209) intentionally left directory-based, per the ticket — only a comment was added explaining why.
  • The HARD RULE was honored: no code or test ever touches the real ~/.local/share/opencode/opencode.db — all discovery tests build a temporary SQLite fixture via @TempDir.
  • Nothing surfaced outside the assigned scope during this work.
## fleetd #234 — session-id-keyed model check + spawn-time quarantine that actually happens Two coupled defects, one owner (per the ticket) to avoid a merge conflict. ### Defect 1 — model read-back keyed on directory, not the resolved session id `OpenCodeSessionDiscovery.actualModelForDirectory` queried `WHERE directory = ?`, the same heuristic `sessionIdForDirectory` uses. A default `fleet_spawn` (no `worktree:`) shares the lead's cwd with every other worker and every past session ever run there, so the model read-back could silently compare against a *different* session's row (this was measured live: a fresh `terra` member's check compared against a 3-day-old `gx` session in the same directory). **Fix:** renamed to `actualModelForSessionId(sessionId)`, keyed on the primary key `id` instead of `directory`. `OpenCodeLauncher`'s `SessionAwareHandle` now caches the resolved id once it is first non-null (`AtomicReference`, `compareAndSet(null, id)`), so a later sibling row appearing in the same shared directory can never flip which session's evidence gets read. If the id has not resolved yet, that is UNKNOWN — never compared (fleetd #175's existing rule). `sessionIdForDirectory` (fleetd #209) is left exactly as-is and still directory-based, per the ticket's explicit instruction — a comment now explains why the heuristic is unavoidable at that layer (nothing else disambiguates a session at this layer, and diffing before/after races under concurrent spawns). ### Defect 2 — the quarantine the ERROR log announces never actually happened The ERROR log said "quarantining this profile's credential" but `Fleetd.java`'s `ExhaustionSink` lambda resolved `target -> session (via sessions.roster()) -> profile -> credential`. `OpenCodeLauncher`'s model-mismatch check fires from `SessionAwareHandle.agentSessionId()`, called during `SessionManager.acquire()` **before** the new session is registered in the roster — the lookup found nothing and `.ifPresent` silently no-opped. **Fix:** added a default 3-arg `ExhaustionSink.onExhausted(target, reason, profile)` overload (defaults to the existing 2-arg method, so `CompletionResolver`'s two call sites — which always call with a target already live in the roster — are unchanged). `OpenCodeLauncher` already holds its own `FleetConfig.Profile` and now passes its profile name directly, bypassing the need for roster resolution at this call site entirely. `Fleetd.java`'s sink (now an anonymous class, not a lambda, so it can actually override the 3-arg method) tries the roster first, falls back to the hint, and — if *neither* resolves — logs loudly at ERROR naming target/reason/profile-hint instead of silently no-opping. ### Mutation-tested, not just asserted Both fixes were proven load-bearing by reverting each independently and confirming its new test fails with a real message, then restoring and confirming it passes again: **Defect 1** — reverted `OpenCodeLauncher.java` + `OpenCodeSessionDiscovery.java` to HEAD, ran `OpenCodeLauncherTest#modelCheckReadsTheResolvedSessionsOwnRowNotWhateverIsNewestInTheSharedDirectory`: ``` org.opentest4j.AssertionFailedError: the session id, once resolved, must not flip to a sibling sharing the directory ==> expected: <ses_ours> but was: <ses_sibling> [ERROR] Tests run: 1, Failures: 1, Errors: 0, Skipped: 0 ``` Restored, re-ran: green. **Defect 2** — mutated the one call site in `OpenCodeLauncher.java` back to the 2-arg `onExhausted(target, reason)` call (the pre-fix shape), ran `OpenCodeLauncherTest#aSpawnTimeModelMismatchActuallyQuarantinesTheCredentialThroughTheRealAcquirePath`: ``` org.opentest4j.AssertionFailedError: the mismatch fires DURING acquire(), before roster registration, and must still reach the quarantine via the profile hint — not silently no-op ==> expected: <true> but was: <false> [ERROR] Tests run: 1, Failures: 1, Errors: 0, Skipped: 0 ``` Restored, re-ran: green. (A second new test, `aRosterOnlySinkSilentlyDropsTheSpawnTimeQuarantine`, pins the pre-fix roster-only-sink behavior directly, using a plain 2-arg lambda that cannot see the hint.) Both new tests go through the **real production call path** (`SessionManager.acquire()` -> `handle.agentSessionId()` -> the sink), not a direct call into the sink or the discovery class. ### Build `mvn clean install` (unpiped, from `fleetd/`): ``` Tests run: 1127, Failures: 0, Errors: 0, Skipped: 0 BUILD SUCCESS ``` ### Files changed - `src/main/java/dev/ltms/fleet/member/OpenCodeSessionDiscovery.java` - `src/main/java/dev/ltms/fleet/member/OpenCodeLauncher.java` - `src/main/java/dev/ltms/fleet/inject/ExhaustionSink.java` - `src/main/java/dev/ltms/fleet/Fleetd.java` (confined to the `ExhaustionSink` lambda ~line 348 — #115's startup-warning work in the rest of that file is untouched) - `src/test/java/dev/ltms/fleet/member/OpenCodeSessionDiscoveryTest.java` - `src/test/java/dev/ltms/fleet/member/OpenCodeLauncherTest.java` ### Caveats for review - `sessionIdForDirectory` (#209) intentionally left directory-based, per the ticket — only a comment was added explaining why. - The HARD RULE was honored: no code or test ever touches the real `~/.local/share/opencode/opencode.db` — all discovery tests build a temporary SQLite fixture via `@TempDir`. - Nothing surfaced outside the assigned scope during this work.
agent added 1 commit 2026-09-03 05:10:03 +02:00
fleetd #234: key the opencode model check on the resolved session id, and make the spawn-time quarantine actually happen
CI / contract (pull_request) Successful in 58s
Details
CI / build (pull_request) Successful in 1m29s
Details
4877992a70 
Defect 1: OpenCodeSessionDiscovery.actualModelForDirectory queried
WHERE directory = ?, the same heuristic sessionIdForDirectory uses. Since a
default fleet_spawn (no worktree:) shares the lead's cwd with every other
worker and every past session ever run there, the model read-back could
silently compare against a DIFFERENT session's row. Renamed to
actualModelForSessionId(sessionId), keyed on the primary key id instead, and
made OpenCodeLauncher's SessionAwareHandle cache the resolved id once
non-null (AtomicReference) so a later sibling row in the same directory can
never flip which session's evidence is read. sessionIdForDirectory (#209) is
left directory-based on purpose, with a comment explaining why the heuristic
is unavoidable at that layer.

Defect 2: the ERROR log claimed "quarantining this profile's credential" but
Fleetd's ExhaustionSink lambda resolved target -> roster -> profile ->
credential, while OpenCodeLauncher's model-mismatch check fires from
agentSessionId() during SessionManager.acquire(), before the session is
registered in the roster -- the lookup found nothing and silently no-opped.
Added a default 3-arg ExhaustionSink.onExhausted(target, reason, profile)
overload (defaults to the 2-arg method, so CompletionResolver's two call
sites are unchanged); OpenCodeLauncher now passes its own already-known
profile name; Fleetd's sink became an anonymous class that tries the roster
first, falls back to the hint, and logs loudly at ERROR naming target/reason
when neither resolves, instead of silently no-oping.

Both fixes proven by mutation: reverting each independently makes its new
test fail with a real assertion message, restoring makes it pass again.

mvn clean install: Tests run: 1127, Failures: 0, Errors: 0, Skipped: 0, BUILD SUCCESS.
agent added 1 commit 2026-09-03 05:22:37 +02:00
fleetd #234 round 2: fix the ExhaustionSink forwarding hop Fleetd.java actually uses
CI / contract (pull_request) Successful in 1m10s
Details
CI / build (pull_request) Successful in 1m18s
Details
c325054242 
The round-1 fix was dead on the real production path. Fleetd.java:177 builds
a forwarding sink (needed because the adapters are constructed before
`sessions` exists, breaking a genuine cycle) as a LAMBDA:

    ExhaustionSink forwardingExhaustionSink =
            (target, reason) -> exhaustionSinkRef.get().onExhausted(target, reason);

A lambda can only implement the interface's one abstract method (the 2-arg
overload), so it silently inherited the 3-arg overload's default body, which
drops the profile hint and calls back into the 2-arg method. OpenCodeLauncher
is constructed with this forwarder, so the hint it supplies (its own
already-known profile name) was thrown away before it ever reached the real
sink built later in Fleetd.main -- reproducing the exact silent no-op round 1
was sent to fix. The 1127 tests from round 1 all injected a sink directly
into OpenCodeLauncher and never went through this forwarding hop, so none of
them could see it.

Fix: forwardingExhaustionSink is now an anonymous class overriding both
overloads, each delegating to whatever exhaustionSinkRef currently holds.

Audited every other ExhaustionSink value in main/: the only other one is
ExhaustionSink.none() (a lambda), which is safe regardless of arity since
both its 2-arg body and the inherited 3-arg default are true no-ops.

New tests:
- ExhaustionSinkForwardingHazardTest: isolates the hazard at the interface
  level (a lambda forwarder drops the hint; an anonymous-class forwarder
  does not), independent of Fleetd.java's specific wiring.
- OpenCodeLauncherTest#theSpawnTimeQuarantineSurvivesTheFleetdStyleForwardingHop:
  replicates Fleetd.java's actual construction order (forwarder built and
  handed to the launcher first, real sink built and pointed at via the
  AtomicReference afterward) and drives the quarantine through it via the
  real SessionManager.acquire() path.

Both proven by mutation: temporarily rewriting each fixed forwarder back
into the pre-fix lambda makes its test fail with a real assertion message
(both matched exactly: "expected: <gx> but was: <null>" for the interface
proof, "expected: <true> but was: <false>" for the composed-wiring test);
restoring makes it pass again. No reverts were committed.

mvn clean install: Tests run: 1130, Failures: 0, Errors: 0, Skipped: 0, BUILD SUCCESS.
agent commented 2026-09-03 05:22:59 +02:00
Author
Member
Copy Link
Copy Source

Round 2 — Defect 2's forwarding hop was also broken, now fixed

You were right: Fleetd.java:177's forwardingExhaustionSink was a lambda, so it could only implement the 2-arg onExhausted(target, reason) and silently inherited the 3-arg overload's default body — dropping OpenCodeLauncher's profile hint before it ever reached the real sink. Confirmed by reading the code at that line and by reproducing your proof test.

Fix

Fleetd.java:177 — forwardingExhaustionSink is now an anonymous class overriding both overloads, each delegating to exhaustionSinkRef.get():

ExhaustionSink forwardingExhaustionSink = new ExhaustionSink() {
    @Override
    public void onExhausted(String target, String reason) {
        exhaustionSinkRef.get().onExhausted(target, reason);
    }

    @Override
    public void onExhausted(String target, String reason, String profile) {
        exhaustionSinkRef.get().onExhausted(target, reason, profile);
    }
};

Item 3 — every other ExhaustionSink value in main/

Searched src/main/java for every construction of ExhaustionSink. Besides Fleetd.java's two sinks (the forwarder just fixed, and the real one built later — already an anonymous class overriding both overloads, unchanged), the only other value anywhere in main/ is ExhaustionSink.none():

static ExhaustionSink none() {
    return (target, reason) -> { };
}

This IS a lambda, but it is safe regardless of arity: its 2-arg body is empty, and the inherited 3-arg default just calls that same empty 2-arg body. There is no hint to drop because it does nothing either way. CompletionResolver and OpenCodeLauncher's no-sink constructors both use it as an inert default — no fix needed there. No other lambda or method-reference ExhaustionSink exists in main/.

Before/after — the exact proof test you gave me

Added ExhaustionSinkForwardingHazardTest#profileHintSurvivesTheForwardingHopUsedInProduction (your snippet, in src/test/java/dev/ltms/fleet/inject/). Mutated its forwarder back to a lambda (the pre-fix shape) and ran it:

org.opentest4j.AssertionFailedError: the profile hint must reach the real sink through the forwarding hop ==> expected: <gx> but was: <null>
Tests run: 1, Failures: 1, Errors: 0, Skipped: 0

Matches your result exactly. Restored the anonymous-class forwarder, re-ran: green.

A second test in the same file, aLambdaForwarderDropsTheProfileHintBeforeItReachesTheRealSink, documents the mechanism directly (asserts the real sink's 3-arg method DOES still run through its own default, but with the hint already null — not "never called").

Composed-wiring test — drives the hint through production-shaped wiring, not just the interface

Added OpenCodeLauncherTest#theSpawnTimeQuarantineSurvivesTheFleetdStyleForwardingHop. It replicates Fleetd.java's actual construction order: an AtomicReference<ExhaustionSink> seeded with ExhaustionSink.none(), OpenCodeLauncher constructed against a forwarder pointed at that reference (mirroring adapters being built before sessions exists), and only afterward builds the real hint-aware sink and points the reference at it (mirroring exhaustionSinkRef.set(exhaustionSink) at the end of Fleetd.main). It then drives the mismatch through the real SessionManager.acquire() path and asserts the credential actually gets quarantined.

Mutation proof: rewrote the test's forwarder back into a lambda, ran it:

org.opentest4j.AssertionFailedError: the profile hint must survive the Fleetd-style forwarding hop and reach the real sink — a lambda forwarder drops it and this must go red ==> expected: <true> but was: <false>
Tests run: 1, Failures: 1, Errors: 0, Skipped: 0

Restored the anonymous-class forwarder, re-ran: green. Confirmed via git status --short after restore that only the intended 3 files carry changes (no stray edits left from the mutation).

Build

mvn clean install, unpiped, run from inside fleetd/ (not mvn -pl fleetd from the repo root):

Tests run: 1130, Failures: 0, Errors: 0, Skipped: 0
BUILD SUCCESS

Files changed this round

  • src/main/java/dev/ltms/fleet/Fleetd.java — still confined to the ExhaustionSink wiring (the forwarder at ~line 177 this time, not the ~348 lambda from round 1)
  • src/test/java/dev/ltms/fleet/inject/ExhaustionSinkForwardingHazardTest.java (new)
  • src/test/java/dev/ltms/fleet/member/OpenCodeLauncherTest.java (new test added)

Pushed to worker/cb234-session-id-keyed-04e1fc-1, same PR (#236), no new PR opened.

## Round 2 — Defect 2's forwarding hop was also broken, now fixed You were right: `Fleetd.java:177`'s `forwardingExhaustionSink` was a lambda, so it could only implement the 2-arg `onExhausted(target, reason)` and silently inherited the 3-arg overload's default body — dropping `OpenCodeLauncher`'s profile hint before it ever reached the real sink. Confirmed by reading the code at that line and by reproducing your proof test. ### Fix `Fleetd.java:177` — `forwardingExhaustionSink` is now an anonymous class overriding **both** overloads, each delegating to `exhaustionSinkRef.get()`: ```java ExhaustionSink forwardingExhaustionSink = new ExhaustionSink() { @Override public void onExhausted(String target, String reason) { exhaustionSinkRef.get().onExhausted(target, reason); } @Override public void onExhausted(String target, String reason, String profile) { exhaustionSinkRef.get().onExhausted(target, reason, profile); } }; ``` ### Item 3 — every other `ExhaustionSink` value in main/ Searched `src/main/java` for every construction of `ExhaustionSink`. Besides `Fleetd.java`'s two sinks (the forwarder just fixed, and the real one built later — already an anonymous class overriding both overloads, unchanged), the only other value anywhere in main/ is `ExhaustionSink.none()`: ```java static ExhaustionSink none() { return (target, reason) -> { }; } ``` This IS a lambda, but it is safe regardless of arity: its 2-arg body is empty, and the inherited 3-arg default just calls that same empty 2-arg body. There is no hint to drop because it does nothing either way. `CompletionResolver` and `OpenCodeLauncher`'s no-sink constructors both use it as an inert default — no fix needed there. No other lambda or method-reference `ExhaustionSink` exists in main/. ### Before/after — the exact proof test you gave me Added `ExhaustionSinkForwardingHazardTest#profileHintSurvivesTheForwardingHopUsedInProduction` (your snippet, in `src/test/java/dev/ltms/fleet/inject/`). Mutated its forwarder back to a lambda (the pre-fix shape) and ran it: ``` org.opentest4j.AssertionFailedError: the profile hint must reach the real sink through the forwarding hop ==> expected: <gx> but was: <null> Tests run: 1, Failures: 1, Errors: 0, Skipped: 0 ``` Matches your result exactly. Restored the anonymous-class forwarder, re-ran: green. A second test in the same file, `aLambdaForwarderDropsTheProfileHintBeforeItReachesTheRealSink`, documents the mechanism directly (asserts the real sink's 3-arg method DOES still run through its own default, but with the hint already null — not "never called"). ### Composed-wiring test — drives the hint through production-shaped wiring, not just the interface Added `OpenCodeLauncherTest#theSpawnTimeQuarantineSurvivesTheFleetdStyleForwardingHop`. It replicates `Fleetd.java`'s actual construction order: an `AtomicReference<ExhaustionSink>` seeded with `ExhaustionSink.none()`, `OpenCodeLauncher` constructed against a forwarder pointed at that reference (mirroring `adapters` being built before `sessions` exists), and only afterward builds the real hint-aware sink and points the reference at it (mirroring `exhaustionSinkRef.set(exhaustionSink)` at the end of `Fleetd.main`). It then drives the mismatch through the real `SessionManager.acquire()` path and asserts the credential actually gets quarantined. **Mutation proof:** rewrote the test's forwarder back into a lambda, ran it: ``` org.opentest4j.AssertionFailedError: the profile hint must survive the Fleetd-style forwarding hop and reach the real sink — a lambda forwarder drops it and this must go red ==> expected: <true> but was: <false> Tests run: 1, Failures: 1, Errors: 0, Skipped: 0 ``` Restored the anonymous-class forwarder, re-ran: green. Confirmed via `git status --short` after restore that only the intended 3 files carry changes (no stray edits left from the mutation). ### Build `mvn clean install`, unpiped, run from inside `fleetd/` (not `mvn -pl fleetd` from the repo root): ``` Tests run: 1130, Failures: 0, Errors: 0, Skipped: 0 BUILD SUCCESS ``` ### Files changed this round - `src/main/java/dev/ltms/fleet/Fleetd.java` — still confined to the `ExhaustionSink` wiring (the forwarder at ~line 177 this time, not the ~348 lambda from round 1) - `src/test/java/dev/ltms/fleet/inject/ExhaustionSinkForwardingHazardTest.java` (new) - `src/test/java/dev/ltms/fleet/member/OpenCodeLauncherTest.java` (new test added) Pushed to `worker/cb234-session-id-keyed-04e1fc-1`, same PR (#236), no new PR opened.
agent added 1 commit 2026-09-03 05:30:45 +02:00
fleetd #234 round 3: make ExhaustionSink's forwarder a shared factory, not a rebuilt-per-caller shape
CI / contract (pull_request) Successful in 42s
Details
CI / build (pull_request) Successful in 1m36s
Details
c935b181dd 
Round 2's tests never reached Fleetd.java at all: both new tests declared
their OWN local copy of the forwarding shape instead of calling production's.
Mutating Fleetd.java's real forwarder back into the broken lambda left those
copies untouched, so the whole suite stayed green while production had
regressed to exactly the bug being fixed -- proven live by the reviewer.

Fix: extracted the forwarding shape into one named factory,
ExhaustionSink.forwardingTo(Supplier<ExhaustionSink> target), with the
"why a lambda here is wrong" explanation moved onto it (the one place the
shape is now written). Fleetd.java's forwarder collapses to one line:

    ExhaustionSink forwardingExhaustionSink = ExhaustionSink.forwardingTo(exhaustionSinkRef::get);

Both new tests now call this same factory instead of rebuilding an anonymous
class inline, so they exercise the identical object production builds:
- ExhaustionSinkForwardingHazardTest: calls ExhaustionSink.forwardingTo
  directly and asserts the hint reaches the real sink through it.
- OpenCodeLauncherTest#theSpawnTimeQuarantineSurvivesTheFleetdStyleForwardingHop:
  same factory call, inside the full Fleetd-shaped construction order
  (forwarder built first, real sink pointed at via the AtomicReference
  afterward), driven through the real SessionManager.acquire() path.

Mutation proof, this time on production code only: deleted the factory's
3-arg override (falls back to the interface default, dropping the hint) --
both new tests go red with no test file touched:

  ExhaustionSinkForwardingHazardTest...: expected: <gx> but was: <null>
  OpenCodeLauncherTest...ForwardingHop:  expected: <true> but was: <false>
  Tests run: 68, Failures: 2

Restored, re-ran: green (Tests run: 68, Failures: 0). Confirmed Fleetd.java
carries no lambda ExhaustionSink anywhere (grep). ExhaustionSink.none() stays
a lambda on purpose -- both its overloads are true no-ops regardless of
arity, so there is no hint to drop.

mvn clean install: Tests run: 1129, Failures: 0, Errors: 0, Skipped: 0, BUILD SUCCESS.
agent commented 2026-09-03 05:31:04 +02:00
Author
Member
Copy Link
Copy Source

Round 3 — extracted the forwarding shape into one shared factory, both tests now call it

You measured it correctly: round 2's two new tests each built their OWN local copy of the forwarding shape instead of calling anything Fleetd.java builds. Mutating Fleetd.java's real forwarder back into a lambda left those copies untouched, so the suite stayed green while production had regressed. Confirmed by reproducing your grep (every Fleetd reference in both test files was in a javadoc comment, none in executable code) and by re-running the same mutation you described.

Fix — ExhaustionSink.forwardingTo

Added to ExhaustionSink.java:

static ExhaustionSink forwardingTo(Supplier<ExhaustionSink> target) {
    return new ExhaustionSink() {
        @Override
        public void onExhausted(String t, String r) {
            target.get().onExhausted(t, r);
        }

        @Override
        public void onExhausted(String t, String r, String p) {
            target.get().onExhausted(t, r, p);
        }
    };
}

Fleetd.java's forwarder is now one line:

ExhaustionSink forwardingExhaustionSink = ExhaustionSink.forwardingTo(exhaustionSinkRef::get);

Moved the "why a lambda here is wrong" explanation onto the factory's javadoc, per your instruction, with a short pointer comment left at the Fleetd.java call site.

Both new tests rewritten to call the factory, not rebuild it

  • ExhaustionSinkForwardingHazardTest#forwardingToDeliversTheProfileHintToWhateverSinkTheSupplierCurrentlyReturns — calls ExhaustionSink.forwardingTo(ref::get) directly.
  • OpenCodeLauncherTest#theSpawnTimeQuarantineSurvivesTheFleetdStyleForwardingHop — same call inside the full construction-order replica, driven through the real SessionManager.acquire() path.

No hand-built forwarder remains in either test file. (I also deleted the round-2 documentation-only test that demonstrated the hazard with a deliberately-broken local lambda — it never called production code either way, and is now redundant with the factory-level test.)

Acceptance

1 & 2 — mutation on the factory body itself, no test file touched:

static ExhaustionSink forwardingTo(Supplier<ExhaustionSink> target) {
    // 3-arg override deleted, falls back to the interface default
    return new ExhaustionSink() {
        @Override
        public void onExhausted(String t, String r) {
            target.get().onExhausted(t, r);
        }
    };
}

Red:

ExhaustionSinkForwardingHazardTest.forwardingToDeliversTheProfileHintToWhateverSinkTheSupplierCurrentlyReturns:43
  ExhaustionSink.forwardingTo must deliver the profile hint to the sink the supplier currently returns
  ==> expected: <gx> but was: <null>
OpenCodeLauncherTest.theSpawnTimeQuarantineSurvivesTheFleetdStyleForwardingHop:1357
  the profile hint must survive the Fleetd-style forwarding hop and reach the real sink
  ==> expected: <true> but was: <false>
Tests run: 68, Failures: 2, Errors: 0, Skipped: 0
BUILD FAILURE

Restored, green:

Tests run: 68, Failures: 0, Errors: 0, Skipped: 0
BUILD SUCCESS

3 — grep -n "ExhaustionSink" src/main/java/dev/ltms/fleet/Fleetd.java: no lambda anywhere — the forwarder is now a forwardingTo(...) call, and the real sink (built later, once sessions exists) is unchanged from round 2, still an anonymous class overriding both overloads. ExhaustionSink.none() stays a lambda on purpose: both its overloads are true no-ops regardless of arity, so there is no hint to drop.

4 — full build, unpiped, from inside fleetd/:

Tests run: 1129, Failures: 0, Errors: 0, Skipped: 0
BUILD SUCCESS

(1129 vs round 2's 1130 — net -1 from deleting the now-redundant documentation-only test above.)

The check you asked me to satisfy

after your change, editing only ExhaustionSink.java or only Fleetd.java must be able to turn a test red

Verified directly: the mutation above only touched ExhaustionSink.java (the factory body), touched no test file, and both new tests went red with the exact assertions above.

Pushed to worker/cb234-session-id-keyed-04e1fc-1 (commit c935b18), same PR #236, no new PR.

## Round 3 — extracted the forwarding shape into one shared factory, both tests now call it You measured it correctly: round 2's two new tests each built their OWN local copy of the forwarding shape instead of calling anything `Fleetd.java` builds. Mutating `Fleetd.java`'s real forwarder back into a lambda left those copies untouched, so the suite stayed green while production had regressed. Confirmed by reproducing your grep (every `Fleetd` reference in both test files was in a javadoc comment, none in executable code) and by re-running the same mutation you described. ### Fix — `ExhaustionSink.forwardingTo` Added to `ExhaustionSink.java`: ```java static ExhaustionSink forwardingTo(Supplier<ExhaustionSink> target) { return new ExhaustionSink() { @Override public void onExhausted(String t, String r) { target.get().onExhausted(t, r); } @Override public void onExhausted(String t, String r, String p) { target.get().onExhausted(t, r, p); } }; } ``` `Fleetd.java`'s forwarder is now one line: ```java ExhaustionSink forwardingExhaustionSink = ExhaustionSink.forwardingTo(exhaustionSinkRef::get); ``` Moved the "why a lambda here is wrong" explanation onto the factory's javadoc, per your instruction, with a short pointer comment left at the `Fleetd.java` call site. ### Both new tests rewritten to call the factory, not rebuild it - `ExhaustionSinkForwardingHazardTest#forwardingToDeliversTheProfileHintToWhateverSinkTheSupplierCurrentlyReturns` — calls `ExhaustionSink.forwardingTo(ref::get)` directly. - `OpenCodeLauncherTest#theSpawnTimeQuarantineSurvivesTheFleetdStyleForwardingHop` — same call inside the full construction-order replica, driven through the real `SessionManager.acquire()` path. No hand-built forwarder remains in either test file. (I also deleted the round-2 documentation-only test that demonstrated the hazard with a deliberately-broken local lambda — it never called production code either way, and is now redundant with the factory-level test.) ### Acceptance **1 & 2 — mutation on the factory body itself**, no test file touched: ```java static ExhaustionSink forwardingTo(Supplier<ExhaustionSink> target) { // 3-arg override deleted, falls back to the interface default return new ExhaustionSink() { @Override public void onExhausted(String t, String r) { target.get().onExhausted(t, r); } }; } ``` Red: ``` ExhaustionSinkForwardingHazardTest.forwardingToDeliversTheProfileHintToWhateverSinkTheSupplierCurrentlyReturns:43 ExhaustionSink.forwardingTo must deliver the profile hint to the sink the supplier currently returns ==> expected: <gx> but was: <null> OpenCodeLauncherTest.theSpawnTimeQuarantineSurvivesTheFleetdStyleForwardingHop:1357 the profile hint must survive the Fleetd-style forwarding hop and reach the real sink ==> expected: <true> but was: <false> Tests run: 68, Failures: 2, Errors: 0, Skipped: 0 BUILD FAILURE ``` Restored, green: ``` Tests run: 68, Failures: 0, Errors: 0, Skipped: 0 BUILD SUCCESS ``` **3** — `grep -n "ExhaustionSink" src/main/java/dev/ltms/fleet/Fleetd.java`: no lambda anywhere — the forwarder is now a `forwardingTo(...)` call, and the real sink (built later, once `sessions` exists) is unchanged from round 2, still an anonymous class overriding both overloads. `ExhaustionSink.none()` stays a lambda on purpose: both its overloads are true no-ops regardless of arity, so there is no hint to drop. **4 — full build, unpiped, from inside `fleetd/`:** ``` Tests run: 1129, Failures: 0, Errors: 0, Skipped: 0 BUILD SUCCESS ``` (1129 vs round 2's 1130 — net -1 from deleting the now-redundant documentation-only test above.) ### The check you asked me to satisfy > after your change, editing only `ExhaustionSink.java` or only `Fleetd.java` must be able to turn a test red Verified directly: the mutation above only touched `ExhaustionSink.java` (the factory body), touched no test file, and both new tests went red with the exact assertions above. Pushed to `worker/cb234-session-id-keyed-04e1fc-1` (commit c935b18), same PR #236, no new PR.
agent added 1 commit 2026-09-03 05:46:34 +02:00
fleetd #234 round 4: invert ExhaustionSink's abstract method so the bug class is unrepresentable
CI / contract (pull_request) Successful in 46s
Details
CI / build (pull_request) Successful in 1m23s
Details
31b028e860 
Round 3's factory fixed the two known call sites but the underlying shape
was still there: a lambda written against ExhaustionSink binds to whichever
overload is abstract, and the 2-arg form held that position, so ANY lambda
-- a call-site forwarder, a hand-built test double, a future caller who has
never heard of fleetd #234 -- could still silently take the hint-dropping
default. Two rounds shipped exactly that mistake in two different places.

Fix: made the 3-arg onExhausted(target, reason, profile) the interface's
single abstract method; the 2-arg form is now a default that delegates with
a null profile. A lambda declared against ExhaustionSink today is forced by
the compiler to take three parameters -- there is no overload left for it to
bind to that can drop the hint. This is enforced by the type system, not by
a test that has to remember to check for it.

Knock-on changes:
- ExhaustionSink.none() -- a 3-arg lambda, still a genuine no-op, now safe
  by construction rather than by care.
- ExhaustionSink.forwardingTo(...) -- collapses to a one-line 3-arg lambda;
  kept as a named factory (round 3's lesson: a test must call the real
  object, not rebuild its shape).
- Fleetd.java's real sink and the two OpenCodeLauncherTest sinks that used
  to be anonymous classes overriding both overloads are now plain lambdas
  too -- the 2-arg override each carried was pure boilerplate once the
  interface provides it as a default.
- CompletionResolver.java itself: UNCHANGED, zero diff (confirmed via
  `git diff --stat` before staging) -- its two call sites still call the
  2-arg onExhausted(target, reason), which is now the default and behaves
  identically. CompletionResolverTest (41 tests, 0 failures) proves this;
  its five ExhaustionSink lambdas needed a mechanical third parameter added
  to keep compiling against the new abstract method, no assertion changed.

Mutation proof, re-run against the new shape: forwardingTo's body edited to
call the 2-arg default instead of passing the hint through (the equivalent
of round 3's "delete the 3-arg override" now that there is only one method
to break) -- both new tests go red with the same assertions as round 3:

  ExhaustionSinkForwardingHazardTest...: expected: <gx> but was: <null>
  OpenCodeLauncherTest...ForwardingHop:  expected: <true> but was: <false>
  Tests run: 68, Failures: 2

Restored, re-ran: green (Tests run: 109, Failures: 0, including
CompletionResolverTest).

Compiler proof (not committed -- a scratch file outside the worktree,
compiled with the real ExhaustionSink.java on the classpath, then deleted):

  ExhaustionSink forwarder = (target, reason) -> System.out.println(target + reason);

  error: incompatible types: incompatible parameter types in lambda expression

A 2-arg lambda against this interface no longer compiles at all.

mvn clean install: Tests run: 1129, Failures: 0, Errors: 0, Skipped: 0, BUILD SUCCESS.
agent commented 2026-09-03 05:46:52 +02:00
Author
Member
Copy Link
Copy Source

Round 4 (final) — inverted ExhaustionSink so the bug class cannot be written

Made the 3-arg onExhausted(target, reason, profile) the interface's single abstract method; the 2-arg form is now a default that delegates with profile = null. A lambda declared against ExhaustionSink today has no 2-arg overload left to bind to — the compiler forces three parameters.

1 — abstract/default inverted

void onExhausted(String target, String reason, String profile);

default void onExhausted(String target, String reason) {
    onExhausted(target, reason, null);
}

2 — CompletionResolver's call sites: untouched

git diff --stat on CompletionResolver.java before staging: zero output — the file has no diff at all. Its two call sites still read exhaustionSink.onExhausted(target, reason), which now resolves to the default, behaving identically to before.

CompletionResolverTest: 41 tests, 0 failures (part of the 109-test run below), proving this. Its five ExhaustionSink lambdas needed a mechanical third parameter added to keep compiling against the new abstract method ((target, reason) -> ... → (target, reason, profile) -> ...) — no assertion in that file changed. Diff for one representative site:

-        ExhaustionSink sink = (target, reason) -> notified.add(target + ": " + reason);
+        ExhaustionSink sink = (target, reason, profile) -> notified.add(target + ": " + reason);

(Flagging: CompletionResolverTest.java is technically outside my file-ownership list for this ticket, but the interface inversion you asked for makes this edit unavoidable everywhere ExhaustionSink is implemented — it's the one file outside my assigned scope this round touched, and only mechanically.)

3 — mutation re-run against the new shape

forwardingTo is now a single-line 3-arg lambda, so round 3's "delete the 3-arg override" doesn't apply verbatim — the equivalent mutation is making the factory call the 2-arg default instead of passing the hint through:

static ExhaustionSink forwardingTo(Supplier<ExhaustionSink> target) {
    return (t, r, p) -> target.get().onExhausted(t, r);   // drops p
}

Red, no test file touched:

ExhaustionSinkForwardingHazardTest.forwardingToDeliversTheProfileHintToWhateverSinkTheSupplierCurrentlyReturns:37
  ==> expected: <gx> but was: <null>
OpenCodeLauncherTest.theSpawnTimeQuarantineSurvivesTheFleetdStyleForwardingHop:1343
  ==> expected: <true> but was: <false>
Tests run: 68, Failures: 2, Errors: 0, Skipped: 0
BUILD FAILURE

Restored, green:

Tests run: 109, Failures: 0, Errors: 0, Skipped: 0
BUILD SUCCESS

(109 = the 68 from before plus CompletionResolverTest's 41, run together to also confirm item 2 in the same pass.)

4 — compiler proof (not committed)

Wrote a scratch file OUTSIDE the worktree (/tmp/cb234-scratch/...), copied the real ExhaustionSink.java next to it, compiled with javac directly:

ExhaustionSink forwarder = (target, reason) -> System.out.println(target + reason);

error: incompatible types: incompatible parameter types in lambda expression
        ExhaustionSink forwarder = (target, reason) -> System.out.println(target + reason);
                                   ^
1 error

Deleted the scratch directory afterward (rm -rf /tmp/cb234-scratch) — nothing staged or committed from it.

5 — full build, unpiped, from inside fleetd/

Tests run: 1129, Failures: 0, Errors: 0, Skipped: 0
BUILD SUCCESS

Other knock-on cleanups

  • ExhaustionSink.none() — now a 3-arg lambda, still a genuine no-op, safe by construction rather than by care.
  • Fleetd.java's real sink, and the two OpenCodeLauncherTest sinks that were anonymous classes overriding both overloads — all now plain lambdas, since the 2-arg override each carried was pure boilerplate once the interface supplies it as a default.

Pushed to worker/cb234-session-id-keyed-04e1fc-1 (commit 31b028e), same PR #236, no new PR.

## Round 4 (final) — inverted ExhaustionSink so the bug class cannot be written Made the 3-arg `onExhausted(target, reason, profile)` the interface's single abstract method; the 2-arg form is now a `default` that delegates with `profile = null`. A lambda declared against `ExhaustionSink` today has no 2-arg overload left to bind to — the compiler forces three parameters. ### 1 — abstract/default inverted ```java void onExhausted(String target, String reason, String profile); default void onExhausted(String target, String reason) { onExhausted(target, reason, null); } ``` ### 2 — `CompletionResolver`'s call sites: untouched `git diff --stat` on `CompletionResolver.java` before staging: **zero output** — the file has no diff at all. Its two call sites still read `exhaustionSink.onExhausted(target, reason)`, which now resolves to the default, behaving identically to before. `CompletionResolverTest`: **41 tests, 0 failures** (part of the 109-test run below), proving this. Its five `ExhaustionSink` lambdas needed a mechanical third parameter added to keep compiling against the new abstract method (`(target, reason) -> ...` → `(target, reason, profile) -> ...`) — no assertion in that file changed. Diff for one representative site: ```diff - ExhaustionSink sink = (target, reason) -> notified.add(target + ": " + reason); + ExhaustionSink sink = (target, reason, profile) -> notified.add(target + ": " + reason); ``` (Flagging: `CompletionResolverTest.java` is technically outside my file-ownership list for this ticket, but the interface inversion you asked for makes this edit unavoidable everywhere `ExhaustionSink` is implemented — it's the one file outside my assigned scope this round touched, and only mechanically.) ### 3 — mutation re-run against the new shape `forwardingTo` is now a single-line 3-arg lambda, so round 3's "delete the 3-arg override" doesn't apply verbatim — the equivalent mutation is making the factory call the 2-arg default instead of passing the hint through: ```java static ExhaustionSink forwardingTo(Supplier<ExhaustionSink> target) { return (t, r, p) -> target.get().onExhausted(t, r); // drops p } ``` Red, no test file touched: ``` ExhaustionSinkForwardingHazardTest.forwardingToDeliversTheProfileHintToWhateverSinkTheSupplierCurrentlyReturns:37 ==> expected: <gx> but was: <null> OpenCodeLauncherTest.theSpawnTimeQuarantineSurvivesTheFleetdStyleForwardingHop:1343 ==> expected: <true> but was: <false> Tests run: 68, Failures: 2, Errors: 0, Skipped: 0 BUILD FAILURE ``` Restored, green: ``` Tests run: 109, Failures: 0, Errors: 0, Skipped: 0 BUILD SUCCESS ``` (109 = the 68 from before plus CompletionResolverTest's 41, run together to also confirm item 2 in the same pass.) ### 4 — compiler proof (not committed) Wrote a scratch file OUTSIDE the worktree (`/tmp/cb234-scratch/...`), copied the real `ExhaustionSink.java` next to it, compiled with `javac` directly: ```java ExhaustionSink forwarder = (target, reason) -> System.out.println(target + reason); ``` ``` error: incompatible types: incompatible parameter types in lambda expression ExhaustionSink forwarder = (target, reason) -> System.out.println(target + reason); ^ 1 error ``` Deleted the scratch directory afterward (`rm -rf /tmp/cb234-scratch`) — nothing staged or committed from it. ### 5 — full build, unpiped, from inside `fleetd/` ``` Tests run: 1129, Failures: 0, Errors: 0, Skipped: 0 BUILD SUCCESS ``` ### Other knock-on cleanups - `ExhaustionSink.none()` — now a 3-arg lambda, still a genuine no-op, safe by construction rather than by care. - `Fleetd.java`'s real sink, and the two `OpenCodeLauncherTest` sinks that were anonymous classes overriding both overloads — all now plain lambdas, since the 2-arg override each carried was pure boilerplate once the interface supplies it as a default. Pushed to `worker/cb234-session-id-keyed-04e1fc-1` (commit 31b028e), same PR #236, no new PR.
ltms commented 2026-09-03 05:57:20 +02:00
Owner
Copy Link
Copy Source

Merged to main in 838a701. Round 4 is the one that ships.

I verified the inversion with javac against the real interface file, not against a copy:

Probe.java:4: error: incompatible types: incompatible parameter types in lambda expression
        ExhaustionSink forwarder = (target, reason) -> System.out.println(target + reason);

The bug class is now unrepresentable rather than merely tested, which is the point. It proved itself straight away: merging Unit 1 (#240) on top produced exactly one compile error — a two-argument lambda that Unit 1 had added to CompletionResolverTest. Under the old shape that lambda would have compiled, run, and silently dropped the profile.

Full integrated build after the merge: 1163 tests, 0 failures, 0 compile errors.

For the record, the three rounds this took, because the pattern is worth remembering:

  1. The 3-argument overload was added as a default, so Fleetd.java:177's two-argument lambda kept compiling and the fix never ran in production.
  2. The new tests built their own forwarder, so mutating production back to a lambda left all 52 tests green.
  3. The forwardingTo factory fixed the call site but still allowed the bad shape elsewhere.

The lesson: when you add an overload, the unit of work is every implementation, and a default method is invisible to a lambda.

Merged to main in `838a701`. Round 4 is the one that ships. I verified the inversion with `javac` against the real interface file, not against a copy: ``` Probe.java:4: error: incompatible types: incompatible parameter types in lambda expression ExhaustionSink forwarder = (target, reason) -> System.out.println(target + reason); ``` The bug class is now unrepresentable rather than merely tested, which is the point. It proved itself straight away: merging Unit 1 (#240) on top produced exactly one compile error — a two-argument lambda that Unit 1 had added to `CompletionResolverTest`. Under the old shape that lambda would have compiled, run, and silently dropped the profile. Full integrated build after the merge: **1163 tests, 0 failures, 0 compile errors**. For the record, the three rounds this took, because the pattern is worth remembering: 1. The 3-argument overload was added as a `default`, so `Fleetd.java:177`'s two-argument lambda kept compiling and the fix never ran in production. 2. The new tests built their own forwarder, so mutating production back to a lambda left all 52 tests green. 3. The `forwardingTo` factory fixed the call site but still allowed the bad shape elsewhere. The lesson: when you add an overload, the unit of work is every *implementation*, and a `default` method is invisible to a lambda.
ltms closed this pull request 2026-09-03 05:57:44 +02:00
Some checks are pending
CI / contract (pull_request) Successful in 46s
Details
CI / build (pull_request) Successful in 1m23s
Details

Pull request closed

Please reopen this pull request to perform a merge.
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
2 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: fleet/fleetd#236
Delete Branch "worker/cb234-session-id-keyed-04e1fc-1"

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?