fleet/fleetd
3
0
Fork 0
Code Issues 73 Pull Requests Actions Packages Projects Releases 1 Wiki Activity

Merge #487: resolve a relative leadRollover.handoverPath against the calling lead's workspace (fleetd #480 follow-up)

Browse Source
This commit is contained in:
Dai Ha
2026-09-12 05:43:12 +07:00
parent 008a457557 261aa056f9
commit 3bf3968bc7
8 changed files with 569 additions and 44 deletions
Download Patch File Download Diff File Expand all files Collapse all files
fleetd/fleetd.example.yaml
+8 -3
View File
@@ -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.<name>.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
fleetd/src/main/java/dev/ltms/fleet/Fleetd.java
+33 -9
View File
@@ -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
* <p><strong>fleetd #480 follow-up:</strong> 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<Map<String, String>> liveLeadTerminals) {
if (cfg.leadRollover() == null) {
return null;
}
return new LeadRollover(leadAgents, () -> config.get().leadRollover());
Function<String, String> 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);
}
/**
fleetd/src/main/java/dev/ltms/fleet/config/FleetConfig.java
+33 -8
View File
@@ -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.<name>.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.";
}
}
fleetd/src/main/java/dev/ltms/fleet/lead/LeadRollover.java
+97 -9
View File
@@ -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;
* <li>{@code agents.send(lead, "/clear")}</li>
* <li>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)</li>
* <li>{@code agents.send(lead, cfg.bootstrapText())}</li>
* <li>{@code agents.send(lead, cfg.bootstrapTextFor(p.handoverPath()))}</li>
* </ol>
* A {@link #confirm} that returns {@link RollDecision#approved()} therefore means <em>"every gate
* passed and the roll is scheduled"</em>, never <em>"the pane has been cleared"</em> — 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.</strong> The later MCP-tool unit that wires
* {@link #open}/{@link #confirm} must pass the resolved caller terminal, not a request field.
*
* <p><strong>A relative {@code handoverPath} resolves against the CALLING lead's workspace, never
* the daemon's own cwd — a fleetd #480 follow-up.</strong> 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.<name>.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<FleetConfig.LeadRollover> configSupplier;
/**
* Terminal id → that lead's configured workspace directory (their {@code
* fleet.leaders.<name>.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<String, String> leadWorkspace;
private final LongSupplier nowMillis;
private final Runnable settleSleeper;
/**
@@ -160,8 +189,9 @@ public final class LeadRollover {
private final Map<String, PendingRollover> pending = new ConcurrentHashMap<>();
/** Production constructor — wall clock, real sleep between settle polls, a real virtual thread. */
public LeadRollover(AgentControl agents, Supplier<FleetConfig.LeadRollover> configSupplier) {
this(agents, configSupplier, System::currentTimeMillis,
public LeadRollover(AgentControl agents, Supplier<FleetConfig.LeadRollover> configSupplier,
Function<String, String> 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<FleetConfig.LeadRollover> configSupplier,
LongSupplier nowMillis, Runnable settleSleeper, Consumer<Runnable> continuationRunner) {
Function<String, String> leadWorkspace, LongSupplier nowMillis,
Runnable settleSleeper, Consumer<Runnable> continuationRunner) {
this.agents = agents;
this.configSupplier = configSupplier;
this.leadWorkspace = leadWorkspace;
this.nowMillis = nowMillis;
this.settleSleeper = settleSleeper;
this.continuationRunner = continuationRunner;
@@ -215,13 +247,67 @@ 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.
*
* <p><strong>The return value is GUARANTEED absolute, not merely usually absolute.</strong>
* {@code leadWorkspace.apply(leadTerminal)} returns an operator-configured {@code
* fleet.leaders.<name>.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.
*
* <ul>
* <li>already absolute → returned unchanged (normalized)</li>
* <li>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}. 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.</li>
* </ul>
*/
private String resolveHandoverPath(String configured, String leadTerminal) {
Path path = Path.of(configured);
if (path.isAbsolute()) {
// 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).toAbsolutePath().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. <strong>This method
@@ -302,7 +388,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 +399,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
*/
fleetd/src/test/java/dev/ltms/fleet/FleetdLeadRolloverWiringTest.java
+29 -12
View File
@@ -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).
*
* <p>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.
* <p>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.
*
* <p><b>This test checks source text, not runtime behaviour.</b> 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.
*
* <p><b>Correction (fleetd #480 relative-handover-path follow-up): that gap used to be real, and
* now is not — but not here.</b> 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 {
@@ -49,14 +64,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
fleetd/src/test/java/dev/ltms/fleet/FleetdLeadRolloverWorkspaceLookupTest.java
+163
View File
@@ -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.
*
* <p>{@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<String,String>} 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.
*
* <p>{@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).
*
* <p><b>Proved against the mutation it exists to catch.</b> 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<String, String> 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");
}
}
fleetd/src/test/java/dev/ltms/fleet/lead/LeadRolloverTest.java
+203 -2
View File
@@ -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<String, String> 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,199 @@ 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<String, String> 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<String, String> 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<String, String> 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<String, String> 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 correction] a RELATIVE fleet.leaders.<name>.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.<name>.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<String, String> 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")
void noConfiguredWorkspaceFallsBackToUserDir() {
FakeHerdr herdr = new FakeHerdr();
AtomicLong clock = new AtomicLong(1_000);
FleetConfig.LeadRollover config = cfg("some-handover.md");
Function<String, String> 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<String, String> 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<String, String> 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");
}
}
fleetd/src/test/java/dev/ltms/fleet/mcp/FleetMcpHandoverTest.java
+3 -1
View File
@@ -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. */