From d895f02bc1e467b9b72248d83f25c21ebdd22ce0 Mon Sep 17 00:00:00 2001 From: Dai Ha Date: Thu, 3 Sep 2026 12:20:28 +0700 Subject: [PATCH] fleetd #248: prove main() wires CompletionResolver's arguments, not just the class MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fleetd.main built three of CompletionResolver's 8 constructor arguments inline (a worktree/branch lookup lambda, and the backend-error pattern lookup + sink locals). Dropping any of them at the call site compiled clean and left every existing test green, because every existing test constructs its own CompletionResolver and only ever proves the class, never main's wiring. Extract each into a static factory on Fleetd (worktreeBranchLookup, backendErrorPatternLookup, backendErrorSink — the same static-factory pattern Fleetd.deliverableTo already uses), test each factory's own behaviour, and add a source-text assertion (FleetdCompletionResolverWiringTest) proving main's CompletionResolver call still passes all three. backendErrorSink is public so BackendOutageFlowTest can exercise the real production sink directly instead of the hand-mirrored copy its own class doc used to describe. No production behaviour changes — mechanical extraction only. --- .../src/main/java/dev/ltms/fleet/Fleetd.java | 187 ++++++++++---- .../FleetdBackendErrorPatternLookupTest.java | 59 +++++ .../fleet/FleetdBackendErrorSinkTest.java | 229 ++++++++++++++++++ .../FleetdCompletionResolverWiringTest.java | 89 +++++++ .../fleet/FleetdWorktreeBranchLookupTest.java | 64 +++++ 5 files changed, 576 insertions(+), 52 deletions(-) create mode 100644 fleetd/src/test/java/dev/ltms/fleet/FleetdBackendErrorPatternLookupTest.java create mode 100644 fleetd/src/test/java/dev/ltms/fleet/FleetdBackendErrorSinkTest.java create mode 100644 fleetd/src/test/java/dev/ltms/fleet/FleetdCompletionResolverWiringTest.java create mode 100644 fleetd/src/test/java/dev/ltms/fleet/FleetdWorktreeBranchLookupTest.java diff --git a/fleetd/src/main/java/dev/ltms/fleet/Fleetd.java b/fleetd/src/main/java/dev/ltms/fleet/Fleetd.java index 0e93f2a..bf9a5d0 100644 --- a/fleetd/src/main/java/dev/ltms/fleet/Fleetd.java +++ b/fleetd/src/main/java/dev/ltms/fleet/Fleetd.java @@ -365,11 +365,11 @@ public final class Fleetd { errorPatternsByProfile.put(name, Pattern.compile(profile.errorPattern())); } }); - BackendErrorPatternLookup backendErrorPatterns = target -> sessions.roster().stream() - .filter(session -> target.equals(session.terminalId())) - .findFirst() - .map(session -> errorPatternsByProfile.get(session.profile())) - .orElse(null); + // fleetd #248: extracted to a static factory (see backendErrorPatternLookup below) so a + // test can prove main() actually PASSES this into CompletionResolver, not only that the + // lookup itself behaves correctly — the exact gap fleetd #248 exists to close. + BackendErrorPatternLookup backendErrorPatterns = backendErrorPatternLookup(sessions::roster, + errorPatternsByProfile); log.info("backend-error classification (fleetd #201 Unit 5): {}", CompletionResolver.coverage(cfg.profiles().keySet(), errorPatternsByProfile.keySet())); // CB-578 stage B: on a classification that actually wins, quarantine the exhausted profile's @@ -424,59 +424,25 @@ public final class Fleetd { // construction-order cycle `exhaustionSinkRef` breaks above, broken the same way: a mutable // holder set once `pushLoop` exists, read lazily from inside the lambda built here. AtomicReference pushLoopRef = new AtomicReference<>(); - // Order: (1) mark the member BACKEND_ERROR; (2) resolve profile/credential through the - // roster — fail loud (never Optional.ifPresent, the fleetd #234 lesson applied to this new - // sink) and notify the lead via onBackendTargetUnmapped when it cannot be resolved; (3) - // record the error in BackendOutagePolicy; (4) on a NEW incident (the record() call that - // actually crosses the threshold), tell the lead via onBackendIncident. - BackendErrorSink backendErrorSink = (target, matchedLine, reason) -> { - sessions.onBackendError(target, reason); - - String profileName = sessions.roster().stream() - .filter(session -> target.equals(session.terminalId())) - .findFirst() - .map(MemberSession::profile) - .orElse(null); - FleetConfig.Profile profile = profileName == null ? null : config.get().profiles().get(profileName); - if (profile == null) { - log.error("backend error on target '{}' ({}) but no profile could be resolved — the " - + "target is not (yet) in the roster — no cool-off applied (fleetd #201 Unit 5)", - target, reason); - ReplyPushLoop loop = pushLoopRef.get(); - if (loop != null) { - loop.onBackendTargetUnmapped(target, reason); - } - return; - } - String credentialId = profile.effectiveCredentialId(); - Optional incident = outagePolicy.record(credentialId, target, reason); - incident.ifPresent(inc -> { - List affectedProfiles = config.get().profiles().values().stream() - .filter(p -> credentialId.equals(p.effectiveCredentialId())) - .map(FleetConfig.Profile::profile) - .sorted() - .toList(); - log.warn("credential '{}' cooling off for {}s after backend errors on {} distinct " - + "target(s) (profile '{}'): {}", credentialId, - inc.remainingCoolOffSeconds(), inc.evidenceCount(), profile.profile(), reason); - ReplyPushLoop loop = pushLoopRef.get(); - if (loop != null) { - loop.onBackendIncident(inc.id(), inc.targets(), credentialId, affectedProfiles, - (int) inc.remainingCoolOffSeconds()); - } - }); - }; + // fleetd #248: extracted to a static factory (see backendErrorSink below), public rather + // than package-private like the other two factories here, so + // dev.ltms.fleet.inject.BackendOutageFlowTest can exercise the REAL production sink + // directly instead of a hand-mirrored copy of this lambda — that copy was precisely the + // gap fleetd #248 exists to close (see that test's class doc for the history). + BackendErrorSink backendErrorSink = backendErrorSink(sessions, () -> config.get().profiles(), + outagePolicy, pushLoopRef::get); AgentControl agents = router.memberAgents(); // Both fleetd#201 Unit 5 (backend-error patterns + sink) and fleetd#241 (the worktree/branch // lookup the fallback report names) land on this one call. The full constructor takes both, // so neither feature is dropped; nowNanos must be passed explicitly to reach it. + // + // fleetd #248: every argument built specifically for this call (backendErrorPatterns and + // backendErrorSink above, and the worktree/branch lookup right here) now comes from a + // static factory tested on its own; FleetdCompletionResolverWiringTest source-asserts that + // THIS call actually passes them, which is the coverage that was missing before. CompletionResolver completion = new CompletionResolver(agents, rendezvous, exhaustedPatterns, exhaustionSink, backendErrorPatterns, backendErrorSink, System::nanoTime, - target -> sessions.roster().stream() - .filter(session -> target.equals(session.terminalId())) - .findFirst() - .map(session -> new CompletionResolver.WorktreeBranch(session.worktree(), session.branch())) - .orElse(null)); + worktreeBranchLookup(sessions::roster)); // CB-113: deliver only to an available worker (its MCP is connected), never its boot window. // CB-301: the manager's presence bridge records availability and drives SPAWNING → READY. MemberPresence presence = sessions.asPresence(); @@ -770,6 +736,123 @@ public final class Fleetd { return target -> presence.isPresent(target) || leads.get().containsKey(target); } + /** + * fleetd #248: package-private factory for the member worktree/branch lookup {@link + * CompletionResolver} uses to name a fallback report's worktree and branch (fleetd#241). + * + *

