From ae7845c375538450fa7e2edb7fc1feff1da008a9 Mon Sep 17 00:00:00 2001 From: Dai Ha Date: Sat, 19 Sep 2026 15:35:32 +0700 Subject: [PATCH] fleetd #589 (Groups 1 & 2): pin 6 main() wiring sites with named factories MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Extracts 6 inline wiring expressions from Fleetd.main() into named, directly-testable package-private static factories, following the FleetdLoopHealthSourceWiringTest (#584) shape, and adds one wiring test per factory: Group 1 (exhaustion/quarantine): - forwardingExhaustionSink(exhaustionSinkRef) — was inline ExhaustionSink.forwardingTo(exhaustionSinkRef::get) - publishExhaustionSink(...) — was two untested statements building the real sink and .set()-ing it into exhaustionSinkRef - liveExhaustedPatterns(config) — was inline new LiveExhaustedPatterns(() -> config.get().profiles()) - exhaustedPatternLookup(roster, liveExhaustedPatterns) — was an inline lambda resolving a herdr target to its profile's live pattern; silently losing this is the worst regression in the sweep, since a real usage-limit refusal would stop being classified as BACKEND_EXHAUSTED Group 2 (CB-596 credential policy): - claudeCodeLauncher(...) — was an inline `new ClaudeCodeLauncher(...)` whose memberCredentials supplier argument was untestable wiring - openCodeLauncher(...) — same, for OpenCodeLauncher Each new test pins its factory behaviorally (never via source-text assertions): built and confirmed RED by name against the named inert mutation, then confirmed GREEN again after restoring, and separately confirmed GREEN after a behavior-preserving reformat/local-variable extraction of the same call, to rule out a disguised source-text test. Suite: 1789 -> 1799 tests (+10, matching the 10 tests added), 0 failures, mvn -o clean install BUILD SUCCESS. Scope strictly limited to main()'s :214-:468 range per the ticket split with the concurrent worker handling Group 3 at line 500+. --- .../src/main/java/dev/ltms/fleet/Fleetd.java | 156 +++++++++++++++--- ...laudeCodeLauncherCredentialWiringTest.java | 84 ++++++++++ ...leetdExhaustedPatternLookupWiringTest.java | 86 ++++++++++ ...etdExhaustionSinkForwardingWiringTest.java | 62 +++++++ ...FleetdExhaustionSinkPublishWiringTest.java | 110 ++++++++++++ ...FleetdLiveExhaustedPatternsWiringTest.java | 72 ++++++++ ...dOpenCodeLauncherCredentialWiringTest.java | 77 +++++++++ 7 files changed, 627 insertions(+), 20 deletions(-) create mode 100644 fleetd/src/test/java/dev/ltms/fleet/FleetdClaudeCodeLauncherCredentialWiringTest.java create mode 100644 fleetd/src/test/java/dev/ltms/fleet/FleetdExhaustedPatternLookupWiringTest.java create mode 100644 fleetd/src/test/java/dev/ltms/fleet/FleetdExhaustionSinkForwardingWiringTest.java create mode 100644 fleetd/src/test/java/dev/ltms/fleet/FleetdExhaustionSinkPublishWiringTest.java create mode 100644 fleetd/src/test/java/dev/ltms/fleet/FleetdLiveExhaustedPatternsWiringTest.java create mode 100644 fleetd/src/test/java/dev/ltms/fleet/FleetdOpenCodeLauncherCredentialWiringTest.java diff --git a/fleetd/src/main/java/dev/ltms/fleet/Fleetd.java b/fleetd/src/main/java/dev/ltms/fleet/Fleetd.java index a93637f..88c092d 100644 --- a/fleetd/src/main/java/dev/ltms/fleet/Fleetd.java +++ b/fleetd/src/main/java/dev/ltms/fleet/Fleetd.java @@ -218,23 +218,23 @@ public final class Fleetd { // there is no 2-arg overload left for any lambda to silently bind to instead), but so a // test can call the exact same object this line builds, instead of asserting a copy of its // shape (round 3's lesson). - ExhaustionSink forwardingExhaustionSink = ExhaustionSink.forwardingTo(exhaustionSinkRef::get); + // fleetd #589 Group 1: extracted to forwardingExhaustionSink(...) below (see that method's + // javadoc) so a dedicated test can prove this factory keeps reading the reference live, + // rather than a rebuilt copy of its shape. + ExhaustionSink forwardingExhaustionSink = forwardingExhaustionSink(exhaustionSinkRef); // 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. + // fleetd #589 Group 2: extracted to claudeCodeLauncher(...)/openCodeLauncher(...) below (see + // those methods' javadoc) so a dedicated test can prove the CB-596 memberCredentials policy + // supplier is actually wired to each adapter, not silently replaced with `() -> null`. if (!claudeProfiles.isEmpty() || opencodeProfiles.isEmpty()) { - adapters.add(new ClaudeCodeLauncher(router.memberAgents(), router.memberSpaces(), guard, - claudeProfiles, cfg.effectiveDefaultProfile(), System::getenv, - cfg.spawnReadyTimeoutMs(), cfg.spawnReadyPollMs(), - () -> config.get().fleet(), - () -> config.get().memberCredentials(), null, config::get)); + adapters.add(claudeCodeLauncher(router.memberAgents(), router.memberSpaces(), guard, + claudeProfiles, cfg, config)); } if (!opencodeProfiles.isEmpty()) { - adapters.add(new OpenCodeLauncher(router.memberAgents(), router.memberSpaces(), - opencodeProfiles, cfg.effectiveDefaultProfile(), System::getenv, - cfg.spawnReadyTimeoutMs(), cfg.spawnReadyPollMs(), - () -> config.get().fleet(), - () -> config.get().memberCredentials(), config::get, forwardingExhaustionSink)); + adapters.add(openCodeLauncher(router.memberAgents(), router.memberSpaces(), + opencodeProfiles, cfg, config, forwardingExhaustionSink)); } AtomicReference> liveCountRef = new AtomicReference<>(_ -> 0); // CB-578 stage B: one quarantine tracker for the whole daemon, shared between the launcher @@ -408,12 +408,12 @@ public final class Fleetd { // LiveExhaustedPatterns's class doc for why this replaces the old compiled-once-at-startup // map. A profile with no exhaustedPattern simply returns null here, so its workers keep // today's completion-fallback behaviour unchanged. - LiveExhaustedPatterns liveExhaustedPatterns = new LiveExhaustedPatterns(() -> config.get().profiles()); - ExhaustedPatternLookup exhaustedPatterns = target -> sessions.roster().stream() - .filter(session -> target.equals(session.terminalId())) - .findFirst() - .map(session -> liveExhaustedPatterns.patternFor(session.profile())) - .orElse(null); + // fleetd #589 Group 1: both extracted to liveExhaustedPatterns(...)/ + // exhaustedPatternLookup(...) below (see those methods' javadoc) — this is the worst + // consequence in the whole #589 sweep: silently losing either wiring means a genuine + // usage-limit refusal is handed back as a real completion instead of BACKEND_EXHAUSTED. + LiveExhaustedPatterns liveExhaustedPatterns = liveExhaustedPatterns(config); + ExhaustedPatternLookup exhaustedPatterns = exhaustedPatternLookup(sessions::roster, liveExhaustedPatterns); // The startup coverage line still reports the boot-time snapshot only — it is printed once, // here, and a reload no longer needs to change what it said; exhaustionDetectionArmed (via // liveExhaustedPatterns.armed, wired into quarantineSource below) is what stays live. @@ -461,11 +461,13 @@ public final class Fleetd { // method's javadoc for the full fleetd #175/#234/#446 history this used to carry inline — // so a dedicated test can drive the exact ExhaustionSink main() builds, not a hand-rebuilt // copy of its shape. - ExhaustionSink exhaustionSink = exhaustionSink(sessions, config, quarantine, - quarantineReasonByCredential, cfg); // fleetd #175: point the forwarding sink handed to OpenCodeLauncher above at the real one, // now that `sessions` exists to resolve target -> session -> profile. - exhaustionSinkRef.set(exhaustionSink); + // fleetd #589 Group 1: both statements (build + set) folded into publishExhaustionSink(...) + // below (see that method's javadoc), so a test can prove the reference is actually + // repointed at the real sink, not silently left at ExhaustionSink.none(). + ExhaustionSink exhaustionSink = publishExhaustionSink(exhaustionSinkRef, sessions, config, + quarantine, quarantineReasonByCredential, cfg); // fleetd #201 Unit 5: the production BackendErrorSink needs `pushLoop` (built further below, // after `sessions`) to tell a lead about an incident or an unmapped target — the same // construction-order cycle `exhaustionSinkRef` breaks above, broken the same way: a mutable @@ -1009,6 +1011,120 @@ public final class Fleetd { cfg.profiles()::keySet, System::nanoTime); } + /** + * fleetd #589 Group 1: the forwarding {@link ExhaustionSink} handed to the adapters built + * before {@code sessions} exists (see the {@code exhaustionSinkRef}/{@code + * forwardingExhaustionSink} locals in {@code main}, just above {@link #capacitySource}'s call + * site). Before this ticket, {@code ExhaustionSink.forwardingTo(exhaustionSinkRef::get)} was + * built inline — nothing a test could call directly, so a mutation swapping the supplier for a + * hardcoded {@code () -> ExhaustionSink.none()} compiled clean and left the suite green: the + * forwarder would silently stop reading the reference at all, and {@link + * #publishExhaustionSink} repointing that reference later would have no effect. + * + *

