From 4bfab6b71805153d4aa9a14577e49af70ecee4d3 Mon Sep 17 00:00:00 2001 From: Dai Ha Date: Sat, 12 Sep 2026 05:19:49 +0700 Subject: [PATCH] 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. */