diff --git a/fleetd/src/main/java/dev/ltms/fleet/Fleetd.java b/fleetd/src/main/java/dev/ltms/fleet/Fleetd.java index a8eed1b..5580446 100644 --- a/fleetd/src/main/java/dev/ltms/fleet/Fleetd.java +++ b/fleetd/src/main/java/dev/ltms/fleet/Fleetd.java @@ -174,7 +174,25 @@ public final class Fleetd { // genuine cycle. Break it exactly like liveCountRef below: a forwarding sink built now, // pointed at the real one once it exists. AtomicReference exhaustionSinkRef = new AtomicReference<>(ExhaustionSink.none()); - ExhaustionSink forwardingExhaustionSink = (target, reason) -> exhaustionSinkRef.get().onExhausted(target, reason); + // fleetd #234: this MUST be an anonymous class, not a lambda. A lambda can only implement + // the interface's single abstract method (the 2-arg overload) — it then inherits the 3-arg + // method's DEFAULT body, which drops the profile hint and calls back into the 2-arg + // overload here, so OpenCodeLauncher's hint (its own already-known profile name, passed + // for exactly the reason explained below at the real sink's construction) never reaches + // the real sink at all. That silently reproduced the very bug this hint exists to fix: a + // lambda here means the roster-miss path always fires, even after the hint was supplied. + // Both overloads must forward explicitly to whatever exhaustionSinkRef currently holds. + 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); + } + }; // The claude-code adapter is the always-present default; keep it even with no profiles (so a // bridge configured with no workers, or opencode-only, still has a well-defined base adapter) // unless opencode is the only kind configured. diff --git a/fleetd/src/test/java/dev/ltms/fleet/inject/ExhaustionSinkForwardingHazardTest.java b/fleetd/src/test/java/dev/ltms/fleet/inject/ExhaustionSinkForwardingHazardTest.java new file mode 100644 index 0000000..c788234 --- /dev/null +++ b/fleetd/src/test/java/dev/ltms/fleet/inject/ExhaustionSinkForwardingHazardTest.java @@ -0,0 +1,96 @@ +package dev.ltms.fleet.inject; + +import org.junit.jupiter.api.Test; + +import java.util.concurrent.atomic.AtomicReference; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNull; + +/** + * fleetd #234 follow-up: a lambda implementing {@link ExhaustionSink} can only ever implement the + * interface's single abstract method — the 2-arg {@link ExhaustionSink#onExhausted(String, String)} + * — so it silently inherits the 3-arg overload's {@code default} body, which drops whatever profile + * hint a caller supplied and calls back into the 2-arg method instead. {@code Fleetd.main} built + * exactly this shape at {@code Fleetd.java:177} — a forwarding sink standing in for the real one + * until {@code sessions} exists (a genuine construction-order cycle) — as a lambda, so every hint + * {@link dev.ltms.fleet.member.OpenCodeLauncher} passed through it was thrown away before it ever + * reached the real sink. The fix (defect 2, round 2) reported ERROR and correctly declined to + * quarantine in that situation — which is exactly why the earlier {@code OpenCodeLauncherTest} + * mutation tests, which inject a sink directly into the launcher and never go through this + * forwarding hop, could not see the bug: the hop itself was the defect. + * + *

This test pins the hazard at the interface level, independent of {@code Fleetd.java}'s + * specific wiring: ANY forwarding sink standing in front of another {@link ExhaustionSink} must be + * an anonymous class (or otherwise override both overloads) — a lambda there is a silent regression + * of this exact bug. See {@code OpenCodeLauncherTest#theSpawnTimeQuarantineSurvivesTheFleetdStyleForwardingHop} + * for the composed, production-shaped reproduction (launcher mismatch → forwarder → real sink → + * quarantine). + */ +class ExhaustionSinkForwardingHazardTest { + + @Test + void aLambdaForwarderDropsTheProfileHintBeforeItReachesTheRealSink() { + AtomicReference hintSeenByRealSink = new AtomicReference<>("NEVER CALLED"); + ExhaustionSink real = new ExhaustionSink() { + @Override + public void onExhausted(String target, String reason) { + onExhausted(target, reason, null); + } + + @Override + public void onExhausted(String target, String reason, String profileHint) { + hintSeenByRealSink.set(profileHint); + } + }; + + AtomicReference ref = new AtomicReference<>(ExhaustionSink.none()); + // The broken shape: a lambda can only implement the 2-arg method. + ExhaustionSink brokenForwarder = (target, reason) -> ref.get().onExhausted(target, reason); + ref.set(real); + + brokenForwarder.onExhausted("term_x", "model mismatch", "gx"); + + assertNull(hintSeenByRealSink.get(), + "documents the hazard: a lambda forwarder can only implement the 2-arg overload, so " + + "it forwards through that overload alone — the real sink's 3-arg method " + + "still runs (via its own default), but with the hint already discarded"); + } + + @Test + void profileHintSurvivesTheForwardingHopUsedInProduction() { + AtomicReference hintSeenByRealSink = new AtomicReference<>("NEVER CALLED"); + ExhaustionSink real = new ExhaustionSink() { + @Override + public void onExhausted(String target, String reason) { + onExhausted(target, reason, null); + } + + @Override + public void onExhausted(String target, String reason, String profileHint) { + hintSeenByRealSink.set(profileHint); + } + }; + + AtomicReference ref = new AtomicReference<>(ExhaustionSink.none()); + // The fixed shape (fleetd #234, matching Fleetd.java:177): an anonymous class overriding + // BOTH overloads, each forwarding to the currently-held sink. + ExhaustionSink forwarding = new ExhaustionSink() { + @Override + public void onExhausted(String target, String reason) { + ref.get().onExhausted(target, reason); + } + + @Override + public void onExhausted(String target, String reason, String profile) { + ref.get().onExhausted(target, reason, profile); + } + }; + ref.set(real); + + forwarding.onExhausted("term_x", "model mismatch", "gx"); + + assertEquals("gx", hintSeenByRealSink.get(), + "the profile hint must reach the real sink through the forwarding hop"); + } +} diff --git a/fleetd/src/test/java/dev/ltms/fleet/member/OpenCodeLauncherTest.java b/fleetd/src/test/java/dev/ltms/fleet/member/OpenCodeLauncherTest.java index bdde1e5..bb6a563 100644 --- a/fleetd/src/test/java/dev/ltms/fleet/member/OpenCodeLauncherTest.java +++ b/fleetd/src/test/java/dev/ltms/fleet/member/OpenCodeLauncherTest.java @@ -1286,4 +1286,81 @@ class OpenCodeLauncherTest { "a roster-only sink cannot see this target yet — the quarantine silently never " + "happens, which is exactly fleetd #234 defect 2"); } + + /** + * fleetd #234, round 2: the two tests above inject a sink DIRECTLY into {@link OpenCodeLauncher}, + * which is not what {@code Fleetd.main} actually does. Production has one more hop: {@code + * Fleetd.java} builds the adapters (including {@code OpenCodeLauncher}) before {@code sessions} + * exists — a genuine construction-order cycle — so it hands the launcher a forwarding + * sink pointed at an {@code AtomicReference}, and only later {@code .set(...)}s + * that reference to the real sink once {@code sessions} is built. That forwarding sink was + * written as a lambda ({@code Fleetd.java:177}), which can only implement the 2-arg overload — + * it silently inherited the 3-arg method's default body, dropping {@link OpenCodeLauncher}'s + * profile hint on every call, so the roster-miss branch fired even after the hint fix above + * shipped. This test reproduces that exact shape (construct with the forwarder, {@code .set()} + * the real sink afterward, same order as {@code Fleetd.main}) and would fail if the forwarder + * were ever written as a lambda again — see the class javadoc on {@code + * ExhaustionSinkForwardingHazardTest} for the isolated proof of the underlying mechanism. + */ + @Test + void theSpawnTimeQuarantineSurvivesTheFleetdStyleForwardingHop(@TempDir Path configRoot, + @TempDir Path discRoot) throws Exception { + FleetConfig.Profile cfg = opencodeCfgWithCredential( + "terra", "opencode/nemotron-3-ultra-free", "openai-shared"); + Map profiles = Map.of(cfg.profile(), cfg); + BackendQuarantine quarantine = new BackendQuarantine(() -> 0L, TimeUnit.SECONDS.toNanos(1800)); + + // Fleetd.java:176 — the forwarding sink is built BEFORE the real one can exist, and the + // launcher below is constructed against this forwarder, exactly like Fleetd.main. + java.util.concurrent.atomic.AtomicReference exhaustionSinkRef = + new java.util.concurrent.atomic.AtomicReference<>(ExhaustionSink.none()); + // Fleetd.java:177 fixed shape — an anonymous class forwarding BOTH overloads. Rewriting + // this as `(target, reason) -> exhaustionSinkRef.get().onExhausted(target, reason)` is + // exactly the regression this test exists to catch. + 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); + } + }; + + FakeHerdr herdr = new FakeHerdr(); + OpenCodeLauncher launcher = serviceWithSink(herdr, configRoot, discRoot, cfg, forwardingExhaustionSink); + SessionManager sessions = new SessionManager(launcher); + + // Fleetd.java:359-390 — the real sink is only built and pointed to AFTER `sessions` exists, + // same order as production. + ExhaustionSink realSink = new ExhaustionSink() { + @Override + public void onExhausted(String target, String reason) { + onExhausted(target, reason, null); // no roster resolution modelled — mirrors a miss + } + + @Override + public void onExhausted(String target, String reason, String profileHint) { + FleetConfig.Profile profile = profileHint == null ? null : profiles.get(profileHint); + if (profile != null) { + quarantine.quarantine(profile.effectiveCredentialId()); + } + } + }; + exhaustionSinkRef.set(realSink); + + OpenCodeSessionDiscoveryTest.writeRecord(discRoot, "ses_x", "/work/dir", 1000L, + "{\"id\":\"gpt-5.6-sol\",\"providerID\":\"openai\"}"); + + assertFalse(quarantine.isQuarantined("openai-shared"), "nothing quarantined before the spawn"); + + MemberSession acquired = sessions.acquire(cfg.profile(), "/work/dir", null, null); + + assertEquals("ses_x", acquired.agentSessionId(), "the id itself still resolves correctly"); + assertTrue(quarantine.isQuarantined("openai-shared"), + "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"); + } }