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. + * + *

    + */ + 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. */