Before this ticket the lookup was an anonymous lambda built inline inside {@code main}'s + * {@code CompletionResolver} constructor call — provably untested wiring, the whole reason + * fleetd #248 exists: dropping that one argument (passing {@code _ -> null} instead) compiled + * clean and left every test green. Extracted here, {@code main} now calls this factory instead + * of building the lambda inline, and a source assertion on that call site + * ({@code FleetdCompletionResolverWiringTest}) proves the argument is still actually passed. + * + *

Takes the roster as a plain {@link Supplier} — not a {@link SessionManager} — so this is + * directly testable with a hand-built session list; no real {@code SessionManager} (launcher, + * worktrees, …) needs constructing. Follows the same {@code static} factory pattern as + * {@link #deliverableTo} above. + * + * @param roster the live member roster, normally {@code sessions::roster} + */ + static Function worktreeBranchLookup( + Supplier> roster) { + return target -> roster.get().stream() + .filter(session -> target.equals(session.terminalId())) + .findFirst() + .map(session -> new CompletionResolver.WorktreeBranch(session.worktree(), session.branch())) + .orElse(null); + } + + /** + * fleetd #248 / fleetd#201 Unit 5: package-private factory for the per-target backend-error + * pattern lookup {@link CompletionResolver} classifies a pane scrape against. Closes over the + * live roster (to resolve a target to a profile) and {@code errorPatternsByProfile} (each + * profile's configured {@code errorPattern}, already compiled by the caller — the same map also + * feeds the coverage log next to where this is called) — nothing else, so it is directly + * testable. See {@link #worktreeBranchLookup} above for why this ticket exists and why the + * factory takes a roster {@link Supplier} rather than a {@link SessionManager}. + * + * @param roster the live member roster, normally {@code sessions::roster} + * @param errorPatternsByProfile every profile that has an {@code errorPattern} configured, + * keyed by profile name + */ + static BackendErrorPatternLookup backendErrorPatternLookup(Supplier> roster, + Map errorPatternsByProfile) { + return target -> roster.get().stream() + .filter(session -> target.equals(session.terminalId())) + .findFirst() + .map(session -> errorPatternsByProfile.get(session.profile())) + .orElse(null); + } + + /** + * fleetd #248 / fleetd#201 Unit 5: factory for the production {@link BackendErrorSink} — the + * collaborator {@link CompletionResolver} notifies when a pane-scrape classification actually + * resolves a waiter as a backend error. Order: (1) mark the member BACKEND_ERROR; (2) resolve + * profile/credential through the roster — fail loud (never {@code Optional.ifPresent}, the + * fleetd #234 lesson applied to this sink) and notify the lead via {@code + * onBackendTargetUnmapped} when it cannot be resolved; (3) record the error in {@code + * outagePolicy}; (4) on a NEW incident (the {@code record()} call that actually crosses the + * threshold), tell the lead via {@code onBackendIncident}. + * + *

{@code public}, unlike {@link #worktreeBranchLookup} and {@link #backendErrorPatternLookup} + * above: {@code dev.ltms.fleet.inject.BackendOutageFlowTest} exercises this exact object as the + * real, wired production path, replacing what its own class doc used to call out as a + * hand-mirrored copy of this lambda ("mirrors {@code Fleetd.main}'s {@code backendErrorSink} + * lambda line-for-line") — that copy proved only itself, never that {@code main} still wires the + * real thing. That was precisely the gap fleetd #248 exists to close. + * + * @param sessions the session registry; both read (roster) and written (onBackendError) + * @param profiles the live profile map, normally {@code () -> config.get().profiles()} in + * {@code main}, or a fixed test map via {@code () -> profiles} + * @param pushLoop the lead-nudge loop, read lazily: {@code main} builds this sink before the + * real {@link ReplyPushLoop} exists (a genuine construction-order cycle, broken + * the same way {@code exhaustionSinkRef} is a few lines above it), so a + * {@link Supplier} reads whatever {@code main} has filled in by the time a real + * backend error fires + */ + public static BackendErrorSink backendErrorSink(SessionManager sessions, + Supplier> profiles, BackendOutagePolicy outagePolicy, + Supplier pushLoop) { + return (target, matchedLine, reason) -> { + sessions.onBackendError(target, reason); + + String profileName = sessions.roster().stream() + .filter(session -> target.equals(session.terminalId())) + .findFirst() + .map(MemberSession::profile) + .orElse(null); + FleetConfig.Profile profile = profileName == null ? null : profiles.get().get(profileName); + if (profile == null) { + log.error("backend error on target '{}' ({}) but no profile could be resolved — the " + + "target is not (yet) in the roster — no cool-off applied (fleetd #201 Unit 5)", + target, reason); + ReplyPushLoop loop = pushLoop.get(); + if (loop != null) { + loop.onBackendTargetUnmapped(target, reason); + } + return; + } + String credentialId = profile.effectiveCredentialId(); + Optional incident = outagePolicy.record(credentialId, target, reason); + incident.ifPresent(inc -> { + List affectedProfiles = profiles.get().values().stream() + .filter(p -> credentialId.equals(p.effectiveCredentialId())) + .map(FleetConfig.Profile::profile) + .sorted() + .toList(); + log.warn("credential '{}' cooling off for {}s after backend errors on {} distinct " + + "target(s) (profile '{}'): {}", credentialId, + inc.remainingCoolOffSeconds(), inc.evidenceCount(), profile.profile(), reason); + ReplyPushLoop loop = pushLoop.get(); + if (loop != null) { + loop.onBackendIncident(inc.id(), inc.targets(), credentialId, affectedProfiles, + (int) inc.remainingCoolOffSeconds()); + } + }); + }; + } + /** Injection seam for {@link #selectReplyInbox}: production binds {@link AmqpReplyInbox#open}. */ @FunctionalInterface interface AmqpOpener { diff --git a/fleetd/src/test/java/dev/ltms/fleet/FleetdBackendErrorPatternLookupTest.java b/fleetd/src/test/java/dev/ltms/fleet/FleetdBackendErrorPatternLookupTest.java new file mode 100644 index 0000000..666fbd7 --- /dev/null +++ b/fleetd/src/test/java/dev/ltms/fleet/FleetdBackendErrorPatternLookupTest.java @@ -0,0 +1,59 @@ +package dev.ltms.fleet; + +import dev.ltms.fleet.inject.BackendErrorPatternLookup; +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 java.util.List; +import java.util.Map; +import java.util.regex.Pattern; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNull; + +/** + * fleetd #248 / fleetd#201 Unit 5: {@link Fleetd#backendErrorPatternLookup} is the factory that + * replaced the local lambda {@code Fleetd.main} used to build {@code backendErrorPatterns} — one + * of the two arguments {@code CompletionResolver} lost cleanly (0 compile errors, every test still + * green) when this ticket's measurement dropped it alongside {@code backendErrorSink}. This class + * proves the factory's own behaviour; {@code FleetdCompletionResolverWiringTest} proves {@code + * main} still passes its result into {@code CompletionResolver}. + */ +class FleetdBackendErrorPatternLookupTest { + + 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); + } + + @Test + @DisplayName("a target on a profile with a configured pattern resolves to that pattern") + void configuredProfileResolves() { + Map byProfile = Map.of("terra", Pattern.compile("(?i)503")); + BackendErrorPatternLookup lookup = + Fleetd.backendErrorPatternLookup(() -> List.of(session("term1", "terra")), byProfile); + + assertEquals("(?i)503", lookup.patternFor("term1").pattern()); + } + + @Test + @DisplayName("a target on a profile with no configured pattern resolves to null") + void unconfiguredProfileResolvesToNull() { + Map byProfile = Map.of("terra", Pattern.compile("x")); + BackendErrorPatternLookup lookup = + Fleetd.backendErrorPatternLookup(() -> List.of(session("term1", "sol")), byProfile); + + assertNull(lookup.patternFor("term1")); + } + + @Test + @DisplayName("an unknown target resolves to null") + void unknownTargetResolvesToNull() { + BackendErrorPatternLookup lookup = + Fleetd.backendErrorPatternLookup(List::of, Map.of("terra", Pattern.compile("x"))); + + assertNull(lookup.patternFor("term_stranger")); + } +} diff --git a/fleetd/src/test/java/dev/ltms/fleet/FleetdBackendErrorSinkTest.java b/fleetd/src/test/java/dev/ltms/fleet/FleetdBackendErrorSinkTest.java new file mode 100644 index 0000000..d1c5086 --- /dev/null +++ b/fleetd/src/test/java/dev/ltms/fleet/FleetdBackendErrorSinkTest.java @@ -0,0 +1,229 @@ +package dev.ltms.fleet; + +import com.fasterxml.jackson.databind.JsonNode; +import com.fasterxml.jackson.databind.ObjectMapper; +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.HerdrClient; +import dev.ltms.fleet.herdr.WorkspaceControl; +import dev.ltms.fleet.inject.BackendErrorSink; +import dev.ltms.fleet.member.ClaudeCodeLauncher; +import dev.ltms.fleet.member.CompositePeerLauncher; +import dev.ltms.fleet.mcp.PrimaryRegistry; +import dev.ltms.fleet.msg.InMemoryReplyInbox; +import dev.ltms.fleet.msg.ReplyPushLoop; +import dev.ltms.fleet.peer.Capability; +import dev.ltms.fleet.peer.PeerHandle; +import dev.ltms.fleet.peer.PeerLauncher; +import dev.ltms.fleet.peer.SpawnRequest; +import dev.ltms.fleet.placement.BackendOutagePolicy; +import dev.ltms.fleet.placement.BackendQuarantine; +import dev.ltms.fleet.placement.PlacementPolicies; +import dev.ltms.fleet.session.MemberSession; +import dev.ltms.fleet.session.SessionManager; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.DisplayName; +import org.junit.jupiter.api.Test; + +import java.util.ArrayList; +import java.util.LinkedHashMap; +import java.util.List; +import java.util.Map; +import java.util.Set; +import java.util.concurrent.CopyOnWriteArrayList; +import java.util.concurrent.CountDownLatch; +import java.util.concurrent.Executors; +import java.util.concurrent.ScheduledExecutorService; +import java.util.concurrent.TimeUnit; +import java.util.concurrent.atomic.AtomicLong; +import java.util.concurrent.atomic.AtomicReference; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertTrue; + +/** + * fleetd #248 / fleetd#201 Unit 5: {@link Fleetd#backendErrorSink} is the factory that replaced + * the local lambda {@code Fleetd.main} used to build {@code backendErrorSink} — the other half of + * the pair this ticket's measurement dropped cleanly (0 compile errors, every test still green). + * + *

Before this ticket, the closest thing to coverage was {@code + * dev.ltms.fleet.inject.BackendOutageFlowTest}, whose own class doc said it "mirrors {@code + * Fleetd.main}'s {@code backendErrorSink} lambda line-for-line" — a hand-copy that proves itself, + * never that {@code main} still wires the real thing. This class exercises the actual production + * factory instead. {@code FleetdCompletionResolverWiringTest} proves {@code main} still passes its + * result into {@code CompletionResolver}. + */ +class FleetdBackendErrorSinkTest { + + private final List schedulers = new ArrayList<>(); + + @AfterEach + void tearDown() { + schedulers.forEach(ScheduledExecutorService::shutdownNow); + } + + private static FleetConfig.Profile stubWorker(String profile, String credentialId) { + return new FleetConfig.Profile(profile, "http://gx00.gw:8000", "coder", + null, "FLEETD_WORKER_TOKEN", List.of("claude"), "tab", "fleetd-workers", + "w #{n}", null, null, null, null, null, null, null, null, null, + null, null, credentialId, null); + } + + private static Map orderedProfiles() { + Map m = new LinkedHashMap<>(); + m.put("terra", stubWorker("terra", "shared-openai")); + m.put("sol", stubWorker("sol", "shared-openai")); + return m; + } + + /** Minimal recording {@code HerdrClient} for the LEAD pane — mirrors ReplyPushLoopTest's own. */ + private static final class RecordingLeadClient implements HerdrClient { + private static final ObjectMapper MAPPER = new ObjectMapper(); + private final List prompts = new CopyOnWriteArrayList<>(); + volatile CountDownLatch sendLatch = new CountDownLatch(1); + + @Override + public JsonNode call(String method, Object params) { + if ("agent.get".equals(method)) { + return MAPPER.createObjectNode().set("agent", MAPPER.createObjectNode() + .put("terminal_id", "term_primary").put("agent_status", "idle")); + } + if ("agent.prompt".equals(method)) { + prompts.add(params); + sendLatch.countDown(); + } + return MAPPER.createObjectNode(); + } + + @Override + public void close() { + } + + int sendCount() { + return prompts.size(); + } + } + + /** A {@link PeerLauncher} that never actually spawns — enough to construct a bare {@link SessionManager}. */ + private static final class NeverSpawnsLauncher implements PeerLauncher { + @Override + public Set capabilities() { + return Set.of(); + } + + @Override + public Set capabilitiesFor(String profileName) { + return Set.of(); + } + + @Override + public PeerHandle spawn(SpawnRequest req) { + throw new UnsupportedOperationException("not reachable — this test never acquires a session"); + } + + @Override + public Set profiles() { + return Set.of(); + } + + @Override + public String defaultProfile() { + return null; + } + + @Override + public String effectiveCwd(SpawnRequest req) { + throw new UnsupportedOperationException("not reachable — this test never acquires a session"); + } + + @Override + public List parityOverlay(String profileName) { + return List.of(); + } + + @Override + public List list() { + return List.of(); + } + + @Override + public int reapOrphanWorkers() { + return 0; + } + + @Override + public void stop(String id) { + } + + @Override + public boolean clearContext(String id) { + return false; + } + } + + @Test + @DisplayName("a target with no resolvable profile logs and returns without recording an incident (never throws)") + void unresolvableProfileDoesNotRecordOrThrow() { + SessionManager sessions = new SessionManager(new NeverSpawnsLauncher()); + BackendOutagePolicy outagePolicy = new BackendOutagePolicy(() -> 0L); + RecordingLeadClient leadClient = new RecordingLeadClient(); + InMemoryReplyInbox inbox = new InMemoryReplyInbox(); + PrimaryRegistry registry = new PrimaryRegistry(null); + ScheduledExecutorService scheduler = Executors.newSingleThreadScheduledExecutor(); + schedulers.add(scheduler); + ReplyPushLoop pushLoop = new ReplyPushLoop(registry, new AgentControl(leadClient), inbox, scheduler, 3, 50); + + BackendErrorSink sink = Fleetd.backendErrorSink(sessions, Map::of, outagePolicy, () -> pushLoop); + sink.onBackendError("term_unmapped", "matched line", "503 Service Unavailable"); + + assertTrue(outagePolicy.remainingCoolOffSeconds("shared-openai").isEmpty(), + "no credential is ever resolvable here, so nothing must be recorded"); + } + + @Test + @DisplayName("two distinct targets classified through the real factory start an incident and cool the credential") + void twoDistinctTargetsStartAnIncident() throws Exception { + FakeHerdr herdr = new FakeHerdr() + .readText("⏺ 503 Service Unavailable: upstream credential rejected\n❯ "); + Map profiles = orderedProfiles(); + AtomicLong clockNanos = new AtomicLong(0L); + BackendOutagePolicy outagePolicy = new BackendOutagePolicy(clockNanos::get); + ClaudeCodeLauncher adapter = new ClaudeCodeLauncher(new AgentControl(herdr), + new WorkspaceControl(herdr), new SubscriptionGuard(Set.of("gx00.gw")), + profiles, "terra", _ -> "tok"); + CompositePeerLauncher workers = new CompositePeerLauncher(List.of(adapter), "terra", profiles, + PlacementPolicies.weighted(), _ -> 0, null, BackendQuarantine.none(), outagePolicy); + SessionManager sessions = new SessionManager(workers); + MemberSession s1 = sessions.acquire("terra", null, null, null); + MemberSession s2 = sessions.acquire("terra", null, null, null); + + PrimaryRegistry registry = new PrimaryRegistry(null); + registry.recordDelegation(s1.terminalId(), "term_primary"); + registry.recordDelegation(s2.terminalId(), "term_primary"); + InMemoryReplyInbox inbox = new InMemoryReplyInbox(); + inbox.own(s1.terminalId()); + inbox.own(s2.terminalId()); + RecordingLeadClient leadClient = new RecordingLeadClient(); + ScheduledExecutorService scheduler = Executors.newSingleThreadScheduledExecutor(); + schedulers.add(scheduler); + ReplyPushLoop pushLoop = new ReplyPushLoop(registry, new AgentControl(leadClient), inbox, scheduler, 3, 50); + AtomicReference pushLoopRef = new AtomicReference<>(pushLoop); + + // The exact object under test: Fleetd's real production factory, not a hand copy. + BackendErrorSink sink = Fleetd.backendErrorSink(sessions, () -> profiles, outagePolicy, pushLoopRef::get); + + sink.onBackendError(s1.terminalId(), "matched line", "503 Service Unavailable"); + assertTrue(outagePolicy.remainingCoolOffSeconds("shared-openai").isEmpty(), + "one distinct target must not start a cool-off"); + + sink.onBackendError(s2.terminalId(), "matched line", "503 Service Unavailable"); + + assertTrue(leadClient.sendLatch.await(3, TimeUnit.SECONDS), + "the second distinct target must cross the threshold and nudge the lead"); + var remaining = outagePolicy.remainingCoolOffSeconds("shared-openai"); + assertTrue(remaining.isPresent(), "two distinct targets must start a cool-off"); + assertEquals(1, leadClient.sendCount()); + } +} diff --git a/fleetd/src/test/java/dev/ltms/fleet/FleetdCompletionResolverWiringTest.java b/fleetd/src/test/java/dev/ltms/fleet/FleetdCompletionResolverWiringTest.java new file mode 100644 index 0000000..f8a1386 --- /dev/null +++ b/fleetd/src/test/java/dev/ltms/fleet/FleetdCompletionResolverWiringTest.java @@ -0,0 +1,89 @@ +package dev.ltms.fleet; + +import java.nio.file.Files; +import java.nio.file.Path; +import org.junit.jupiter.api.DisplayName; +import org.junit.jupiter.api.Test; + +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertTrue; + +/** + * fleetd #248: this is the test that was actually missing. {@code Fleetd.main} builds its {@code + * CompletionResolver} from an 8-argument constructor, and the ticket's own measurement proved two + * ways to silently unwire it — both compiled with 0 errors and left every existing test green: + * + *
    + *
  • replacing the worktree/branch argument (the 8th) with {@code _ -> null} — drops + * fleetd#241's fallback-report location entirely;
    • + *
  • replacing {@code backendErrorPatterns, backendErrorSink} (5th/6th) with {@code + * BackendErrorPatternLookup.legacy(), BackendErrorSink.none()} — drops fleetd#201 Unit 5's + * backend-error classification and cool-off entirely.
    • + *
+ * + *

Neither mutation could be caught by any test that constructs its own {@code + * CompletionResolver} (every test before this one did exactly that) or by a test of {@link + * Fleetd#worktreeBranchLookup}, {@link Fleetd#backendErrorPatternLookup}, or {@link + * Fleetd#backendErrorSink} in isolation (see {@code FleetdWorktreeBranchLookupTest}, {@code + * FleetdBackendErrorPatternLookupTest}, {@code FleetdBackendErrorSinkTest}) — those prove the + * factories work, never that {@code main} still calls them. This class is a plain source-text + * assertion on {@code Fleetd.java} — crude, but honest about what it checks, and it turns red the + * instant the wiring is dropped, mirroring the same fallback shape {@link + * FleetdFleetAppConstructionTest} already uses for a different constructor argument. + * + *

This test checks source text, not runtime behaviour. It never constructs a {@code + * CompletionResolver} and never runs {@code main}. + */ +class FleetdCompletionResolverWiringTest { + + private static String fleetdSource() throws Exception { + return Files.readString(Path.of("src/main/java/dev/ltms/fleet/Fleetd.java")); + } + + @Test + @DisplayName("[SOURCE TEXT] CompletionResolver's construction call still names backendErrorPatterns and backendErrorSink") + void backendErrorArgumentsAreStillNamedAtTheCallSite() throws Exception { + String source = fleetdSource(); + assertTrue(source.contains( + "exhaustionSink, backendErrorPatterns, backendErrorSink, System::nanoTime,"), + "CompletionResolver's construction call must still pass backendErrorPatterns and " + + "backendErrorSink as its 5th/6th arguments. Replacing them with " + + "BackendErrorPatternLookup.legacy()/BackendErrorSink.none() (fleetd #248's measured " + + "mutation) compiles with 0 errors and leaves every behavioural test green — this " + + "source check is what must go red instead."); + } + + @Test + @DisplayName("[SOURCE TEXT] CompletionResolver's construction call still passes worktreeBranchLookup(sessions::roster)") + void worktreeBranchLookupIsStillPassedAtTheCallSite() throws Exception { + String source = fleetdSource(); + assertTrue(source.contains("worktreeBranchLookup(sessions::roster)"), + "CompletionResolver's construction call must still pass worktreeBranchLookup(sessions::roster) " + + "as its 8th (last) argument. Replacing it with the inert `_ -> null` (fleetd #248's " + + "other measured mutation) compiles with 0 errors and leaves every behavioural test " + + "green — this source check is what must go red instead."); + assertFalse(source.contains("System::nanoTime,\n _ -> null"), + "the worktree/branch argument must never regress to the inert `_ -> null` literal"); + } + + @Test + @DisplayName("[SOURCE TEXT] backendErrorPatterns is assigned from the extracted backendErrorPatternLookup(...) factory") + void backendErrorPatternsComesFromTheFactory() throws Exception { + String source = fleetdSource(); + assertTrue(source.contains( + "BackendErrorPatternLookup backendErrorPatterns = backendErrorPatternLookup(sessions::roster,"), + "backendErrorPatterns must be assigned from Fleetd.backendErrorPatternLookup(...), not an " + + "inline lambda that a source check on the CompletionResolver call alone cannot see " + + "through"); + } + + @Test + @DisplayName("[SOURCE TEXT] backendErrorSink is assigned from the extracted backendErrorSink(...) factory") + void backendErrorSinkComesFromTheFactory() throws Exception { + String source = fleetdSource(); + assertTrue(source.contains( + "BackendErrorSink backendErrorSink = backendErrorSink(sessions, () -> config.get().profiles(),"), + "backendErrorSink must be assigned from Fleetd.backendErrorSink(...), not an inline lambda " + + "that a source check on the CompletionResolver call alone cannot see through"); + } +} diff --git a/fleetd/src/test/java/dev/ltms/fleet/FleetdWorktreeBranchLookupTest.java b/fleetd/src/test/java/dev/ltms/fleet/FleetdWorktreeBranchLookupTest.java new file mode 100644 index 0000000..9b51fc8 --- /dev/null +++ b/fleetd/src/test/java/dev/ltms/fleet/FleetdWorktreeBranchLookupTest.java @@ -0,0 +1,64 @@ +package dev.ltms.fleet; + +import dev.ltms.fleet.inject.CompletionResolver; +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 java.util.ArrayList; +import java.util.List; +import java.util.function.Function; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNull; + +/** + * fleetd #248: {@link Fleetd#worktreeBranchLookup} is the factory that replaced the anonymous + * lambda {@code Fleetd.main} used to build inline, as the 8th (last) argument to {@code + * CompletionResolver}'s constructor. Before this ticket that argument was untestable wiring: + * replacing it with {@code _ -> null} compiled clean and every existing test stayed green, because + * every existing test builds its own {@code CompletionResolver} directly rather than going through + * {@code main}. This class proves the factory's own behaviour; {@code + * FleetdCompletionResolverWiringTest} proves {@code main} still passes it in. + */ +class FleetdWorktreeBranchLookupTest { + + private static MemberSession session(String terminal, String worktree, String branch) { + return new MemberSession("pane-" + terminal, terminal, "terra", MemberRole.DEV, + "/cwd", null, 0L, 0L, 0, MemberSession.State.READY, worktree, branch); + } + + @Test + @DisplayName("a known target resolves to its session's worktree and branch") + void knownTargetResolves() { + Function lookup = + Fleetd.worktreeBranchLookup(() -> List.of(session("term1", "/wt/worker_x", "worker/x"))); + + CompletionResolver.WorktreeBranch resolved = lookup.apply("term1"); + + assertEquals("/wt/worker_x", resolved.worktree()); + assertEquals("worker/x", resolved.branch()); + } + + @Test + @DisplayName("an unknown target resolves to null, not a thrown exception") + void unknownTargetResolvesToNull() { + Function lookup = + Fleetd.worktreeBranchLookup(() -> List.of(session("term1", "/wt/worker_x", "worker/x"))); + + assertNull(lookup.apply("term_stranger")); + } + + @Test + @DisplayName("the roster is read through the supplier on every call, not snapshotted") + void rosterIsReadThroughOnEveryCall() { + List roster = new ArrayList<>(); + Function lookup = Fleetd.worktreeBranchLookup(() -> roster); + + assertNull(lookup.apply("term_late")); + roster.add(session("term_late", "/wt/late", "worker/late")); + + assertEquals("/wt/late", lookup.apply("term_late").worktree()); + } +}