diff --git a/fleetd/src/test/java/dev/ltms/fleet/FleetdLeadRolloverWiringTest.java b/fleetd/src/test/java/dev/ltms/fleet/FleetdLeadRolloverWiringTest.java index 89884f8..c792e53 100644 --- a/fleetd/src/test/java/dev/ltms/fleet/FleetdLeadRolloverWiringTest.java +++ b/fleetd/src/test/java/dev/ltms/fleet/FleetdLeadRolloverWiringTest.java @@ -13,16 +13,31 @@ import static org.junit.jupiter.api.Assertions.assertTrue; * FleetdCompletionResolverWiringTest}'s pattern — five log-only reporters in {@code Fleetd.main} * already survived mutation batteries this exact way (fleetd #415's extraction antidote note). * - *

No behavioural test can catch this wiring dropping out: {@code LeadRolloverTest} constructs - * its own {@code LeadRollover} directly (as every prior test of an extracted factory does), so a - * mutation that deletes the {@code leadRollover(...)} call from {@code main} — or replaces one of - * its arguments with something that silently compiles, e.g. {@code router.leadAgents()} swapped - * for {@code null}, or the whole assignment swapped for a bare {@code null} literal — leaves every - * behavioural test green. This is a plain string read, guarded by an unrelated anchor assertion so - * a broken or empty file read cannot pass as a real change. + *

What this class still covers, and what it never claimed to. {@code LeadRolloverTest} + * constructs its own {@code LeadRollover} directly (as every prior test of an extracted factory + * does) with a hand-built lookup, so a mutation that deletes the {@code leadRollover(...)} call + * from {@code main} — or replaces one of its arguments with something that still compiles, e.g. + * {@code router.leadAgents()} swapped for {@code null}, or the whole assignment swapped for a bare + * {@code null} literal — leaves every behavioural test green. This is a plain string read, guarded + * by an unrelated anchor assertion so a broken or empty file read cannot pass as a real change. * *

This test checks source text, not runtime behaviour. It never constructs a {@code - * LeadRollover} and never runs {@code main}. + * LeadRollover} and never runs {@code main}. It pins the {@code leadRollover(...)} CALL SITE's + * argument list — that {@code main} still passes {@code leads} at all — never what the factory + * DOES with that argument once inside its own body. + * + *

