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.<name>.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.
This commit is contained in:
@@ -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
|
||||
|
||||
@@ -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);
|
||||
}
|
||||
|
||||
/**
|
||||
|
||||
@@ -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.";
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -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,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.
|
||||
*
|
||||
* <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}</li>
|
||||
* </ul>
|
||||
*/
|
||||
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. <strong>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
|
||||
*/
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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,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<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] 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");
|
||||
}
|
||||
}
|
||||
|
||||
@@ -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. */
|
||||
|
||||
Reference in New Issue
Block a user