From 4bfab6b71805153d4aa9a14577e49af70ecee4d3 Mon Sep 17 00:00:00 2001 From: Dai Ha Date: Sat, 12 Sep 2026 05:19:49 +0700 Subject: [PATCH 1/3] fleetd #480 follow-up: resolve a relative leadRollover.handoverPath against the calling lead's workspace LeadRollover.open() now resolves handoverPath to an absolute path exactly once, against the calling lead's fleet.leaders..cwd (falling back to the daemon's own user.dir when that lead has none configured), matching the LeadLauncher#launch precedent. PendingRollover stores only the resolved absolute path, so checkHandover's exists/empty/fresh checks, the path handed back to the lead in the fleet_handover open response, and the default bootstrapText sentence all see the same absolute location instead of a value resolved against whatever directory the daemon process happened to start in. FleetConfig.LeadRollover.bootstrapText is no longer defaulted in the compact constructor (it would otherwise still bake in the raw, possibly-relative handoverPath); a new bootstrapTextFor (resolvedHandoverPath) method builds the default sentence from the resolved path instead. Fleetd.leadRollover(...) gains a required liveLeadTerminals parameter to build the terminal to lead-name to Leader.cwd lookup, read live through the existing `leads` supplier and ConfigRef on every call, never off a startup snapshot. --- fleetd/fleetd.example.yaml | 11 +- .../src/main/java/dev/ltms/fleet/Fleetd.java | 42 ++++- .../dev/ltms/fleet/config/FleetConfig.java | 41 +++- .../dev/ltms/fleet/lead/LeadRollover.java | 88 ++++++++- .../fleet/FleetdLeadRolloverWiringTest.java | 10 +- .../dev/ltms/fleet/lead/LeadRolloverTest.java | 175 +++++++++++++++++- .../ltms/fleet/mcp/FleetMcpHandoverTest.java | 4 +- 7 files changed, 335 insertions(+), 36 deletions(-) diff --git a/fleetd/fleetd.example.yaml b/fleetd/fleetd.example.yaml index 4e4f7ea..2097fcf 100644 --- a/fleetd/fleetd.example.yaml +++ b/fleetd/fleetd.example.yaml @@ -103,7 +103,12 @@ bind: # # handoverPath: REQUIRED when this block is present — where the handover file a fresh lead session # reads must live. No default (an operator-specific path); a present block with no -# handoverPath refuses to start. +# handoverPath refuses to start. May be relative: it then resolves against the +# CALLING lead's own fleet.leaders..cwd (falling back to the daemon's own +# working directory when that lead has none configured) — never against whatever +# directory the daemon process happens to have been started in. An absolute path is +# used unchanged. Prefer an absolute path if the daemon and the lead's pane might not +# share a working directory (fleetd #480 follow-up). # requireOperatorConfirm: true # default true — confirm() refuses unless the caller also passes # # operatorConfirmed: true # maxDocAgeSeconds: 3600 # default 3600 — refuse a handover file older than this @@ -114,8 +119,8 @@ bind: # clearSettleSeconds: 20 # default 20 — how long to wait for the pane to become injectable # # again AFTER /clear before giving up (never sends bootstrapText # # if this elapses). A separate, second wait from turnSettleSeconds. -# bootstrapText: "..." # default names handoverPath — sent to the lead once its pane -# # settles after /clear +# bootstrapText: "..." # default names the RESOLVED (absolute) handoverPath — sent to +# # the lead once its pane settles after /clear # leadRollover: # handoverPath: /path/to/handover.md # requireOperatorConfirm: true diff --git a/fleetd/src/main/java/dev/ltms/fleet/Fleetd.java b/fleetd/src/main/java/dev/ltms/fleet/Fleetd.java index ae2e929..429e1c7 100644 --- a/fleetd/src/main/java/dev/ltms/fleet/Fleetd.java +++ b/fleetd/src/main/java/dev/ltms/fleet/Fleetd.java @@ -600,7 +600,7 @@ public final class Fleetd { // layer from the connection, the same way auth/CallerResolver#resolve builds a // Principal.leader(...)), never from a single-slot lookup — see LeadRollover's class // javadoc, fleetd #480 correction 2. - LeadRollover leadRollover = leadRollover(cfg, router.leadAgents(), config); + LeadRollover leadRollover = leadRollover(cfg, router.leadAgents(), config, leads); MessageService messages = new MessageService(router, injector, rendezvous, replyInbox, pushLoop, metrics); @@ -1089,20 +1089,44 @@ public final class Fleetd { * the connection and pass it in, never take it as a request field. See {@link LeadRollover}'s * class javadoc. * - * @param cfg the startup config snapshot — read ONCE here, only to decide whether to - * construct the object at all, exactly like {@code cfg.leadHeartbeat()} - * @param leadAgents the {@link AgentControl} instance that reaches the LEAD's pane (not - * {@code memberAgents}), normally {@code router.leadAgents()} - * @param config the live {@link ConfigRef}, captured only inside the returned supplier — - * never dereferenced here + *