Correction (fleetd #480 relative-handover-path follow-up): that gap used to be real, and + * now is not — but not here. This class's javadoc previously claimed "no behavioural test can + * catch this wiring dropping out" for the whole factory, including the lambda {@code + * leadRollover(...)} builds internally (terminal → lead name → {@code Leader.cwd()}). That claim + * was proven true at the time — mutating that lambda's body to {@code String leadName = null;} + * (always "no lead found", which silently reintroduces the daemon-cwd bug this ticket fixes) left + * the full suite green, {@code Tests run: 1669, Failures: 0}. It is no longer true: {@code + * FleetdLeadRolloverWorkspaceLookupTest} now calls {@code Fleetd.leadRollover(...)} directly with a + * real {@link dev.ltms.fleet.config.ConfigRef} built from a temp {@code fleetd.yaml}, and fails + * against that exact one-line mutation. So: THIS class still covers only the call site's argument + * list; {@code FleetdLeadRolloverWorkspaceLookupTest} is what now covers the lambda's body. Neither + * one subsumes the other — keep both. */ class FleetdLeadRolloverWiringTest { diff --git a/fleetd/src/test/java/dev/ltms/fleet/FleetdLeadRolloverWorkspaceLookupTest.java b/fleetd/src/test/java/dev/ltms/fleet/FleetdLeadRolloverWorkspaceLookupTest.java new file mode 100644 index 0000000..ec63553 --- /dev/null +++ b/fleetd/src/test/java/dev/ltms/fleet/FleetdLeadRolloverWorkspaceLookupTest.java @@ -0,0 +1,163 @@ +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.lead.LeadRollover; +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 static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNotNull; + +/** + * fleetd #480 relative-handover-path follow-up, correction round: a BEHAVIOURAL test of {@link + * Fleetd#leadRollover}'s own body — the terminal → lead-name → {@code Leader.cwd()} lookup it + * builds — not another source-text pin. + * + *

{@code FleetdLeadRolloverWiringTest} (a plain string read) still earns its keep: it pins the + * call site's ARGUMENT LIST, so a mutation that drops {@code leads} back out of the call, or + * swaps it for {@code Map::of}, still goes red there. But nothing before this class exercised the + * LAMBDA BODY {@code leadRollover(...)} builds — the terminal-to-workspace {@code + * Function} that {@link LeadRollover#open} actually calls. Every prior test either + * exercised {@code LeadRollover} directly with a hand-built lookup ({@code LeadRolloverTest}), or + * read source text without ever calling the factory ({@code FleetdLeadRolloverWiringTest}) — so a + * mutation that breaks the lookup ITSELF (e.g. always resolving no lead, or reading a snapshot + * instead of the live supplier) left every existing test green while the daemon's real wiring + * silently reintroduced the exact bug this whole ticket fixes: a relative {@code handoverPath} + * resolving against the daemon's own {@code cwd} instead of the calling lead's. + * + *

{@code Fleetd.leadRollover(...)} is package-private and {@code static}, so this test — living + * in the same {@code dev.ltms.fleet} package — calls it directly, exactly the way {@code + * FleetdExhaustionDetectionArmedWiringTest} and {@code FleetdCapacitySourceWiringTest} already call + * other package-private startup factories with a real {@link ConfigRef} built from a temp {@code + * fleetd.yaml} (never the gitignored live one). + * + *

Proved against the mutation it exists to catch. Before this class was added, mutating + * {@code leadRollover(...)}'s lambda body to {@code String leadName = null;} (always "no lead + * found", which forces every relative {@code handoverPath} onto the {@code user.dir} fallback — + * i.e. the original bug) left the full suite green: {@code Tests run: 1669, Failures: 0}. With + * {@link #relativeHandoverPathResolvesAgainstTheLeadsConfiguredCwd()} added, the same one-line + * mutation now fails that test (it asserts the resolved path equals the configured lead's {@code + * cwd}, which the mutant can never produce) — see this ticket's fleet_reply history for both runs. + */ +class FleetdLeadRolloverWorkspaceLookupTest { + + private static AgentControl fakeAgents() { + return new AgentControl(new FakeHerdr()); + } + + @Test + @DisplayName("[BEHAVIOURAL] Fleetd.leadRollover(...) resolves a relative handoverPath against " + + "the CALLING lead's configured cwd, not the daemon's own working directory") + void relativeHandoverPathResolvesAgainstTheLeadsConfiguredCwd(@TempDir Path dir) throws Exception { + Path leadCwd = dir.resolve("lead-workspace"); + Files.createDirectories(leadCwd); + Path yaml = dir.resolve("fleetd.yaml"); + Files.writeString(yaml, """ + bind: + port: 8080 + fleet: + leaders: + opus: + tab: "lead: opus" + cwd: "%s" + leadRollover: + handoverPath: handover.md + """.formatted(leadCwd.toString())); + ConfigRef config = new ConfigRef(yaml, FleetConfig.load(yaml)); + + LeadRollover rollover = Fleetd.leadRollover(config.get(), fakeAgents(), config, + () -> Map.of("term_opus", "opus")); + assertNotNull(rollover, "leadRollover: is present in the loaded config, so the factory " + + "must construct an object"); + + LeadRollover.PendingRollover pending = rollover.open("term_opus", "test"); + + String expected = leadCwd.resolve("handover.md").normalize().toString(); + assertEquals(expected, pending.handoverPath(), + "a relative handoverPath must resolve against the CALLING lead's configured cwd " + + "through the REAL Fleetd.leadRollover(...) wiring — not the daemon's own " + + "working directory. This is the exact axis that was proven uncovered: " + + "mutating the factory's lambda body to always report \"no lead found\" " + + "left every prior test green."); + } + + @Test + @DisplayName("[BEHAVIOURAL] a terminal not present in the live lead-terminal map falls back " + + "to the daemon's own user.dir") + void terminalNotInLiveMapFallsBackToUserDir(@TempDir Path dir) throws Exception { + Path yaml = dir.resolve("fleetd.yaml"); + Files.writeString(yaml, """ + bind: + port: 8080 + leadRollover: + handoverPath: handover.md + """); + ConfigRef config = new ConfigRef(yaml, FleetConfig.load(yaml)); + + // No lead has been discovered yet — exactly the real shape of a lead the live tab scan + // has not yet scanned, or one with no fleet.leaders entry at all. + LeadRollover rollover = Fleetd.leadRollover(config.get(), fakeAgents(), config, Map::of); + assertNotNull(rollover); + + LeadRollover.PendingRollover pending = rollover.open("term_unknown", "test"); + + String expected = Path.of(System.getProperty("user.dir")).resolve("handover.md") + .normalize().toString(); + assertEquals(expected, pending.handoverPath(), + "a lead not present in the live terminal→name map must fall back to the daemon's " + + "own user.dir — the same fallback LeadLauncher#launch already uses for a " + + "lead with no configured cwd. This is deliberate, pinned behaviour, not an " + + "accident of the null-check chain."); + } + + @Test + @DisplayName("[BEHAVIOURAL] the terminal→lead-name lookup is read LIVE on every open() call, " + + "never snapshotted at Fleetd.leadRollover(...) construction time") + void workspaceLookupIsReadLiveNotSnapshotted(@TempDir Path dir) throws Exception { + Path leadCwd = dir.resolve("lead-workspace"); + Files.createDirectories(leadCwd); + Path yaml = dir.resolve("fleetd.yaml"); + Files.writeString(yaml, """ + bind: + port: 8080 + fleet: + leaders: + opus: + tab: "lead: opus" + cwd: "%s" + leadRollover: + handoverPath: handover.md + """.formatted(leadCwd.toString())); + ConfigRef config = new ConfigRef(yaml, FleetConfig.load(yaml)); + + // EMPTY at the moment leadRollover(...) is called. A lookup captured (snapshotted) here + // instead of read live through the supplier on every call would never see the entry added + // below — exactly the natural mistake to make, since leads are discovered by a live tab + // scan that runs AFTER this factory is constructed at startup. + Map liveLeadTerminals = new HashMap<>(); + LeadRollover rollover = Fleetd.leadRollover(config.get(), fakeAgents(), config, + () -> liveLeadTerminals); + assertNotNull(rollover); + + // The lead is "discovered" only now — mutate the SAME backing map the supplier reads from. + liveLeadTerminals.put("term_opus", "opus"); + + LeadRollover.PendingRollover pending = rollover.open("term_opus", "test"); + + String expected = leadCwd.resolve("handover.md").normalize().toString(); + assertEquals(expected, pending.handoverPath(), + "the terminal→lead-name lookup must be read LIVE on every open() call — a lead " + + "discovered by the tab scan AFTER Fleetd.leadRollover(...) was constructed " + + "must still resolve correctly, not only one that was already live at " + + "construction time"); + } +}