Extracted the same way {@link #capacitySource}/{@link #loopHealthSource} were, so {@code + * FleetdExhaustionSinkForwardingWiringTest} can call this factory directly with a real {@link + * AtomicReference}, mutate the reference AFTER the forwarder is built, and prove the forwarder + * still reads it live rather than a fixed target captured at construction time. + */ + static ExhaustionSink forwardingExhaustionSink(AtomicReference exhaustionSinkRef) { + return ExhaustionSink.forwardingTo(exhaustionSinkRef::get); + } + + /** + * fleetd #589 Group 1: publish the real {@link ExhaustionSink} — built the same way {@link + * #exhaustionSink} always was — into the forwarding reference {@link #forwardingExhaustionSink} + * built above, replacing {@code main}'s previously untested two-statement sequence ({@code + * ExhaustionSink exhaustionSink = exhaustionSink(...); exhaustionSinkRef.set(exhaustionSink);}). + * Before this ticket, nothing proved the {@code .set(...)} call actually received the real sink + * rather than a hardcoded {@code ExhaustionSink.none()} — the whole point of {@code + * exhaustionSinkRef} existing (fleetd #175) is that {@link + * dev.ltms.fleet.member.OpenCodeLauncher}'s model-mismatch check, built before {@code sessions} + * exists, keeps working once this line runs; silently keeping the reference at {@code none()} + * would mean that check permanently does nothing, with the full suite still green because no + * existing test drives this exact call site. + * + *

Returns the built sink so {@code main} can still pass it to {@link CompletionResolver}'s + * constructor at the same call site it already does, without building it twice. + */ + static ExhaustionSink publishExhaustionSink(AtomicReference exhaustionSinkRef, + SessionManager sessions, ConfigRef config, BackendQuarantine quarantine, + Map quarantineReasonByCredential, FleetConfig cfg) { + ExhaustionSink sink = exhaustionSink(sessions, config, quarantine, quarantineReasonByCredential, cfg); + exhaustionSinkRef.set(sink); + return sink; + } + + /** + * fleetd #589 Group 1: the live {@code exhaustedPattern} source (fleetd #446) {@link + * CompletionResolver} enforces on, extracted out of {@code main} for the same reason {@link + * #capacitySource} was. Before this ticket {@code new LiveExhaustedPatterns(() -> + * config.get().profiles())} was built inline; replacing the supplier with a hardcoded {@code () + * -> Map.of()} compiled clean and left the suite green, meaning every profile's {@code + * exhaustedPattern} would silently stop being recognised and a genuine usage-limit refusal + * would be handed back as a real completion instead of {@code BACKEND_EXHAUSTED}. + */ + static LiveExhaustedPatterns liveExhaustedPatterns(ConfigRef config) { + return new LiveExhaustedPatterns(() -> config.get().profiles()); + } + + /** + * fleetd #589 Group 1: the {@link ExhaustedPatternLookup} {@link CompletionResolver} enforces + * on, resolving a herdr {@code target} to its session's profile and then to that profile's live + * {@link LiveExhaustedPatterns#patternFor}. Extracted out of {@code main} the same way {@link + * #worktreeBranchLookup} was — same {@code Supplier>} roster shape, same + * reason: before this ticket the lambda was built inline, and replacing it with {@code target -> + * null} (the exact shape of {@link ExhaustedPatternLookup#none()}) compiled clean and left the + * suite green. This is the worst consequence in the whole #589 sweep (see the ticket): a + * genuine usage-limit refusal would stop being classified as {@code BACKEND_EXHAUSTED} and + * would be handed back to a waiting {@code fleet_send} as if it were real completed work. + * + * @param roster the live member roster, normally {@code sessions::roster} + */ + static ExhaustedPatternLookup exhaustedPatternLookup(Supplier> roster, + LiveExhaustedPatterns liveExhaustedPatterns) { + return target -> roster.get().stream() + .filter(session -> target.equals(session.terminalId())) + .findFirst() + .map(session -> liveExhaustedPatterns.patternFor(session.profile())) + .orElse(null); + } + + /** + * fleetd #589 Group 2: the production {@link ClaudeCodeLauncher} adapter, extracted out of + * {@code main} the same way {@link #capacitySource} was. Before this ticket the constructor + * call (11 arguments, including the CB-596 {@code memberCredentials} policy supplier) was built + * inline; replacing the {@code () -> config.get().memberCredentials()} argument with {@code () + * -> null} compiled clean and left the suite green — {@code memberCredentials} is not {@code + * null} itself (a lambda is never {@code null}), so {@link + * dev.ltms.fleet.member.HerdrPeerLauncher#applyMemberCredentialPolicy} sees {@code + * memberCredentials.get() == null} and silently shadows nothing, reopening the exact CB-592 + * exposure gap CB-596's policy closed. {@code FleetdClaudeCodeLauncherCredentialWiringTest} + * calls this factory with a real {@link ConfigRef} carrying a {@code memberCredentials:} block + * and proves a known-but-not-allowed name is actually shadowed on {@code spawn()}. + */ + static ClaudeCodeLauncher claudeCodeLauncher(AgentControl agents, WorkspaceControl spaces, + SubscriptionGuard guard, Map claudeProfiles, FleetConfig cfg, + ConfigRef config) { + return new ClaudeCodeLauncher(agents, spaces, guard, claudeProfiles, cfg.effectiveDefaultProfile(), + System::getenv, cfg.spawnReadyTimeoutMs(), cfg.spawnReadyPollMs(), + () -> config.get().fleet(), () -> config.get().memberCredentials(), null, config::get); + } + + /** + * fleetd #589 Group 2: the production {@link OpenCodeLauncher} adapter, the {@code opencode} + * counterpart to {@link #claudeCodeLauncher} above and extracted for the identical reason: the + * same {@code () -> config.get().memberCredentials()} argument, reopening the same CB-592 + * exposure gap if silently replaced with {@code () -> null}. + */ + static OpenCodeLauncher openCodeLauncher(AgentControl agents, WorkspaceControl spaces, + Map opencodeProfiles, FleetConfig cfg, ConfigRef config, + ExhaustionSink forwardingExhaustionSink) { + return new OpenCodeLauncher(agents, spaces, opencodeProfiles, cfg.effectiveDefaultProfile(), + System::getenv, cfg.spawnReadyTimeoutMs(), cfg.spawnReadyPollMs(), + () -> config.get().fleet(), () -> config.get().memberCredentials(), config::get, + forwardingExhaustionSink); + } + /** * fleetd #426: package-private factory for {@code fleet_list}'s {@code healthCoverage} source, * extracted out of {@code main} for the same reason {@link #capacitySource} and {@link diff --git a/fleetd/src/test/java/dev/ltms/fleet/FleetdClaudeCodeLauncherCredentialWiringTest.java b/fleetd/src/test/java/dev/ltms/fleet/FleetdClaudeCodeLauncherCredentialWiringTest.java new file mode 100644 index 0000000..658cb48 --- /dev/null +++ b/fleetd/src/test/java/dev/ltms/fleet/FleetdClaudeCodeLauncherCredentialWiringTest.java @@ -0,0 +1,84 @@ +package dev.ltms.fleet; + +import dev.ltms.fleet.config.ConfigRef; +import dev.ltms.fleet.config.FleetConfig; +import dev.ltms.fleet.guard.SubscriptionGuard; +import dev.ltms.fleet.herdr.AgentControl; +import dev.ltms.fleet.herdr.FakeHerdr; +import dev.ltms.fleet.herdr.WorkspaceControl; +import dev.ltms.fleet.member.ClaudeCodeLauncher; +import org.junit.jupiter.api.DisplayName; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.io.TempDir; + +import java.nio.file.Files; +import java.nio.file.Path; +import java.util.Map; +import java.util.Set; + +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertNotNull; + +/** + * fleetd #589 Group 2: {@link Fleetd#claudeCodeLauncher} is the factory that replaced {@code + * main}'s inline {@code new ClaudeCodeLauncher(...)} call, whose 10th argument is the CB-596 {@code + * memberCredentials} policy supplier ({@code () -> config.get().memberCredentials()}). Before this + * ticket that argument was untestable wiring: replacing it with {@code () -> null} compiled with 0 + * errors and left every existing test green, since no existing test builds the exact object {@code + * main} wires and then spawns it. {@code memberCredentials} being a lambda is never itself {@code + * null}, so {@link dev.ltms.fleet.member.HerdrPeerLauncher#applyMemberCredentialPolicy} sees {@code + * memberCredentials.get() == null} and silently shadows nothing — reopening the exact CB-592 + * exposure gap CB-596's policy closed (gitea issue #82). + * + *

This test drives the factory with a real {@link ConfigRef} carrying a {@code + * memberCredentials:} block, spawns through the resulting launcher, and inspects what {@code + * tab.create} actually carried — the same observable surface {@code ClaudeCodeLauncherTest}'s + * {@code everyKnownNameNotAllowedIsShadowedWithTheSentinel} uses for the launcher's own credential + * policy, applied here to prove {@code main}'s wiring reaches it. + */ +class FleetdClaudeCodeLauncherCredentialWiringTest { + + private static final String YAML = """ + bind: + host: 127.0.0.1 + port: 8765 + herdrSocket: ~/.config/herdr/herdr.sock + profiles: + ltms-local: + baseUrl: http://gx00.gw:8000 + model: coder + memberCredentials: + policy: deny-by-default + known: + - GITEA_ACCESS_TOKEN + """; + + @SuppressWarnings("unchecked") + private static Map startEnv(FakeHerdr herdr) { + return (Map) ((Map) herdr.lastCall("tab.create").params()).get("env"); + } + + @Test + @DisplayName("main's memberCredentials wiring reaches ClaudeCodeLauncher: a known-but-not-allowed " + + "name is shadowed on spawn") + void memberCredentialsWiringReachesClaudeCodeLauncher(@TempDir Path dir) throws Exception { + Path file = dir.resolve("fleetd.yaml"); + Files.writeString(file, YAML); + FleetConfig cfg = FleetConfig.load(file); + ConfigRef config = ConfigRef.fixed(cfg); + FakeHerdr herdr = new FakeHerdr(); + + ClaudeCodeLauncher launcher = Fleetd.claudeCodeLauncher(new AgentControl(herdr), + new WorkspaceControl(herdr), new SubscriptionGuard(Set.of("gx00.gw")), + cfg.profiles(), cfg, config); + launcher.spawn(); + + String shadowed = startEnv(herdr).get("GITEA_ACCESS_TOKEN"); + assertNotNull(shadowed, + "GITEA_ACCESS_TOKEN is 'known' but not 'allow'-ed in the loaded config — it must be " + + "explicitly shadowed on spawn; replacing the memberCredentials supplier with " + + "() -> null at the Fleetd.claudeCodeLauncher call site must fail this " + + "assertion, since a null policy shadows nothing"); + assertFalse(shadowed.isBlank(), "the overlay value must be non-blank"); + } +} diff --git a/fleetd/src/test/java/dev/ltms/fleet/FleetdExhaustedPatternLookupWiringTest.java b/fleetd/src/test/java/dev/ltms/fleet/FleetdExhaustedPatternLookupWiringTest.java new file mode 100644 index 0000000..66a0e24 --- /dev/null +++ b/fleetd/src/test/java/dev/ltms/fleet/FleetdExhaustedPatternLookupWiringTest.java @@ -0,0 +1,86 @@ +package dev.ltms.fleet; + +import dev.ltms.fleet.config.ConfigRef; +import dev.ltms.fleet.config.FleetConfig; +import dev.ltms.fleet.inject.ExhaustedPatternLookup; +import dev.ltms.fleet.inject.LiveExhaustedPatterns; +import dev.ltms.fleet.peer.MemberRole; +import dev.ltms.fleet.session.MemberSession; +import org.junit.jupiter.api.DisplayName; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.io.TempDir; + +import java.nio.file.Files; +import java.nio.file.Path; +import java.util.List; +import java.util.regex.Pattern; + +import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertNull; +import static org.junit.jupiter.api.Assertions.assertTrue; + +/** + * fleetd #589 Group 1: {@link Fleetd#exhaustedPatternLookup} is the factory that replaced {@code + * main}'s inline lambda — resolve a herdr {@code target} to its session's profile, then to that + * profile's live {@link LiveExhaustedPatterns#patternFor}. Same shape as {@link + * Fleetd#worktreeBranchLookup} (which {@code FleetdWorktreeBranchLookupTest} pins the same way). + * + *

Before this ticket the lambda was built inline in {@code main} and untestable: replacing it + * with {@code target -> null} — the exact shape of {@link ExhaustedPatternLookup#none()} — compiled + * with 0 errors and left every existing test green. Per the ticket, this is the worst consequence + * in the whole #589 sweep: a genuine usage-limit refusal would stop being classified as {@code + * BACKEND_EXHAUSTED} and would be handed back to a waiting {@code fleet_send} as if it were real + * completed work. + */ +class FleetdExhaustedPatternLookupWiringTest { + + private static final String YAML = """ + bind: + host: 127.0.0.1 + port: 8765 + herdrSocket: ~/.config/herdr/herdr.sock + profiles: + terra: + baseUrl: http://gx00.gw:8000 + model: claude-opus-5 + exhaustedPattern: "usage limit" + """; + + private static MemberSession session(String terminal, String profile) { + return new MemberSession("pane-" + terminal, terminal, profile, MemberRole.DEV, + "/cwd", null, 0L, 0L, 0, MemberSession.State.READY, null, null); + } + + private static LiveExhaustedPatterns liveExhaustedPatterns(Path dir) throws Exception { + Path file = dir.resolve("fleetd.yaml"); + Files.writeString(file, YAML); + FleetConfig cfg = FleetConfig.load(file); + return new LiveExhaustedPatterns(() -> ConfigRef.fixed(cfg).get().profiles()); + } + + @Test + @DisplayName("a known target resolves through its session's profile to that profile's live pattern") + void knownTargetResolvesThroughItsProfile(@TempDir Path dir) throws Exception { + LiveExhaustedPatterns patterns = liveExhaustedPatterns(dir); + ExhaustedPatternLookup lookup = Fleetd.exhaustedPatternLookup( + () -> List.of(session("term1", "terra")), patterns); + + Pattern resolved = lookup.patternFor("term1"); + + assertNotNull(resolved, + "the lookup must resolve term1 -> profile 'terra' -> LiveExhaustedPatterns.patternFor(" + + "'terra') — replacing the lambda body with 'target -> null' at the " + + "Fleetd.exhaustedPatternLookup call site must fail this assertion"); + assertTrue(resolved.matcher("the usage limit has been reached").find()); + } + + @Test + @DisplayName("an unknown target resolves to null, not a thrown exception") + void unknownTargetResolvesToNull(@TempDir Path dir) throws Exception { + LiveExhaustedPatterns patterns = liveExhaustedPatterns(dir); + ExhaustedPatternLookup lookup = Fleetd.exhaustedPatternLookup( + () -> List.of(session("term1", "terra")), patterns); + + assertNull(lookup.patternFor("term_stranger")); + } +} diff --git a/fleetd/src/test/java/dev/ltms/fleet/FleetdExhaustionSinkForwardingWiringTest.java b/fleetd/src/test/java/dev/ltms/fleet/FleetdExhaustionSinkForwardingWiringTest.java new file mode 100644 index 0000000..06f1583 --- /dev/null +++ b/fleetd/src/test/java/dev/ltms/fleet/FleetdExhaustionSinkForwardingWiringTest.java @@ -0,0 +1,62 @@ +package dev.ltms.fleet; + +import dev.ltms.fleet.inject.ExhaustionSink; +import org.junit.jupiter.api.DisplayName; +import org.junit.jupiter.api.Test; + +import java.util.concurrent.atomic.AtomicBoolean; +import java.util.concurrent.atomic.AtomicReference; + +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertTrue; + +/** + * fleetd #589 Group 1: {@link Fleetd#forwardingExhaustionSink} is the factory that replaced + * {@code main}'s inline {@code ExhaustionSink.forwardingTo(exhaustionSinkRef::get)} (fleetd #175's + * construction-order break: the adapters need a sink before {@code sessions} exists to build the + * real one). Before this ticket that call site was untestable wiring: replacing the supplier + * argument with a hardcoded {@code () -> ExhaustionSink.none()} compiled with 0 errors and left + * every existing test green, because no test builds the object {@code main} actually wires and + * then mutates the reference afterward — every existing {@code ExhaustionSink.forwardingTo} caller + * in this codebase reads and writes the SAME reference within one test, so a hardcoded-none supplier + * and a correctly-forwarding one are indistinguishable to them. + * + *

This test builds the reference, builds the forwarder from it, and only THEN repoints the + * reference at a spy sink — the discriminating order fleetd #175's whole design depends on + * ({@code exhaustionSinkRef} starts at {@code none()} and is repointed once {@code sessions} + * exists). A forwarder that captured a fixed target at construction time (the inert form) can never + * see that later repoint. + */ +class FleetdExhaustionSinkForwardingWiringTest { + + @Test + @DisplayName("the forwarder reads the reference live: repointing it AFTER construction is honoured") + void forwarderReadsTheReferenceLiveNotAFixedTargetCapturedAtConstruction() { + AtomicReference exhaustionSinkRef = new AtomicReference<>(ExhaustionSink.none()); + ExhaustionSink forwarder = Fleetd.forwardingExhaustionSink(exhaustionSinkRef); + + AtomicBoolean spyCalled = new AtomicBoolean(false); + exhaustionSinkRef.set((target, reason, profile) -> spyCalled.set(true)); + + forwarder.onExhausted("term_x", "usage limit reached", "terra"); + + assertTrue(spyCalled.get(), + "forwardingExhaustionSink must delegate to whatever exhaustionSinkRef currently " + + "holds — hardcoding the supplier to () -> ExhaustionSink.none() at the " + + "Fleetd.forwardingExhaustionSink call site must fail this assertion, " + + "since the spy set into the reference after construction would never run"); + } + + @Test + @DisplayName("before any repoint, the forwarder is inert — it starts at none(), not a crash") + void beforeAnyRepointTheForwarderIsInert() { + AtomicReference exhaustionSinkRef = new AtomicReference<>(ExhaustionSink.none()); + ExhaustionSink forwarder = Fleetd.forwardingExhaustionSink(exhaustionSinkRef); + + AtomicBoolean spyCalled = new AtomicBoolean(false); + forwarder.onExhausted("term_x", "usage limit reached", "terra"); + + assertFalse(spyCalled.get(), "nothing was ever wired to be called here — this only pins " + + "that the factory does not throw before a real sink is published"); + } +} diff --git a/fleetd/src/test/java/dev/ltms/fleet/FleetdExhaustionSinkPublishWiringTest.java b/fleetd/src/test/java/dev/ltms/fleet/FleetdExhaustionSinkPublishWiringTest.java new file mode 100644 index 0000000..31d6141 --- /dev/null +++ b/fleetd/src/test/java/dev/ltms/fleet/FleetdExhaustionSinkPublishWiringTest.java @@ -0,0 +1,110 @@ +package dev.ltms.fleet; + +import dev.ltms.fleet.config.ConfigRef; +import dev.ltms.fleet.config.FleetConfig; +import dev.ltms.fleet.guard.SubscriptionGuard; +import dev.ltms.fleet.herdr.AgentControl; +import dev.ltms.fleet.herdr.FakeHerdr; +import dev.ltms.fleet.herdr.WorkspaceControl; +import dev.ltms.fleet.inject.ExhaustionSink; +import dev.ltms.fleet.member.ClaudeCodeLauncher; +import dev.ltms.fleet.placement.BackendQuarantine; +import dev.ltms.fleet.session.SessionManager; +import org.junit.jupiter.api.DisplayName; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.io.TempDir; + +import java.nio.file.Files; +import java.nio.file.Path; +import java.util.HashMap; +import java.util.Map; +import java.util.Set; +import java.util.concurrent.TimeUnit; +import java.util.concurrent.atomic.AtomicReference; + +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertTrue; + +/** + * fleetd #589 Group 1: {@link Fleetd#publishExhaustionSink} is the factory that replaced {@code + * main}'s previously untested two-statement sequence — build the real {@link + * Fleetd#exhaustionSink}, then {@code exhaustionSinkRef.set(exhaustionSink)}. {@link + * Fleetd#exhaustionSink} itself is already pinned by {@code FleetdExhaustionSinkWarningTest} (its + * log text) — what was NEVER pinned is the {@code .set(...)} call: {@code main} could replace it + * with {@code exhaustionSinkRef.set(ExhaustionSink.none())} and compile with 0 errors, leaving + * every existing test green, because {@link Fleetd#exhaustionSink}'s own tests build and call the + * sink directly, never through the reference {@code main} publishes it into. + * + *

This test proves the PUBLISHED reference — not a freshly rebuilt sink — is the one that + * actually quarantines a credential, by reading {@link BackendQuarantine#isQuarantined} after + * calling {@code exhaustionSinkRef.get().onExhausted(...)}, the same object {@link + * Fleetd#forwardingExhaustionSink} forwards to in production. + */ +class FleetdExhaustionSinkPublishWiringTest { + + private static final String YAML = """ + bind: + host: 127.0.0.1 + port: 8765 + herdrSocket: ~/.config/herdr/herdr.sock + profiles: + terra: + baseUrl: http://gx00.gw:8000 + model: claude-opus-5 + guard: + offSubscriptionHosts: + - gx00.gw + """; + + private static SessionManager emptyRosterSessions() { + FakeHerdr h = new FakeHerdr(); + FleetConfig.Profile dummy = new FleetConfig.Profile( + "dummy", "http://gx00.gw:8000", "coder", null, "FLEETD_WORKER_TOKEN", null, + "tab", "fleetd-workers", "worker: {profile} #{n}", null, null, null); + ClaudeCodeLauncher launcher = new ClaudeCodeLauncher(new AgentControl(h), new WorkspaceControl(h), + new SubscriptionGuard(Set.of("gx00.gw")), Map.of(dummy.profile(), dummy), dummy.profile(), _ -> "tok"); + // Never acquires a session — publishExhaustionSink's built sink resolves target -> profile + // via the profileHint fallback (fleetd #234), exactly like OpenCodeLauncher's real call + // site does, so this never needs a populated roster. + return new SessionManager(launcher); + } + + @Test + @DisplayName("the published reference actually quarantines — not a rebuilt-but-never-set sink") + void publishedReferenceActuallyQuarantines(@TempDir Path dir) throws Exception { + Path file = dir.resolve("fleetd.yaml"); + Files.writeString(file, YAML); + FleetConfig cfg = FleetConfig.load(file); + ConfigRef config = ConfigRef.fixed(cfg); + BackendQuarantine quarantine = new BackendQuarantine(() -> 0L, TimeUnit.MINUTES.toNanos(30)); + Map reasonByCredential = new HashMap<>(); + AtomicReference exhaustionSinkRef = new AtomicReference<>(ExhaustionSink.none()); + + Fleetd.publishExhaustionSink(exhaustionSinkRef, emptyRosterSessions(), config, quarantine, + reasonByCredential, cfg); + exhaustionSinkRef.get().onExhausted("term_x", "The usage limit has been reached", "terra"); + + assertTrue(quarantine.isQuarantined("terra"), + "publishExhaustionSink must repoint exhaustionSinkRef at the REAL sink — " + + "replacing the .set(...) call with exhaustionSinkRef.set(ExhaustionSink.none()) " + + "at the Fleetd.publishExhaustionSink call site must fail this assertion, " + + "since none()'s onExhausted does nothing"); + } + + @Test + @DisplayName("before publishing, the reference is still inert — no quarantine, no crash") + void beforePublishingTheReferenceIsStillInert(@TempDir Path dir) throws Exception { + Path file = dir.resolve("fleetd.yaml"); + Files.writeString(file, YAML); + FleetConfig cfg = FleetConfig.load(file); + ConfigRef config = ConfigRef.fixed(cfg); + BackendQuarantine quarantine = new BackendQuarantine(() -> 0L, TimeUnit.MINUTES.toNanos(30)); + AtomicReference exhaustionSinkRef = new AtomicReference<>(ExhaustionSink.none()); + + exhaustionSinkRef.get().onExhausted("term_x", "The usage limit has been reached", "terra"); + + assertFalse(quarantine.isQuarantined("terra"), + "nothing was published yet — this only pins the starting state the other test's " + + "assertion actually distinguishes from"); + } +} diff --git a/fleetd/src/test/java/dev/ltms/fleet/FleetdLiveExhaustedPatternsWiringTest.java b/fleetd/src/test/java/dev/ltms/fleet/FleetdLiveExhaustedPatternsWiringTest.java new file mode 100644 index 0000000..5b2f8de --- /dev/null +++ b/fleetd/src/test/java/dev/ltms/fleet/FleetdLiveExhaustedPatternsWiringTest.java @@ -0,0 +1,72 @@ +package dev.ltms.fleet; + +import dev.ltms.fleet.config.ConfigRef; +import dev.ltms.fleet.config.FleetConfig; +import dev.ltms.fleet.inject.LiveExhaustedPatterns; +import org.junit.jupiter.api.DisplayName; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.io.TempDir; + +import java.nio.file.Files; +import java.nio.file.Path; + +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertNull; +import static org.junit.jupiter.api.Assertions.assertTrue; + +/** + * fleetd #589 Group 1: {@link Fleetd#liveExhaustedPatterns} is the factory that replaced {@code + * main}'s inline {@code new LiveExhaustedPatterns(() -> config.get().profiles())}. Before this + * ticket, that supplier argument was untestable wiring: replacing it with a hardcoded {@code () -> + * Map.of()} compiled with 0 errors and left every existing test green, because {@code + * LiveExhaustedPatternsTest} builds its own instance directly with a hand-supplied map and never + * goes through {@code main}'s call site. + * + *

Silently losing this wiring means every profile's {@code exhaustedPattern} stops being + * recognised — {@link Fleetd#exhaustedPatternLookup} would never see a match, and a genuine + * usage-limit refusal would be handed back to a waiting {@code fleet_send} as real completed work. + */ +class FleetdLiveExhaustedPatternsWiringTest { + + private static final String YAML = """ + bind: + host: 127.0.0.1 + port: 8765 + herdrSocket: ~/.config/herdr/herdr.sock + profiles: + terra: + baseUrl: http://gx00.gw:8000 + model: claude-opus-5 + exhaustedPattern: "usage limit" + gx: + baseUrl: http://gx00.gw:8000 + """; + + private static ConfigRef loadConfig(Path dir) throws Exception { + Path file = dir.resolve("fleetd.yaml"); + Files.writeString(file, YAML); + FleetConfig cfg = FleetConfig.load(file); + return ConfigRef.fixed(cfg); + } + + @Test + @DisplayName("a profile with a configured exhaustedPattern is armed, with a compiled matcher") + void configuredProfileIsArmed(@TempDir Path dir) throws Exception { + LiveExhaustedPatterns patterns = Fleetd.liveExhaustedPatterns(loadConfig(dir)); + + assertTrue(patterns.armed("terra"), + "the config's live profiles() supplier must reach LiveExhaustedPatterns — hardcoding " + + "the supplier to () -> Map.of() at the Fleetd.liveExhaustedPatterns call " + + "site must fail this assertion"); + assertTrue(patterns.patternFor("terra").matcher("the usage limit has been reached").find()); + } + + @Test + @DisplayName("a profile with no configured exhaustedPattern is not armed, but is still resolvable") + void unconfiguredProfileIsNotArmed(@TempDir Path dir) throws Exception { + LiveExhaustedPatterns patterns = Fleetd.liveExhaustedPatterns(loadConfig(dir)); + + assertFalse(patterns.armed("gx"), "'gx' has no exhaustedPattern configured"); + assertNull(patterns.patternFor("gx")); + } +} diff --git a/fleetd/src/test/java/dev/ltms/fleet/FleetdOpenCodeLauncherCredentialWiringTest.java b/fleetd/src/test/java/dev/ltms/fleet/FleetdOpenCodeLauncherCredentialWiringTest.java new file mode 100644 index 0000000..1ce0fe3 --- /dev/null +++ b/fleetd/src/test/java/dev/ltms/fleet/FleetdOpenCodeLauncherCredentialWiringTest.java @@ -0,0 +1,77 @@ +package dev.ltms.fleet; + +import dev.ltms.fleet.config.ConfigRef; +import dev.ltms.fleet.config.FleetConfig; +import dev.ltms.fleet.herdr.AgentControl; +import dev.ltms.fleet.herdr.FakeHerdr; +import dev.ltms.fleet.herdr.WorkspaceControl; +import dev.ltms.fleet.inject.ExhaustionSink; +import dev.ltms.fleet.member.OpenCodeLauncher; +import org.junit.jupiter.api.DisplayName; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.io.TempDir; + +import java.nio.file.Files; +import java.nio.file.Path; +import java.util.Map; + +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertNotNull; + +/** + * fleetd #589 Group 2: {@link Fleetd#openCodeLauncher} is the factory that replaced {@code main}'s + * inline {@code new OpenCodeLauncher(...)} call — the {@code opencode} counterpart to {@link + * Fleetd#claudeCodeLauncher}, extracted for the identical reason. Its {@code memberCredentials} + * argument is the same {@code () -> config.get().memberCredentials()} supplier; replacing it with + * {@code () -> null} compiled with 0 errors and left every existing test green before this ticket, + * reopening the same CB-592 exposure gap CB-596's policy closed. + * + *

Same observable surface as {@code OpenCodeLauncherTest}'s own {@code memberCredentials} tests: + * spawn through the launcher {@code main} actually wires and inspect what {@code tab.create} + * carried. + */ +class FleetdOpenCodeLauncherCredentialWiringTest { + + private static final String YAML = """ + bind: + host: 127.0.0.1 + port: 8765 + herdrSocket: ~/.config/herdr/herdr.sock + profiles: + gemini: + kind: opencode + model: google/gemini-2.5-pro + memberCredentials: + policy: deny-by-default + known: + - GITEA_ACCESS_TOKEN + """; + + @SuppressWarnings("unchecked") + private static Map startEnv(FakeHerdr herdr) { + return (Map) ((Map) herdr.lastCall("tab.create").params()).get("env"); + } + + @Test + @DisplayName("main's memberCredentials wiring reaches OpenCodeLauncher: a known-but-not-allowed " + + "name is shadowed on spawn") + void memberCredentialsWiringReachesOpenCodeLauncher(@TempDir Path dir) throws Exception { + Path file = dir.resolve("fleetd.yaml"); + Files.writeString(file, YAML); + FleetConfig cfg = FleetConfig.load(file); + ConfigRef config = ConfigRef.fixed(cfg); + FakeHerdr herdr = new FakeHerdr(); + + OpenCodeLauncher launcher = Fleetd.openCodeLauncher(new AgentControl(herdr), + new WorkspaceControl(herdr), cfg.profiles(), cfg, config, ExhaustionSink.none()); + launcher.spawn(); + + String shadowed = startEnv(herdr).get("GITEA_ACCESS_TOKEN"); + assertNotNull(shadowed, + "GITEA_ACCESS_TOKEN is 'known' but not 'allow'-ed in the loaded config — it must be " + + "explicitly shadowed on spawn; replacing the memberCredentials supplier with " + + "() -> null at the Fleetd.openCodeLauncher call site must fail this " + + "assertion, since a null policy shadows nothing"); + assertFalse(shadowed.isBlank(), "the overlay value must be non-blank"); + } +}