fleetd #480 follow-up: also builds the terminal → lead-workspace lookup + * {@link LeadRollover#open} needs to resolve a relative {@code handoverPath} against the + * CALLING lead's own {@code cwd} rather than the daemon's — the daemon and a lead's pane can + * have different working directories (this repo nests {@code fleetd/} inside its own root, so + * they already differ on this host). The lookup is terminal → lead name (via {@code + * liveLeadTerminals}) → that lead's {@code cwd} (via {@code config.get().fleet().leaders()}), + * and both hops are read LIVE on every call, never off a snapshot taken here: leads are + * discovered by a live tab scan ({@code LeadTabScanner}), so a map captured at construction + * time could be empty (no lead has been scanned yet) or stale (a lead added since). + * + * @param cfg the startup config snapshot — read ONCE here, only to decide whether + * to construct the object at all, exactly like {@code + * cfg.leadHeartbeat()} + * @param leadAgents the {@link AgentControl} instance that reaches the LEAD's pane (not + * {@code memberAgents}), normally {@code router.leadAgents()} + * @param config the live {@link ConfigRef}, captured only inside the returned + * supplier and the workspace lookup — never dereferenced here + * @param liveLeadTerminals terminal id → lead NAME for every CURRENTLY recognised lead, normally + * the same {@code leads} supplier {@code main} already builds for + * {@code HerdrRouter}/{@link #leadSeatLookup} — never a value snapshot * @return a constructed {@link LeadRollover}, or {@code null} when {@code leadRollover:} is * absent from the startup config */ - static LeadRollover leadRollover(FleetConfig cfg, AgentControl leadAgents, ConfigRef config) { + static LeadRollover leadRollover(FleetConfig cfg, AgentControl leadAgents, ConfigRef config, + Supplier> liveLeadTerminals) { if (cfg.leadRollover() == null) { return null; } - return new LeadRollover(leadAgents, () -> config.get().leadRollover()); + Function leadWorkspace = terminal -> { + String leadName = liveLeadTerminals.get().get(terminal); + if (leadName == null) { + return null; + } + FleetConfig.Fleet fleet = config.get().fleet(); + FleetConfig.Leader leader = fleet == null ? null : fleet.leaders().get(leadName); + return leader == null ? null : leader.cwd(); + }; + return new LeadRollover(leadAgents, () -> config.get().leadRollover(), leadWorkspace); } /** diff --git a/fleetd/src/main/java/dev/ltms/fleet/config/FleetConfig.java b/fleetd/src/main/java/dev/ltms/fleet/config/FleetConfig.java index 6eb1483..b53707d 100644 --- a/fleetd/src/main/java/dev/ltms/fleet/config/FleetConfig.java +++ b/fleetd/src/main/java/dev/ltms/fleet/config/FleetConfig.java @@ -1369,7 +1369,13 @@ public record FleetConfig( * lead session reads must live. There is no sane non-null default for an * operator-specific path, so a present block with a {@code null}/blank * {@code handoverPath} is refused at config load; see - * {@link #validateLeadRollover()}. + * {@link #validateLeadRollover()}. May be relative: {@code + * dev.ltms.fleet.lead.LeadRollover#open} resolves a relative path against + * the CALLING lead's configured {@code fleet.leaders..cwd}, falling + * back to the daemon's own working directory ({@code + * System.getProperty("user.dir")}) when that lead has none configured — + * never against whatever directory the daemon process happens to have been + * started in for its own sake. An absolute path is used unchanged. * @param requireOperatorConfirm default {@code true} — {@code confirm()} refuses unless the * caller also passes {@code operatorConfirmed: true}. Set {@code false} to * let the three handover-file checks alone gate the roll. @@ -1382,9 +1388,14 @@ public record FleetConfig( * @param clearSettleSeconds default 20 — bound on how long to wait for the lead's pane to * report an injectable state again after {@code /clear} before giving up. A * roll that times out here never sends {@code bootstrapText}. - * @param bootstrapText default a sentence naming {@code handoverPath} — sent to the lead's pane - * once it settles after {@code /clear}, telling the fresh session where to - * read the handover and carry on. + * @param bootstrapText default a sentence naming the RESOLVED handover path — sent to the + * lead's pane once it settles after {@code /clear}, telling the fresh + * session where to read the handover and carry on. Left {@code null} here + * when the operator configures none: the default sentence cannot be built + * at construction time because it must name the path AFTER {@code + * dev.ltms.fleet.lead.LeadRollover#open} has resolved a relative {@code + * handoverPath} against the calling lead's workspace, which this record has + * no way to know — see {@link #bootstrapTextFor(String)}. */ @JsonIgnoreProperties(ignoreUnknown = true) public record LeadRollover(String handoverPath, Boolean requireOperatorConfirm, @@ -1395,10 +1406,24 @@ public record FleetConfig( maxDocAgeSeconds = (maxDocAgeSeconds == null || maxDocAgeSeconds <= 0) ? 3600 : maxDocAgeSeconds; turnSettleSeconds = (turnSettleSeconds == null || turnSettleSeconds <= 0) ? 20 : turnSettleSeconds; clearSettleSeconds = (clearSettleSeconds == null || clearSettleSeconds <= 0) ? 20 : clearSettleSeconds; - bootstrapText = (bootstrapText == null || bootstrapText.isBlank()) - ? "Fresh lead session: read the handover file at " + handoverPath - + " and carry on from there." - : bootstrapText; + bootstrapText = (bootstrapText == null || bootstrapText.isBlank()) ? null : bootstrapText; + } + + /** + * The text actually sent to the lead's pane once it settles after {@code /clear}: the + * operator's configured {@link #bootstrapText} when one is set, otherwise the default + * sentence built from {@code resolvedHandoverPath}. + * + * @param resolvedHandoverPath the ABSOLUTE path {@code dev.ltms.fleet.lead.LeadRollover + * #open} already resolved — never the raw configured {@link + * #handoverPath}, which may still be relative and would name a + * directory the fresh lead session's own pane cannot resolve + */ + public String bootstrapTextFor(String resolvedHandoverPath) { + return bootstrapText != null + ? bootstrapText + : "Fresh lead session: read the handover file at " + resolvedHandoverPath + + " and carry on from there."; } } diff --git a/fleetd/src/main/java/dev/ltms/fleet/lead/LeadRollover.java b/fleetd/src/main/java/dev/ltms/fleet/lead/LeadRollover.java index c22ee8e..f43c3c8 100644 --- a/fleetd/src/main/java/dev/ltms/fleet/lead/LeadRollover.java +++ b/fleetd/src/main/java/dev/ltms/fleet/lead/LeadRollover.java @@ -15,6 +15,7 @@ import java.util.UUID; import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.TimeUnit; import java.util.function.Consumer; +import java.util.function.Function; import java.util.function.LongSupplier; import java.util.function.Supplier; @@ -52,7 +53,7 @@ import java.util.function.Supplier; *

  • {@code agents.send(lead, "/clear")}
    • *
  • wait again for the pane to report injectable, bounded by {@code clearSettleSeconds} (this * is the original, pre-correction wait — still here, just no longer the only one)
    • - *
  • {@code agents.send(lead, cfg.bootstrapText())}
    • + *
  • {@code agents.send(lead, cfg.bootstrapTextFor(p.handoverPath()))}
    • * * A {@link #confirm} that returns {@link RollDecision#approved()} therefore means "every gate * passed and the roll is scheduled", never "the pane has been cleared" — the pane may @@ -87,6 +88,21 @@ import java.util.function.Supplier; * build a {@code Principal.leader(...)} (see its use of {@code ConnectionIdentity.Caller#terminal}) * — never a value the client supplies or chooses. The later MCP-tool unit that wires * {@link #open}/{@link #confirm} must pass the resolved caller terminal, not a request field. + * + *

    A relative {@code handoverPath} resolves against the CALLING lead's workspace, never + * the daemon's own cwd — a fleetd #480 follow-up. The daemon and a lead's own pane can + * have different working directories (this repo nests {@code fleetd/} inside its own root, so the + * daemon's cwd and {@code fleet.leaders..cwd} already differ on this host). {@link #open} + * resolves {@code cfg.handoverPath()} to an ABSOLUTE path exactly once — against {@code + * leadWorkspace.apply(leadTerminal)} when that lookup returns a non-null, non-blank workspace, and + * against {@code System.getProperty("user.dir")} otherwise (the same fallback {@code + * LeadLauncher#launch} already uses for a lead with no configured {@code cwd}) — and stores only + * that absolute path on {@link PendingRollover}. Every later read of {@code + * PendingRollover#handoverPath()} (the freshness/exists/empty checks in {@link #checkHandover}, + * the value {@code FleetMcp} hands back to the lead in the {@code open} response so it knows where + * to WRITE the file, and {@link FleetConfig.LeadRollover#bootstrapTextFor} which names it in the + * text typed into the fresh session) therefore already sees the resolved absolute form and never + * needs to resolve anything itself. */ public final class LeadRollover { @@ -100,6 +116,11 @@ public final class LeadRollover { * * @param leadTerminal the lead pane that opened this request — the only terminal that may * later {@link #confirm} it (see {@link RefusalReason#NOT_YOUR_ROLLOVER}) + * @param handoverPath the ABSOLUTE, resolved handover path — never the raw configured value, + * which may have been relative. {@link #open} resolves it once, against the + * calling lead's workspace, before storing it here; see this class's + * javadoc. This is the value the MCP layer hands back to the lead as + * "write your file here", so callers may rely on it always being absolute. */ public record PendingRollover(String token, String leadTerminal, String handoverPath, long requestedAtMillis) {} @@ -148,6 +169,14 @@ public final class LeadRollover { private final AgentControl agents; private final Supplier configSupplier; + /** + * Terminal id → that lead's configured workspace directory (their {@code + * fleet.leaders..cwd}), or {@code null} when the terminal names no currently-recognised + * lead. {@link #open} calls this to resolve a relative {@code handoverPath} — see this class's + * javadoc. Required: there is no sane default that would not silently reintroduce the + * daemon-cwd bug this parameter exists to fix. + */ + private final Function leadWorkspace; private final LongSupplier nowMillis; private final Runnable settleSleeper; /** @@ -160,8 +189,9 @@ public final class LeadRollover { private final Map pending = new ConcurrentHashMap<>(); /** Production constructor — wall clock, real sleep between settle polls, a real virtual thread. */ - public LeadRollover(AgentControl agents, Supplier configSupplier) { - this(agents, configSupplier, System::currentTimeMillis, + public LeadRollover(AgentControl agents, Supplier configSupplier, + Function leadWorkspace) { + this(agents, configSupplier, leadWorkspace, System::currentTimeMillis, () -> sleepUninterruptibly(SETTLE_POLL_MS), r -> Thread.ofVirtual().name("lead-rollover-continuation-").start(r)); } @@ -174,9 +204,11 @@ public final class LeadRollover { * nanoTime} freezes while the host sleeps (fleetd #386). */ LeadRollover(AgentControl agents, Supplier configSupplier, - LongSupplier nowMillis, Runnable settleSleeper, Consumer continuationRunner) { + Function leadWorkspace, LongSupplier nowMillis, + Runnable settleSleeper, Consumer continuationRunner) { this.agents = agents; this.configSupplier = configSupplier; + this.leadWorkspace = leadWorkspace; this.nowMillis = nowMillis; this.settleSleeper = settleSleeper; this.continuationRunner = continuationRunner; @@ -215,13 +247,49 @@ public final class LeadRollover { } String token = UUID.randomUUID().toString(); long requestedAt = nowMillis.getAsLong(); - PendingRollover p = new PendingRollover(token, leadTerminal, cfg.handoverPath(), requestedAt); + String resolvedPath = resolveHandoverPath(cfg.handoverPath(), leadTerminal); + PendingRollover p = new PendingRollover(token, leadTerminal, resolvedPath, requestedAt); pending.put(token, p); - log.info("lead-rollover: open token={} lead={} handoverPath={} reason={}", - token, leadTerminal, p.handoverPath(), reason); + if (resolvedPath.equals(cfg.handoverPath())) { + log.info("lead-rollover: open token={} lead={} handoverPath={} reason={}", + token, leadTerminal, resolvedPath, reason); + } else { + // The configured value was relative (or merely un-normalized) and resolved to a + // different string — log both, so an operator reading this line can see which + // directory the daemon actually looked in, not just the value it was given. + log.info("lead-rollover: open token={} lead={} configuredHandoverPath={} " + + "resolvedHandoverPath={} reason={}", + token, leadTerminal, cfg.handoverPath(), resolvedPath, reason); + } return p; } + /** + * Resolve {@code configured} to an absolute path exactly once, here, so nothing downstream + * ({@link #checkHandover}, the MCP layer's {@code open} response, {@link + * FleetConfig.LeadRollover#bootstrapTextFor}) ever has to resolve — or worse, silently + * mis-resolve — a relative path again. + * + *

      + *
    • already absolute → returned unchanged (normalized)
      • + *
    • relative → resolved against {@code leadWorkspace.apply(leadTerminal)} when that is + * non-null and non-blank; otherwise against {@code System.getProperty("user.dir")} — the + * same fallback {@code LeadLauncher#launch} uses for a lead with no configured {@code + * cwd}
      • + *
    + */ + private String resolveHandoverPath(String configured, String leadTerminal) { + Path path = Path.of(configured); + if (path.isAbsolute()) { + return path.normalize().toString(); + } + String workspace = leadWorkspace.apply(leadTerminal); + Path base = (workspace == null || workspace.isBlank()) + ? Path.of(System.getProperty("user.dir")) + : Path.of(workspace); + return base.resolve(path).normalize().toString(); + } + /** * Validate every gate, then — if and only if all of them pass — hand a one-shot continuation * that performs the actual roll to {@code continuationRunner} and return. This method @@ -302,7 +370,7 @@ public final class LeadRollover { lead, cfg.clearSettleSeconds(), p.token()); return; } - agents.send(lead, cfg.bootstrapText()); + agents.send(lead, cfg.bootstrapTextFor(p.handoverPath())); log.info("lead-rollover: rolled token={} lead={}", p.token(), lead); } @@ -313,7 +381,9 @@ public final class LeadRollover { /** * The three handover-file checks, in order: exists, not empty, fresh (modified after - * {@link #open}'s timestamp and not older than {@code maxDocAgeSeconds}). + * {@link #open}'s timestamp and not older than {@code maxDocAgeSeconds}). Stats {@code + * p.handoverPath()} directly — {@link #open} already resolved it to an absolute path, so this + * never has to guess which directory it means. * * @return the first failing check's refusal, or {@code null} when all three pass */ diff --git a/fleetd/src/test/java/dev/ltms/fleet/FleetdLeadRolloverWiringTest.java b/fleetd/src/test/java/dev/ltms/fleet/FleetdLeadRolloverWiringTest.java index 9e536d8..89884f8 100644 --- a/fleetd/src/test/java/dev/ltms/fleet/FleetdLeadRolloverWiringTest.java +++ b/fleetd/src/test/java/dev/ltms/fleet/FleetdLeadRolloverWiringTest.java @@ -49,14 +49,16 @@ class FleetdLeadRolloverWiringTest { void mainStillCallsTheLeadRolloverFactory() throws Exception { String source = fleetdSource(); assertTrue(source.contains( - "LeadRollover leadRollover = leadRollover(cfg, router.leadAgents(), config);"), + "LeadRollover leadRollover = leadRollover(cfg, router.leadAgents(), config, leads);"), "Fleetd.main must still assign `LeadRollover leadRollover = leadRollover(cfg, " - + "router.leadAgents(), config);`. Dropping this call, or swapping one of its " - + "arguments for something that still compiles (e.g. null in place of " + + "router.leadAgents(), config, leads);`. Dropping this call, or swapping one of " + + "its arguments for something that still compiles (e.g. null in place of " + "router.leadAgents()), leaves every behavioural test green — this source check is " + "what must go red instead. fleetd #480 correction 2 deliberately dropped " + "primaryRegistry from this call — see LeadRollover's class javadoc for why a " - + "single-slot lookup was wrong here."); + + "single-slot lookup was wrong here. The fleetd #480 relative-handover-path " + + "follow-up added `leads` (terminal → lead name) so the factory can resolve a " + + "relative handoverPath against the calling lead's own workspace."); } @Test diff --git a/fleetd/src/test/java/dev/ltms/fleet/lead/LeadRolloverTest.java b/fleetd/src/test/java/dev/ltms/fleet/lead/LeadRolloverTest.java index 22213cb..d2f56ff 100644 --- a/fleetd/src/test/java/dev/ltms/fleet/lead/LeadRolloverTest.java +++ b/fleetd/src/test/java/dev/ltms/fleet/lead/LeadRolloverTest.java @@ -14,6 +14,7 @@ import java.io.IOException; import java.nio.file.Files; import java.nio.file.Path; import java.util.concurrent.atomic.AtomicLong; +import java.util.function.Function; import java.util.function.LongSupplier; import static org.junit.jupiter.api.Assertions.*; @@ -70,8 +71,13 @@ class LeadRolloverTest { private static LeadRollover newRollover(HerdrClient herdr, FleetConfig.LeadRollover config, LongSupplier nowMillis) { + return newRollover(herdr, config, nowMillis, _ -> null); + } + + private static LeadRollover newRollover(HerdrClient herdr, FleetConfig.LeadRollover config, + LongSupplier nowMillis, Function leadWorkspace) { AgentControl agents = new AgentControl(herdr); - return new LeadRollover(agents, () -> config, nowMillis, () -> { }, Runnable::run); + return new LeadRollover(agents, () -> config, leadWorkspace, nowMillis, () -> { }, Runnable::run); } private Path writeHandover(String content) throws IOException { @@ -395,7 +401,7 @@ class LeadRolloverTest { void openThrowsWhenNotConfigured() { FakeHerdr herdr = new FakeHerdr(); AgentControl agents = new AgentControl(herdr); - LeadRollover rollover = new LeadRollover(agents, () -> null, () -> 1_000L, () -> { }, Runnable::run); + LeadRollover rollover = new LeadRollover(agents, () -> null, _ -> null, () -> 1_000L, () -> { }, Runnable::run); assertThrows(IllegalStateException.class, () -> rollover.open(LEAD, "context is full")); } @@ -475,4 +481,169 @@ class LeadRolloverTest { LeadRollover.RollDecision again = rollover.confirm(LEAD, pending.token(), true); assertEquals(LeadRollover.RefusalReason.UNKNOWN_TOKEN, again.reason()); } + + // ---- fleetd #480 follow-up: a relative handoverPath resolves against the CALLING lead's own + // workspace, never the daemon's cwd ------------------------------------------------------ + + @Test + @DisplayName("[fleetd #480 follow-up] a relative handoverPath resolves against the lead's " + + "configured workspace, and PendingRollover carries the absolute path") + void relativeHandoverPathResolvesAgainstLeadWorkspace() { + Path workspace = tmp.resolve("lead-workspace"); + FakeHerdr herdr = new FakeHerdr(); + AtomicLong clock = new AtomicLong(1_000); + FleetConfig.LeadRollover config = cfg("handover.md"); // relative — no directory component + Function leadWorkspace = terminal -> LEAD.equals(terminal) ? workspace.toString() : null; + LeadRollover rollover = newRollover(herdr, config, fixedClock(clock), leadWorkspace); + + LeadRollover.PendingRollover pending = rollover.open(LEAD, "context is full"); + + String expected = workspace.resolve("handover.md").normalize().toString(); + assertEquals(expected, pending.handoverPath(), + "a relative handoverPath must resolve against the CALLING lead's own workspace, " + + "never the daemon's own working directory"); + assertTrue(Path.of(pending.handoverPath()).isAbsolute(), + "the resolved path stored on PendingRollover must always be absolute"); + } + + @Test + @DisplayName("[fleetd #480 follow-up] confirm() accepts a handover file written at the " + + "resolved absolute location for a relative handoverPath") + void confirmAcceptsAFileWrittenAtTheResolvedLocation() throws IOException { + Path workspace = tmp.resolve("lead-workspace"); + Files.createDirectories(workspace); + Path expected = workspace.resolve("handover.md"); + Files.writeString(expected, "handover contents"); + + FakeHerdr herdr = new FakeHerdr(); + AtomicLong clock = new AtomicLong(1_000); + FleetConfig.LeadRollover config = cfg("handover.md"); + Function leadWorkspace = terminal -> LEAD.equals(terminal) ? workspace.toString() : null; + LeadRollover rollover = newRollover(herdr, config, fixedClock(clock), leadWorkspace); + + LeadRollover.PendingRollover pending = rollover.open(LEAD, "context is full"); + assertEquals(expected.toString(), pending.handoverPath()); + + LeadRollover.RollDecision decision = rollover.confirm(LEAD, pending.token(), true); + assertTrue(decision.accepted(), "expected approval; got: " + decision.reason() + " / " + decision.detail()); + } + + @Test + @DisplayName("[fleetd #480 follow-up] a file at the same relative name under a DIFFERENT " + + "directory is NOT accepted — resolution is against the lead's own workspace only") + void relativeHandoverPathDoesNotMatchAFileUnderADifferentDirectory() throws IOException { + Path workspace = tmp.resolve("lead-workspace"); + Path decoy = tmp.resolve("decoy-directory"); + Files.createDirectories(workspace); + Files.createDirectories(decoy); + // the decoy directory holds a file with the SAME relative name — if resolution ever fell + // back to searching, or resolved against the wrong base, this would be wrongly found. + Files.writeString(decoy.resolve("handover.md"), "decoy contents — must never be read"); + + FakeHerdr herdr = new FakeHerdr(); + AtomicLong clock = new AtomicLong(1_000); + FleetConfig.LeadRollover config = cfg("handover.md"); + Function leadWorkspace = terminal -> LEAD.equals(terminal) ? workspace.toString() : null; + LeadRollover rollover = newRollover(herdr, config, fixedClock(clock), leadWorkspace); + + LeadRollover.PendingRollover pending = rollover.open(LEAD, "context is full"); + assertEquals(workspace.resolve("handover.md").toString(), pending.handoverPath()); + + LeadRollover.RollDecision decision = rollover.confirm(LEAD, pending.token(), true); + assertFalse(decision.accepted()); + assertEquals(LeadRollover.RefusalReason.HANDOVER_MISSING, decision.reason(), + "the file at workspace/handover.md does not exist — a decoy file at the same " + + "relative name under a different directory must never be mistaken for it"); + } + + @Test + @DisplayName("[fleetd #480 follow-up] an absolute handoverPath is used unchanged — the lead " + + "workspace lookup is never even consulted") + void absoluteHandoverPathIsUnchangedByResolution() throws IOException { + Path absolute = writeHandover("handover contents"); // tmp.resolve(...) — always absolute + FakeHerdr herdr = new FakeHerdr(); + AtomicLong clock = new AtomicLong(1_000); + FleetConfig.LeadRollover config = cfg(absolute.toString()); + // a workspace lookup that would resolve a RELATIVE path somewhere completely different — + // proving it is never consulted at all for an already-absolute handoverPath. + Function leadWorkspace = terminal -> tmp.resolve("some-other-workspace").toString(); + LeadRollover rollover = newRollover(herdr, config, fixedClock(clock), leadWorkspace); + + LeadRollover.PendingRollover pending = rollover.open(LEAD, "context is full"); + + assertEquals(absolute.normalize().toString(), pending.handoverPath(), + "an absolute handoverPath must be used unchanged (aside from normalization)"); + } + + @Test + @DisplayName("[fleetd #480 follow-up] a terminal with no configured workspace (null lookup " + + "result) falls back to the daemon's own user.dir") + void noConfiguredWorkspaceFallsBackToUserDir() { + FakeHerdr herdr = new FakeHerdr(); + AtomicLong clock = new AtomicLong(1_000); + FleetConfig.LeadRollover config = cfg("some-handover.md"); + Function leadWorkspace = _ -> null; // no configured workspace for anyone + LeadRollover rollover = newRollover(herdr, config, fixedClock(clock), leadWorkspace); + + LeadRollover.PendingRollover pending = rollover.open(LEAD, "context is full"); + + String expected = Path.of(System.getProperty("user.dir")).resolve("some-handover.md") + .normalize().toString(); + assertEquals(expected, pending.handoverPath(), + "with no configured workspace for this lead, resolution must fall back to the " + + "daemon's own user.dir — the same fallback LeadLauncher#launch already uses " + + "for a lead with no configured cwd"); + } + + @Test + @DisplayName("[fleetd #480 follow-up] a terminal whose workspace lookup returns a blank string " + + "also falls back to the daemon's own user.dir") + void blankConfiguredWorkspaceFallsBackToUserDir() { + FakeHerdr herdr = new FakeHerdr(); + AtomicLong clock = new AtomicLong(1_000); + FleetConfig.LeadRollover config = cfg("some-handover.md"); + Function leadWorkspace = _ -> " "; + LeadRollover rollover = newRollover(herdr, config, fixedClock(clock), leadWorkspace); + + LeadRollover.PendingRollover pending = rollover.open(LEAD, "context is full"); + + String expected = Path.of(System.getProperty("user.dir")).resolve("some-handover.md") + .normalize().toString(); + assertEquals(expected, pending.handoverPath(), + "a blank (non-null) workspace lookup result must be treated the same as null — " + + "fall back to user.dir, never resolve against an empty base"); + } + + @Test + @DisplayName("[fleetd #480 follow-up] the default bootstrapText that is actually SENT names " + + "the RESOLVED absolute handoverPath, not the raw relative configured value") + void defaultBootstrapTextNamesTheResolvedAbsolutePath() throws IOException { + Path workspace = tmp.resolve("lead-workspace"); + Files.createDirectories(workspace); + Path expected = workspace.resolve("handover.md"); + Files.writeString(expected, "handover contents"); + + FakeHerdr herdr = new FakeHerdr(); // default agentStatus "idle" — both waits settle immediately + AtomicLong clock = new AtomicLong(1_000); + // bootstrapText left null so the DEFAULT sentence is built — from the RESOLVED path + FleetConfig.LeadRollover config = new FleetConfig.LeadRollover("handover.md", true, 3600, 20, 20, null); + Function leadWorkspace = terminal -> LEAD.equals(terminal) ? workspace.toString() : null; + LeadRollover rollover = newRollover(herdr, config, fixedClock(clock), leadWorkspace); + + LeadRollover.PendingRollover pending = rollover.open(LEAD, "context is full"); + LeadRollover.RollDecision decision = rollover.confirm(LEAD, pending.token(), true); + + assertTrue(decision.accepted(), "expected approval; got: " + decision.reason() + " / " + decision.detail()); + var prompts = herdr.calls.stream().filter(c -> "agent.prompt".equals(c.method())).toList(); + assertEquals(2, prompts.size(), "expected exactly two agent.prompt calls: /clear then the " + + "default bootstrapText"); + String bootstrapSent = prompts.get(1).params().toString(); + assertTrue(bootstrapSent.contains(expected.toString()), + "the default bootstrapText actually sent to the pane must name the RESOLVED " + + "absolute path (" + expected + ") — a fresh session's own pane cannot " + + "resolve a relative path against a directory it never had. Actual text " + + "sent: " + bootstrapSent); + assertFalse(bootstrapSent.contains("\"handover.md\""), + "must not name the raw relative configured value in the text actually sent"); + } } diff --git a/fleetd/src/test/java/dev/ltms/fleet/mcp/FleetMcpHandoverTest.java b/fleetd/src/test/java/dev/ltms/fleet/mcp/FleetMcpHandoverTest.java index 5ca8961..ed0b8ef 100644 --- a/fleetd/src/test/java/dev/ltms/fleet/mcp/FleetMcpHandoverTest.java +++ b/fleetd/src/test/java/dev/ltms/fleet/mcp/FleetMcpHandoverTest.java @@ -68,7 +68,9 @@ class FleetMcpHandoverTest { } private LeadRollover newRollover(String handoverPath) { - return new LeadRollover(agents, () -> cfg(handoverPath)); + // Every handoverPath this test class uses comes from tmp.resolve(...), which is already + // absolute, so the workspace lookup is never actually consulted — a no-op lookup is enough. + return new LeadRollover(agents, () -> cfg(handoverPath), _ -> null); } /** A fully wired FleetMcp on fakes (mirrors FleetMcpAuthzTest's helper), plus a leadRollover. */ From 042b8c99ddf17a091ce23fc0ee0e716514d230f1 Mon Sep 17 00:00:00 2001 From: Dai Ha Date: Sat, 12 Sep 2026 05:32:25 +0700 Subject: [PATCH 2/3] fleetd #480 follow-up correction: cover Fleetd.leadRollover(...)'s own wiring behaviourally Add FleetdLeadRolloverWorkspaceLookupTest, calling the package-private Fleetd.leadRollover(...) factory directly (with a real ConfigRef built from a temp fleetd.yaml, never the gitignored live one) to prove the terminal -> lead-name -> Leader.cwd() lookup it builds actually works: a relative handoverPath resolves against the calling lead's configured cwd; a terminal absent from the live lead-terminal map falls back to user.dir; and the lookup is read live, not snapshotted at construction time (a lead discovered by the tab scan after leadRollover(...) was built still resolves correctly). Proved this closes the gap: mutating the factory's lambda body (String leadName = null;, always "no lead found", which forces the daemon-cwd fallback this ticket exists to fix) left the full 1669-test suite green before this commit. With the new test added, the same one-line mutation now fails 2 of its 3 cases; reverting it goes green again (3/3). Mutation applied/reverted only during verification and is not part of this commit (git diff on Fleetd.java is empty). FleetdLeadRolloverWiringTest's class javadoc corrected: it previously claimed no behavioural test could catch this wiring dropping out, which was true only before this commit and only covered the factory's own body, not its call site. Restated what each test class actually covers: the source- text pin covers the call site's argument list; the new behavioural test covers the lambda's body. --- .../fleet/FleetdLeadRolloverWiringTest.java | 31 +++- ...FleetdLeadRolloverWorkspaceLookupTest.java | 163 ++++++++++++++++++ 2 files changed, 186 insertions(+), 8 deletions(-) create mode 100644 fleetd/src/test/java/dev/ltms/fleet/FleetdLeadRolloverWorkspaceLookupTest.java 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"); + } +} From 261aa056f9f0be9d5dae14555f11c82d28f4e70a Mon Sep 17 00:00:00 2001 From: Dai Ha Date: Sat, 12 Sep 2026 05:41:40 +0700 Subject: [PATCH 3/3] fleetd #480 follow-up correction 2: guarantee resolveHandoverPath is always absolute MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit LeadRollover.resolveHandoverPath's relative branch resolved the configured handoverPath against leadWorkspace.apply(...) (fleet.leaders..cwd) but never forced the result absolute. If an operator writes a RELATIVE cwd, the returned path stays relative, silently breaking the "always absolute" contract documented on PendingRollover. Fix: call toAbsolutePath() unconditionally on both branches (the already-absolute input branch, where it is a no-op, and the relative branch), so neither branch trusts isAbsolute() alone to already imply what toAbsolutePath() enforces. Method javadoc now states the absolute result is guaranteed, not merely usual. Added a test: a lead with a RELATIVE cwd and a relative handoverPath still yields an absolute PendingRollover.handoverPath. Asserts both isAbsolute() and the exact resolved value, since isAbsolute() alone would also pass for a path resolved against the wrong base. Proved the test discriminates: reverting the toAbsolutePath() calls (keeping the test) made it fail with an AssertionFailedError ("expected: but was: "); restoring the fix made it pass again. Note: FleetConfig has no validation on fleet.leaders..cwd at config load (grep across every validate* method: 0 matches for .cwd()) — a relative cwd is silently accepted. Not adding validation here per instruction; that is a separate ticket. --- .../dev/ltms/fleet/lead/LeadRollover.java | 24 +++++++++++++-- .../dev/ltms/fleet/lead/LeadRolloverTest.java | 30 +++++++++++++++++++ 2 files changed, 51 insertions(+), 3 deletions(-) diff --git a/fleetd/src/main/java/dev/ltms/fleet/lead/LeadRollover.java b/fleetd/src/main/java/dev/ltms/fleet/lead/LeadRollover.java index f43c3c8..29cbb0c 100644 --- a/fleetd/src/main/java/dev/ltms/fleet/lead/LeadRollover.java +++ b/fleetd/src/main/java/dev/ltms/fleet/lead/LeadRollover.java @@ -270,24 +270,42 @@ public final class LeadRollover { * FleetConfig.LeadRollover#bootstrapTextFor}) ever has to resolve — or worse, silently * mis-resolve — a relative path again. * + *

    The return value is GUARANTEED absolute, not merely usually absolute. + * {@code leadWorkspace.apply(leadTerminal)} returns an operator-configured {@code + * fleet.leaders..cwd} string, and nothing forces an operator to write an absolute one — + * a relative {@code cwd} resolved with plain {@link Path#resolve} would still yield a relative + * result, silently reopening the exact bug this class exists to fix (every later reader back to + * interpreting an ambiguous string against ITS OWN working directory). {@link + * Path#toAbsolutePath()} closes that: it resolves any remaining relative path against {@code + * user.dir} (the JVM's own cwd), which is the correct base for an operator-written path the + * daemon process itself is meant to interpret, exactly like the {@code user.dir} fallback used + * below. Applying it unconditionally on both branches means the ALREADY-absolute branch stays a + * no-op (an absolute path is unaffected by {@code toAbsolutePath()}) while the relative-{@code + * cwd} branch above is closed the same way. + * *

      *
    • already absolute → returned unchanged (normalized)
      • *
    • relative → resolved against {@code leadWorkspace.apply(leadTerminal)} when that is * non-null and non-blank; otherwise against {@code System.getProperty("user.dir")} — the * same fallback {@code LeadLauncher#launch} uses for a lead with no configured {@code - * cwd}
      • + * cwd}. If {@code leadWorkspace}'s own answer is itself relative (an operator wrote a + * relative {@code cwd:}), the result is finished off against the daemon's own + * {@code user.dir} — see the paragraph above. *
    */ private String resolveHandoverPath(String configured, String leadTerminal) { Path path = Path.of(configured); if (path.isAbsolute()) { - return path.normalize().toString(); + // toAbsolutePath() is a no-op for an already-absolute path — kept here anyway so both + // branches call the exact same guarantee, rather than one branch relying on + // isAbsolute() alone to already imply what toAbsolutePath() enforces. + return path.toAbsolutePath().normalize().toString(); } String workspace = leadWorkspace.apply(leadTerminal); Path base = (workspace == null || workspace.isBlank()) ? Path.of(System.getProperty("user.dir")) : Path.of(workspace); - return base.resolve(path).normalize().toString(); + return base.resolve(path).toAbsolutePath().normalize().toString(); } /** diff --git a/fleetd/src/test/java/dev/ltms/fleet/lead/LeadRolloverTest.java b/fleetd/src/test/java/dev/ltms/fleet/lead/LeadRolloverTest.java index d2f56ff..8d69d92 100644 --- a/fleetd/src/test/java/dev/ltms/fleet/lead/LeadRolloverTest.java +++ b/fleetd/src/test/java/dev/ltms/fleet/lead/LeadRolloverTest.java @@ -575,6 +575,36 @@ class LeadRolloverTest { "an absolute handoverPath must be used unchanged (aside from normalization)"); } + @Test + @DisplayName("[fleetd #480 follow-up correction] a RELATIVE fleet.leaders..cwd still " + + "yields an ABSOLUTE PendingRollover.handoverPath") + void relativeLeadWorkspaceCwdStillYieldsAnAbsoluteHandoverPath() { + FakeHerdr herdr = new FakeHerdr(); + AtomicLong clock = new AtomicLong(1_000); + FleetConfig.LeadRollover config = cfg("handover.md"); // relative handoverPath + // Nothing in FleetConfig validates fleet.leaders..cwd, so an operator can write a + // RELATIVE one — this must still resolve to an absolute PendingRollover.handoverPath, + // never silently reopen the exact bug this class fixes. + String relativeWorkspace = "relative-lead-workspace"; + Function leadWorkspace = terminal -> LEAD.equals(terminal) ? relativeWorkspace : null; + LeadRollover rollover = newRollover(herdr, config, fixedClock(clock), leadWorkspace); + + LeadRollover.PendingRollover pending = rollover.open(LEAD, "context is full"); + + assertTrue(Path.of(pending.handoverPath()).isAbsolute(), + "the resolved path must be absolute even when the configured cwd itself is relative"); + + // Asserting isAbsolute() alone would also pass for a path resolved against the WRONG base + // (e.g. some unrelated absolute directory) — pin the actual value too. + String expected = Path.of(System.getProperty("user.dir")).resolve(relativeWorkspace) + .resolve("handover.md").toAbsolutePath().normalize().toString(); + assertEquals(expected, pending.handoverPath(), + "a relative cwd must be finished off against the daemon's own user.dir, exactly " + + "like a missing cwd — never left relative, which would silently reopen the " + + "exact bug this class fixes: every later reader interpreting an ambiguous " + + "path against ITS OWN working directory"); + } + @Test @DisplayName("[fleetd #480 follow-up] a terminal with no configured workspace (null lookup " + "result) falls back to the daemon's own user.dir")