From 62646957ea494e59a9b0319f9fc0b8a0a984f24d Mon Sep 17 00:00:00 2001 From: Dai Ha Date: Fri, 11 Sep 2026 07:07:24 +0700 Subject: [PATCH 1/2] fleetd #480 Unit C: wire fleet_handover MCP tool onto LeadRollover MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds the fleet_handover tool (open/confirm/cancel) as a thin adapter over LeadRollover, registered unconditionally so the charter tool-surface gate sees a stable set regardless of whether leadRollover: is configured. With a null LeadRollover every action degrades to a clean NOT_CONFIGURED refusal instead of throwing. Gated on a new Authz.Action.HANDOVER (primary-only, same as SPAWN/STOP/DRAIN). The caller's own connection-resolved terminal is the only lead identity ever used — the tool's input schema carries no terminal/session/leadTerminal parameter, so a lead can only ever roll itself. Fleetd.main now passes its existing leadRollover local into FleetMcp via a new trailing constructor parameter. --- .../src/main/java/dev/ltms/fleet/Fleetd.java | 5 +- .../main/java/dev/ltms/fleet/auth/Authz.java | 13 +- .../java/dev/ltms/fleet/mcp/FleetMcp.java | 204 +++++++++++++- .../java/dev/ltms/fleet/mcp/FleetTool.java | 3 +- .../ltms/fleet/mcp/FleetMcpHandoverTest.java | 259 ++++++++++++++++++ 5 files changed, 477 insertions(+), 7 deletions(-) create mode 100644 fleetd/src/test/java/dev/ltms/fleet/mcp/FleetMcpHandoverTest.java diff --git a/fleetd/src/main/java/dev/ltms/fleet/Fleetd.java b/fleetd/src/main/java/dev/ltms/fleet/Fleetd.java index 3e8fe3c..ae2e929 100644 --- a/fleetd/src/main/java/dev/ltms/fleet/Fleetd.java +++ b/fleetd/src/main/java/dev/ltms/fleet/Fleetd.java @@ -711,7 +711,10 @@ public final class Fleetd { // reach. Read from the SAME snapshot leadMailbox itself opened from (cfg.coordinator()), // not the live config.get() — coordinator wiring is already boot-time-fixed (see // leadMailbox above), so peers follows the same rule rather than half hot-reloading. - cfg.coordinator() == null ? List.of() : cfg.coordinator().peers()); + cfg.coordinator() == null ? List.of() : cfg.coordinator().peers(), + // fleetd #480 Unit C: the executor behind fleet_handover — null whenever + // leadRollover: is not configured (see the leadRollover local above). + leadRollover); // CB-637: the receive half. Only constructed when a lead mailbox actually opened — with no // coordinator (or an unreachable one) there is nothing to deliver, so no scheduler is diff --git a/fleetd/src/main/java/dev/ltms/fleet/auth/Authz.java b/fleetd/src/main/java/dev/ltms/fleet/auth/Authz.java index 1c186e0..bc505b3 100644 --- a/fleetd/src/main/java/dev/ltms/fleet/auth/Authz.java +++ b/fleetd/src/main/java/dev/ltms/fleet/auth/Authz.java @@ -41,7 +41,16 @@ public final class Authz { */ COORD_READ, /** Scrape the metrics endpoint. */ - METRICS + METRICS, + /** + * Drive the lead-rollover executor ({@code fleet_handover}: open/confirm/cancel a + * self-replace, fleetd #480 Unit C). Primary-only, same as {@link #SPAWN}/{@link #STOP}/ + * {@link #DRAIN} — and, unlike those, the terminal it acts on is never even an argument: + * {@code LeadRollover#open}/{@code #confirm} are always called with the CALLER's own + * connection-resolved terminal (see {@code dev.ltms.fleet.lead.LeadRollover}'s class + * javadoc, fleetd #480 correction 2), so a primary can only ever roll itself. + */ + HANDOVER } /** @@ -60,7 +69,7 @@ public final class Authz { // deliberately does NOT get these (CB-548), so it cannot tear down or stand up workers // even though it coordinates them; and a worker driving any of these would be a worker // escalating into the orchestrator role. - case SPAWN, STOP, DRAIN -> caller.isPrimary(); + case SPAWN, STOP, DRAIN, HANDOVER -> caller.isPrimary(); // Delivering a turn is open to the primary and the architect: an architect delegates // to workers (that is the role's point) but still has no lifecycle rights. A worker is diff --git a/fleetd/src/main/java/dev/ltms/fleet/mcp/FleetMcp.java b/fleetd/src/main/java/dev/ltms/fleet/mcp/FleetMcp.java index 21d53cc..d566521 100644 --- a/fleetd/src/main/java/dev/ltms/fleet/mcp/FleetMcp.java +++ b/fleetd/src/main/java/dev/ltms/fleet/mcp/FleetMcp.java @@ -12,6 +12,7 @@ import dev.ltms.fleet.metrics.Metrics; import dev.ltms.fleet.inject.MemberPresence; import dev.ltms.fleet.inject.CompletionResolver; import dev.ltms.fleet.herdr.HerdrException; +import dev.ltms.fleet.lead.LeadRollover; import dev.ltms.fleet.msg.LeadChannel; import dev.ltms.fleet.msg.LeadMessage; import dev.ltms.fleet.msg.MessageService; @@ -105,6 +106,13 @@ public final class FleetMcp { private final LeadChannel leadChannel; /** fleetd #361: {@code coordinator.peers} — see {@link CoordinationSource}. Empty when unset. */ private final List peers; + /** + * fleetd #480 Unit C: the executor behind {@code fleet_handover}. {@code null} whenever + * {@code leadRollover:} is not configured — {@code fleet_handover} is still registered (see + * this class's javadoc on the charter tool-surface gate), and every action then degrades to a + * clean {@code NOT_CONFIGURED} refusal rather than throwing. See {@link #handover}. + */ + private final LeadRollover leadRollover; /** Capacity facts used by {@code fleet_list}; production must supply the placement live count. */ public record CapacitySource(Function liveCount, Function maxLoad, @@ -301,8 +309,7 @@ public final class FleetMcp { } /** - * As above, with fleetd #361 {@code coordinator.peers} (see {@link CoordinationSource}). This is - * what {@code Fleetd.main} actually wires up. + * As above, with fleetd #361 {@code coordinator.peers} (see {@link CoordinationSource}). * * @param leadSeats required — pass {@link LeadSeatSource#none()} for a caller that does not want * the feature, never a defaulting overload (the same rule {@code quarantine} and @@ -315,6 +322,27 @@ public final class FleetMcp { CallerResolver callers, Metrics metrics, CapacitySource capacity, HealthCoverageSource healthCoverage, QuarantineSource quarantine, LeadChannel leadChannel, OutageSource outage, LeadSeatSource leadSeats, List peers) { + this(messages, workers, sessions, identity, presence, primaryRegistry, callers, metrics, capacity, + healthCoverage, quarantine, leadChannel, outage, leadSeats, peers, null); + } + + /** + * As above, with fleetd #480 Unit C: the {@link LeadRollover} executor behind + * {@code fleet_handover}. This is what {@code Fleetd.main} actually wires up. + * + * @param leadRollover {@code null} whenever {@code leadRollover:} is not configured — an + * upgraded daemon must never silently acquire the ability to clear a lead's + * own pane (mirrors {@code Fleetd.leadRollover(...)}'s own construction + * gate). {@code fleet_handover} is registered unconditionally either way — + * see this class's javadoc and fleetd #474's charter tool-surface gate — + * and every action degrades to a clean refusal naming {@code NOT_CONFIGURED} + * instead of throwing. See {@link #handover}. + */ + public FleetMcp(MessageService messages, PeerLauncher workers, SessionManager sessions, + ConnectionIdentity identity, MemberPresence presence, PrimaryRegistry primaryRegistry, + CallerResolver callers, Metrics metrics, CapacitySource capacity, HealthCoverageSource healthCoverage, + QuarantineSource quarantine, LeadChannel leadChannel, OutageSource outage, + LeadSeatSource leadSeats, List peers, LeadRollover leadRollover) { this.leadChannel = leadChannel; this.peers = peers == null ? List.of() : List.copyOf(peers); this.capacity = capacity; @@ -322,6 +350,7 @@ public final class FleetMcp { this.outage = Objects.requireNonNull(outage, "outage"); this.leadSeats = Objects.requireNonNull(leadSeats, "leadSeats"); this.healthCoverage = healthCoverage; + this.leadRollover = leadRollover; McpJsonMapper json = new JacksonMcpJsonMapperSupplier().get(); this.transport = HttpServletStreamableServerTransportProvider.builder() .jsonMapper(json) @@ -477,6 +506,15 @@ public final class FleetMcp { if (denied != null) return denied; return whoami(principal(exchange), sessions); }; + // fleetd #480 Unit C: fleet_handover. No terminal/session argument at all — the lead pane + // to roll is ALWAYS the caller's own connection-resolved terminal (never a request field), + // per LeadRollover's class javadoc (fleetd #480 correction 2). + BiFunction handoverHandler = + (exchange, req) -> { + McpSchema.CallToolResult denied = deny(exchange, toolAction("fleet_handover", req.arguments()), null); + if (denied != null) return denied; + return handover(leadRollover, callerTerminal(exchange), req.arguments()); + }; McpSchema.Tool fleetSend = sendTool(); McpSchema.Tool fleetReply = replyTool(); @@ -489,6 +527,7 @@ public final class FleetMcp { McpSchema.Tool fleetStop = stopTool(); McpSchema.Tool fleetProfiles = profilesTool(); McpSchema.Tool fleetWhoami = whoamiTool(); + McpSchema.Tool fleetHandover = handoverTool(); // fleetd #469: the tool schemas above are already named from FleetTool.wireName(), but // this is the check that a schema was not accidentally dropped, duplicated, or added @@ -498,7 +537,8 @@ public final class FleetMcp { // startup failure, not a silent gap. Set registeredToolNames = Set.of(fleetSend.name(), fleetReply.name(), fleetAsk.name(), fleetStatus.name(), fleetPoll.name(), fleetAck.name(), fleetSpawn.name(), - fleetList.name(), fleetStop.name(), fleetProfiles.name(), fleetWhoami.name()); + fleetList.name(), fleetStop.name(), fleetProfiles.name(), fleetWhoami.name(), + fleetHandover.name()); if (!registeredToolNames.equals(FleetTool.wireNames())) { throw new IllegalStateException("fleetd #469: registered MCP tools " + registeredToolNames + " do not match the canonical tool set " + FleetTool.wireNames() @@ -519,6 +559,7 @@ public final class FleetMcp { .toolCall(fleetStop, stopHandler) .toolCall(fleetProfiles, profilesHandler) .toolCall(fleetWhoami, whoamiHandler) + .toolCall(fleetHandover, handoverHandler) .build(); this.authz = callers; this.metrics = metrics; @@ -683,6 +724,19 @@ public final class FleetMcp { server.closeGracefully(); } + /** + * The tools this server has actually registered with the MCP SDK, wire schema included. + * + *

Package-private, for tests that must check the REGISTERED schema rather than this + * class's own source text — e.g. proving {@code fleet_handover}'s input schema carries no + * caller-terminal parameter (fleetd #480 Unit C acceptance criterion 4). A source-text scrape + * cannot tell "the schema builder omits this key" apart from "a typo means it never runs" the + * way asking the constructed server itself can. + */ + List registeredTools() { + return server.listTools(); + } + // --- tool logic (thin adapters over the services; unit-testable) --------------------------- /** @@ -940,6 +994,7 @@ public final class FleetMcp { case ACK -> Authz.Action.DRAIN; case SPAWN -> Authz.Action.SPAWN; case STOP -> Authz.Action.STOP; + case HANDOVER -> Authz.Action.HANDOVER; }; } @@ -1164,6 +1219,119 @@ public final class FleetMcp { return text(json(m)); } + /** + * {@code fleet_handover} (fleetd #480 Unit C): drive {@link LeadRollover#open}/ + * {@link LeadRollover#confirm}/{@link LeadRollover#cancel} from a tool call. + * + *

{@code callerTerminal} is the CALLING lead's terminal id, resolved by the MCP layer from + * the connection (see {@link #callerTerminal(McpSyncServerExchange)}) — never a request field. + * This is why this tool's input schema ({@link #handoverTool}) carries no terminal/session/ + * leadTerminal parameter of any kind: a lead can only ever open or confirm a rollover of its + * OWN pane (see {@code LeadRollover}'s class javadoc, fleetd #480 correction 2). + * + *

{@code leadRollover} is {@code null} whenever {@code leadRollover:} is not configured. + * This tool is registered unconditionally regardless (see this class's javadoc on the fleetd + * #474 charter tool-surface gate), so every action here must degrade to a clean, structured + * refusal naming {@code NOT_CONFIGURED} rather than ever throwing. + */ + static McpSchema.CallToolResult handover(LeadRollover leadRollover, String callerTerminal, + Map args) { + String action = str(args, "action"); + if (isBlank(action)) { + return error("action is required: \"open\", \"confirm\" or \"cancel\""); + } + return switch (action) { + case "open" -> handoverOpen(leadRollover, callerTerminal, str(args, "reason")); + case "confirm" -> handoverConfirm(leadRollover, callerTerminal, str(args, "token"), + truthy(args, "operatorConfirmed")); + case "cancel" -> handoverCancel(leadRollover, str(args, "token")); + default -> error("unknown action \"" + action + "\" — must be \"open\", \"confirm\" or \"cancel\""); + }; + } + + /** + * {@code action: "open"}. On the null-{@code leadRollover} path (not configured) and on the + * config-removed-since-construction path ({@link LeadRollover#open} itself throws {@link + * IllegalStateException} for that), both degrade to the same clean {@code NOT_CONFIGURED} + * refusal — never an escaping exception. + */ + private static McpSchema.CallToolResult handoverOpen(LeadRollover leadRollover, String callerTerminal, + String reason) { + if (leadRollover == null) { + return notConfigured(); + } + if (isBlank(callerTerminal)) { + // An unnamed primary (token/loopback path, no resolved pane) has nowhere for the + // eventual /clear + bootstrap to land — LeadRollover#open would throw + // IllegalArgumentException for the same reason; refuse cleanly here instead. + return error("fleet_handover requires a named lead pane (a resolved connection terminal) " + + "to open a rollover request against — an unnamed primary has none"); + } + try { + LeadRollover.PendingRollover p = leadRollover.open(callerTerminal, reason); + Map m = new LinkedHashMap<>(); + m.put("token", p.token()); + m.put("handoverPath", p.handoverPath()); + m.put("requestedAtMillis", p.requestedAtMillis()); + return text(json(m)); + } catch (IllegalStateException e) { + // leadRollover: was removed from config by a hot reload since this FleetMcp was + // constructed — same clean refusal as the null-at-construction case above. + return notConfigured(); + } + } + + /** {@code action: "confirm"}. Surfaces every {@link LeadRollover.RefusalReason} verbatim. */ + private static McpSchema.CallToolResult handoverConfirm(LeadRollover leadRollover, String callerTerminal, + String token, boolean operatorConfirmed) { + if (leadRollover == null) { + return refusalJson(false, "NOT_CONFIGURED", "leadRollover: is not configured"); + } + if (isBlank(token)) { + return error("token is required for action \"confirm\""); + } + LeadRollover.RollDecision d = leadRollover.confirm(callerTerminal, token, operatorConfirmed); + return refusalJson(d.accepted(), d.reason() == null ? null : d.reason().name(), d.detail()); + } + + /** {@code action: "cancel"}. An unknown token is a clean "no pending request", never an error. */ + private static McpSchema.CallToolResult handoverCancel(LeadRollover leadRollover, String token) { + if (leadRollover == null) { + Map m = new LinkedHashMap<>(); + m.put("cancelled", false); + m.put("reason", "NOT_CONFIGURED"); + m.put("detail", "leadRollover: is not configured"); + return text(json(m)); + } + if (isBlank(token)) { + return error("token is required for action \"cancel\""); + } + boolean existed = leadRollover.cancel(token); + Map m = new LinkedHashMap<>(); + m.put("cancelled", existed); + if (!existed) { + m.put("detail", "no pending request for token " + token); + } + return text(json(m)); + } + + /** The one shared {@code NOT_CONFIGURED} refusal shape for {@code open}/{@code confirm}. */ + private static McpSchema.CallToolResult notConfigured() { + return refusalJson(false, "NOT_CONFIGURED", "leadRollover: is not configured"); + } + + private static McpSchema.CallToolResult refusalJson(boolean accepted, String reason, String detail) { + Map m = new LinkedHashMap<>(); + m.put("accepted", accepted); + if (reason != null) { + m.put("reason", reason); + } + if (detail != null) { + m.put("detail", detail); + } + return text(json(m)); + } + // --- fleet management logic (CB-108 / CB-301) -------------------------------------------- /** {@code fleet_spawn} without cwd/caller context (default resolution). */ @@ -2051,6 +2219,31 @@ public final class FleetMcp { objectSchema(Map.of(), List.of())); } + private static McpSchema.Tool handoverTool() { + return tool(FleetTool.HANDOVER.wireName(), + "Replace your OWN lead session once its context is full: write a handover file, " + + "then use this to have fleetd clear your pane and bootstrap a fresh lead " + + "session against it. Three actions: 'open' (requests a token and the " + + "handoverPath you must write the handover file to before confirming), " + + "'confirm' (validates every gate and — only if every one passes — schedules " + + "the roll; it does NOT itself clear the pane, the roll runs once this call's " + + "own turn ends), and 'cancel' (drops a pending request without rolling). " + + "Primary-only. There is deliberately no terminal/session/leadTerminal " + + "parameter: the pane to roll is always resolved from YOUR OWN connection, " + + "never a value you pass, so you can only ever roll yourself — never another " + + "lead. Requires leadRollover: to be configured; when it is not, every action " + + "returns a clean refusal naming NOT_CONFIGURED instead of failing.", + objectSchema(Map.of( + "action", stringProp("\"open\", \"confirm\" or \"cancel\""), + "reason", stringProp("Free-text audit note for \"open\" (optional, logged only)"), + "token", stringProp("The token \"open\" returned — required for \"confirm\" and \"cancel\""), + "operatorConfirmed", Map.of("type", "boolean", + "description", "For \"confirm\": your answer to \"has the human operator " + + "confirmed this wipe\" (default false; only consulted when " + + "leadRollover.requireOperatorConfirm is true)")), + List.of("action"))); + } + // --- small helpers ------------------------------------------------------------------------- // The SDK 2.0.0 deprecates its own Tool builders without a stable replacement — isolate it here. @@ -2085,6 +2278,11 @@ public final class FleetMcp { return v instanceof Number n ? n.longValue() : null; } + /** {@code true} only when {@code args.get(key)} is the boolean {@code true} — absent/null/anything else is {@code false}. */ + private static boolean truthy(Map args, String key) { + return Boolean.TRUE.equals(args.get(key)); + } + private static long clamp(long ms) { return Math.clamp(ms, 1, MAX_TIMEOUT_MS); } diff --git a/fleetd/src/main/java/dev/ltms/fleet/mcp/FleetTool.java b/fleetd/src/main/java/dev/ltms/fleet/mcp/FleetTool.java index de54be1..e5fa652 100644 --- a/fleetd/src/main/java/dev/ltms/fleet/mcp/FleetTool.java +++ b/fleetd/src/main/java/dev/ltms/fleet/mcp/FleetTool.java @@ -43,7 +43,8 @@ public enum FleetTool { LIST("fleet_list"), STOP("fleet_stop"), PROFILES("fleet_profiles"), - WHOAMI("fleet_whoami"); + WHOAMI("fleet_whoami"), + HANDOVER("fleet_handover"); private final String wireName; diff --git a/fleetd/src/test/java/dev/ltms/fleet/mcp/FleetMcpHandoverTest.java b/fleetd/src/test/java/dev/ltms/fleet/mcp/FleetMcpHandoverTest.java new file mode 100644 index 0000000..5ca8961 --- /dev/null +++ b/fleetd/src/test/java/dev/ltms/fleet/mcp/FleetMcpHandoverTest.java @@ -0,0 +1,259 @@ +package dev.ltms.fleet.mcp; + +import dev.ltms.fleet.auth.Authz; +import dev.ltms.fleet.auth.CallerResolver; +import dev.ltms.fleet.auth.MemberRegistry; +import dev.ltms.fleet.auth.Principal; +import dev.ltms.fleet.config.FleetConfig; +import dev.ltms.fleet.guard.SubscriptionGuard; +import dev.ltms.fleet.herdr.AgentControl; +import dev.ltms.fleet.herdr.FakeHerdr; +import dev.ltms.fleet.herdr.PaneLocator; +import dev.ltms.fleet.herdr.WorkspaceControl; +import dev.ltms.fleet.inject.Injector; +import dev.ltms.fleet.lead.LeadRollover; +import dev.ltms.fleet.member.ClaudeCodeLauncher; +import dev.ltms.fleet.msg.InMemoryReplyInbox; +import dev.ltms.fleet.msg.MessageService; +import dev.ltms.fleet.msg.Rendezvous; +import dev.ltms.fleet.session.FakeWorktrees; +import dev.ltms.fleet.session.SessionManager; +import io.modelcontextprotocol.spec.McpSchema; +import org.junit.jupiter.api.AfterEach; +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.List; +import java.util.Map; +import java.util.Set; + +import static org.junit.jupiter.api.Assertions.*; + +/** + * fleetd #480 Unit C — the {@code fleet_handover} MCP tool, the surface that finally calls + * {@link LeadRollover#open}/{@link LeadRollover#confirm}/{@link LeadRollover#cancel}. + * + *

Uses {@link LeadRollover}'s PUBLIC constructor (real wall clock, real 250ms settle poll, a + * real virtual-thread continuation runner) rather than its package-private test constructor — + * this test lives in {@code dev.ltms.fleet.mcp}, not {@code dev.ltms.fleet.lead}, and does not + * need to control the post-{@code confirm()} continuation's timing: it only asserts the + * SYNCHRONOUS return value of {@code open}/{@code confirm}/{@code cancel}, which is exactly what + * {@code FleetMcp.handover} forwards to the client. {@code turnSettleSeconds}/{@code + * clearSettleSeconds} are kept at 1s so a confirmed request's background continuation (which this + * class does not wait on or assert against) gives up quickly rather than polling for 20s on a + * daemon virtual thread. + */ +class FleetMcpHandoverTest { + + @TempDir + Path tmp; + + private static final String LEAD = "term_lead"; + private static final String OTHER_LEAD = "term_other_lead"; + + private final FakeHerdr herdr = new FakeHerdr(); + private final AgentControl agents = new AgentControl(herdr); + private FleetMcp mcp; + + @AfterEach + void close() { + if (mcp != null) mcp.close(); + } + + private static FleetConfig.LeadRollover cfg(String handoverPath) { + return new FleetConfig.LeadRollover(handoverPath, false, 3600, 1, 1, "read the handover file"); + } + + private LeadRollover newRollover(String handoverPath) { + return new LeadRollover(agents, () -> cfg(handoverPath)); + } + + /** A fully wired FleetMcp on fakes (mirrors FleetMcpAuthzTest's helper), plus a leadRollover. */ + private FleetMcp mcp(LeadRollover leadRollover) { + FleetConfig.Profile pcfg = new FleetConfig.Profile( + "ltms-local", "http://gx00.gw:8000", "coder", null, "FLEETD_WORKER_TOKEN", null, + "tab", "fleetd-workers", "worker: {profile} #{n}", null, null, null); + ClaudeCodeLauncher workers = new ClaudeCodeLauncher(agents, new WorkspaceControl(herdr), + new SubscriptionGuard(Set.of("gx00.gw")), Map.of(pcfg.profile(), pcfg), pcfg.profile(), + _ -> "tok"); + SessionManager sessions = new SessionManager(workers, new FakeWorktrees()); + MessageService messages = new MessageService(agents, new Injector(agents), new Rendezvous(), + new InMemoryReplyInbox()); + ConnectionIdentity identity = new ConnectionIdentity(new PaneLocator(herdr), _ -> 999_999); + + mcp = new FleetMcp(messages, workers, sessions, identity, sessions.asPresence(), + new PrimaryRegistry(null), + CallerResolver.withLeadsAndMembers(identity, false, null, Map::of, new MemberRegistry(null)), + null, FleetMcp.CapacitySource.none(), new FleetMcp.HealthCoverageSource(() -> "off"), + FleetMcp.QuarantineSource.none(), null, FleetMcp.OutageSource.none(), + FleetMcp.LeadSeatSource.none(), List.of(), leadRollover); + return mcp; + } + + private static String textOf(McpSchema.CallToolResult r) { + return ((McpSchema.TextContent) r.content().getFirst()).text(); + } + + private static String extractToken(String json) { + int i = json.indexOf("\"token\":\""); + assertTrue(i >= 0, "no token field in: " + json); + int start = i + "\"token\":\"".length(); + int end = json.indexOf('"', start); + return json.substring(start, end); + } + + // --- acceptance 1: registered whether or not leadRollover: is configured ------------------- + + @Test + @DisplayName("fleet_handover is registered whether or not leadRollover: is configured") + void registeredEitherWay() { + assertTrue(FleetTool.wireNames().contains("fleet_handover"), + "FleetTool must list fleet_handover as part of the canonical tool surface"); + + FleetMcp withNull = mcp(null); + assertTrue(withNull.registeredTools().stream().anyMatch(t -> "fleet_handover".equals(t.name())), + "fleet_handover must be registered even with no LeadRollover constructed"); + withNull.close(); + + FleetMcp withConfigured = mcp(newRollover(tmp.resolve("h.md").toString())); + assertTrue(withConfigured.registeredTools().stream().anyMatch(t -> "fleet_handover".equals(t.name())), + "fleet_handover must be registered when a LeadRollover IS constructed too"); + } + + // --- acceptance 2: null LeadRollover -> clean NOT_CONFIGURED, no throw, every action ------- + + @Test + @DisplayName("with leadRollover: absent, every action returns a clean NOT_CONFIGURED refusal and never throws") + void nullLeadRolloverRefusesCleanlyForEveryAction() { + McpSchema.CallToolResult open = assertDoesNotThrow( + () -> FleetMcp.handover(null, LEAD, Map.of("action", "open"))); + assertFalse(open.isError(), "a refusal is not a protocol error: " + textOf(open)); + assertTrue(textOf(open).contains("NOT_CONFIGURED"), textOf(open)); + + McpSchema.CallToolResult confirm = assertDoesNotThrow(() -> FleetMcp.handover(null, LEAD, + Map.of("action", "confirm", "token", "whatever"))); + assertFalse(confirm.isError()); + assertTrue(textOf(confirm).contains("NOT_CONFIGURED"), textOf(confirm)); + + McpSchema.CallToolResult cancel = assertDoesNotThrow(() -> FleetMcp.handover(null, LEAD, + Map.of("action", "cancel", "token", "whatever"))); + assertFalse(cancel.isError()); + assertTrue(textOf(cancel).contains("NOT_CONFIGURED"), textOf(cancel)); + } + + @Test + @DisplayName("a blank/unknown action is a clean tool error, never an exception") + void unknownActionIsACleanError() { + McpSchema.CallToolResult missing = assertDoesNotThrow(() -> FleetMcp.handover(null, LEAD, Map.of())); + assertTrue(missing.isError()); + + McpSchema.CallToolResult bogus = assertDoesNotThrow( + () -> FleetMcp.handover(null, LEAD, Map.of("action", "bogus"))); + assertTrue(bogus.isError()); + } + + // --- acceptance 3: non-primary refused by the authorization gate --------------------------- + + @Test + @DisplayName("fleet_handover maps to Authz.Action.HANDOVER, primary-only") + void onlyThePrimaryIsPermitted() { + assertEquals(Authz.Action.HANDOVER, FleetMcp.toolAction("fleet_handover", Map.of())); + + FleetMcp m = mcp(null); + assertNull(m.denyFor(Principal.primary(1), Authz.Action.HANDOVER, null), + "the primary may drive fleet_handover"); + + McpSchema.CallToolResult deniedWorker = + m.denyFor(Principal.worker("term_w", 2), Authz.Action.HANDOVER, null); + assertNotNull(deniedWorker, "a worker must be refused fleet_handover"); + assertTrue(deniedWorker.isError()); + + McpSchema.CallToolResult deniedArchitect = + m.denyFor(Principal.architect("slot", "term_a", 3), Authz.Action.HANDOVER, null); + assertNotNull(deniedArchitect, "an architect has no lifecycle rights either — same gate as SPAWN/STOP/DRAIN"); + assertTrue(deniedArchitect.isError()); + } + + // --- acceptance 4: the registered schema names no caller-terminal parameter ---------------- + + @Test + @DisplayName("the registered fleet_handover schema names no terminal/session/leadTerminal parameter") + void schemaCarriesNoCallerIdentityParameter() { + FleetMcp m = mcp(null); + McpSchema.Tool tool = m.registeredTools().stream() + .filter(t -> "fleet_handover".equals(t.name())) + .findFirst() + .orElseThrow(() -> new AssertionError("fleet_handover was not registered")); + + @SuppressWarnings("unchecked") + Map properties = (Map) tool.inputSchema().get("properties"); + assertNotNull(properties, "tool has no 'properties' in its input schema"); + for (String forbidden : List.of("terminal", "sessionId", "leadTerminal", "callerTerminal", "target")) { + assertFalse(properties.containsKey(forbidden), + "fleet_handover's REGISTERED schema must not carry a caller-identity parameter, " + + "found '" + forbidden + "' in " + properties.keySet()); + } + } + + // --- acceptance 5: open then confirm on the same connection; ownership is enforced -------- + + @Test + @DisplayName("open then confirm on the same terminal succeeds; a different terminal gets NOT_YOUR_ROLLOVER") + void openThenConfirmRoundTripsAndOwnershipIsEnforced() throws Exception { + Path handover = tmp.resolve("handover.md"); + Files.writeString(handover, "not written yet"); + LeadRollover rollover = newRollover(handover.toString()); + + McpSchema.CallToolResult openResult = FleetMcp.handover(rollover, LEAD, Map.of("action", "open")); + assertFalse(openResult.isError(), textOf(openResult)); + String token = extractToken(textOf(openResult)); + + // Rewrite the handover file so its mtime is measurably after open()'s requestedAtMillis — + // LeadRollover#confirm's freshness check (HANDOVER_STALE) requires this. + Thread.sleep(50); + Files.writeString(handover, "the real handover content"); + + McpSchema.CallToolResult wrongCaller = FleetMcp.handover(rollover, OTHER_LEAD, + Map.of("action", "confirm", "token", token)); + assertFalse(wrongCaller.isError(), "a refusal is a legitimate outcome, not a protocol error"); + assertTrue(textOf(wrongCaller).contains("NOT_YOUR_ROLLOVER"), + "a different lead terminal confirming must surface NOT_YOUR_ROLLOVER: " + textOf(wrongCaller)); + + McpSchema.CallToolResult confirmed = FleetMcp.handover(rollover, LEAD, + Map.of("action", "confirm", "token", token)); + assertFalse(confirmed.isError(), textOf(confirmed)); + assertTrue(textOf(confirmed).contains("\"accepted\":true"), + "the SAME terminal that opened the request must be able to confirm it: " + textOf(confirmed)); + } + + // --- acceptance 6: cancel on an unknown token is clean, not a failure ---------------------- + + @Test + @DisplayName("cancel on an unknown token reports no pending request, rather than failing") + void cancelUnknownTokenIsCleanNotAFailure() { + LeadRollover rollover = newRollover(tmp.resolve("h.md").toString()); + + McpSchema.CallToolResult r = FleetMcp.handover(rollover, LEAD, + Map.of("action", "cancel", "token", "does-not-exist")); + assertFalse(r.isError()); + assertTrue(textOf(r).contains("\"cancelled\":false"), textOf(r)); + assertTrue(textOf(r).contains("no pending request"), textOf(r)); + } + + @Test + @DisplayName("cancel on a token actually opened reports cancelled:true") + void cancelKnownTokenSucceeds() { + LeadRollover rollover = newRollover(tmp.resolve("h.md").toString()); + String token = extractToken(textOf(FleetMcp.handover(rollover, LEAD, Map.of("action", "open")))); + + McpSchema.CallToolResult r = FleetMcp.handover(rollover, LEAD, + Map.of("action", "cancel", "token", token)); + assertFalse(r.isError()); + assertTrue(textOf(r).contains("\"cancelled\":true"), textOf(r)); + } + + // --- acceptance 7 (wiring) is covered by FleetdLeadRolloverWiringTest, unchanged ----------- +} -- 2.52.0 From eb0557621e7901735014f610c8a7bb934f741acc Mon Sep 17 00:00:00 2001 From: Dai Ha Date: Fri, 11 Sep 2026 07:25:08 +0700 Subject: [PATCH 2/2] fleetd #480 correction round: collapse FleetMcp to one required constructor MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit FleetMcp had a defaulted 15-argument constructor that delegated to the new 16-argument one with an implicit null for leadRollover. Dropping the leadRollover argument from Fleetd.main's FleetMcp(...) call fell back to that shorter overload, compiled fine, and left all 1659 tests green — the live daemon would then answer NOT_CONFIGURED to fleet_handover forever with nothing going red. Delete every overload that could reach the 16-arg constructor with a silently-defaulted leadRollover (11/12/13/14/15-arg forms all chained to it), leaving the 16-arg constructor as FleetMcp's sole public constructor. Update FleetMcpAuthzTest's call site to pass every parameter explicitly (leadChannel null, OutageSource.none(), LeadSeatSource.none(), List.of(), leadRollover null) — Fleetd.java and FleetMcpHandoverTest already called the full form. Proved with mvn -o -q compile: removing the leadRollover argument from Fleetd.main now fails to compile instead of silently defaulting. No behaviour changes — NOT_CONFIGURED refusals are unchanged. --- .../java/dev/ltms/fleet/mcp/FleetMcp.java | 129 ++++++------------ .../dev/ltms/fleet/mcp/FleetMcpAuthzTest.java | 7 +- 2 files changed, 51 insertions(+), 85 deletions(-) diff --git a/fleetd/src/main/java/dev/ltms/fleet/mcp/FleetMcp.java b/fleetd/src/main/java/dev/ltms/fleet/mcp/FleetMcp.java index d566521..6817169 100644 --- a/fleetd/src/main/java/dev/ltms/fleet/mcp/FleetMcp.java +++ b/fleetd/src/main/java/dev/ltms/fleet/mcp/FleetMcp.java @@ -251,91 +251,52 @@ public final class FleetMcp { } /** - * @param callers resolves each call's {@link Principal}; {@code null} disables authorization. - * This surface needs its own enforcement: {@code /mcp} is a raw servlet on - * Jetty's context handler and never passes through Javalin's {@code before} - * filter, so the REST guard does not cover it. - * @param metrics registry for auth-failure counting; may be {@code null} - * @param quarantine CB-578 stage B facts for {@code fleet_profiles}; required — pass - * {@link QuarantineSource#none()} for a caller that does not want the feature - */ - public FleetMcp(MessageService messages, PeerLauncher workers, SessionManager sessions, - ConnectionIdentity identity, MemberPresence presence, PrimaryRegistry primaryRegistry, - CallerResolver callers, Metrics metrics, CapacitySource capacity, HealthCoverageSource healthCoverage, - QuarantineSource quarantine) { - this(messages, workers, sessions, identity, presence, primaryRegistry, callers, metrics, capacity, - healthCoverage, quarantine, null, OutageSource.none(), LeadSeatSource.none()); - } - - /** - * As above, with this daemon's lead-to-lead channel (CB-637). {@code leadChannel} is - * {@code null} whenever no {@code coordinator:} block is configured or its broker could not be - * reached at boot — cross-daemon lead messaging is simply off, and {@code fleet_send{coordId}} - * says so rather than failing obscurely. - */ - public FleetMcp(MessageService messages, PeerLauncher workers, SessionManager sessions, - ConnectionIdentity identity, MemberPresence presence, PrimaryRegistry primaryRegistry, - CallerResolver callers, Metrics metrics, CapacitySource capacity, HealthCoverageSource healthCoverage, - QuarantineSource quarantine, LeadChannel leadChannel) { - this(messages, workers, sessions, identity, presence, primaryRegistry, callers, metrics, capacity, - healthCoverage, quarantine, leadChannel, OutageSource.none(), LeadSeatSource.none()); - } - - /** - * As above, with fleetd #201 Unit 5 cool-off facts for {@code fleet_list}/{@code fleet_profiles} - * (see {@link OutageSource}). + * The only constructor (fleetd #480 Unit C correction round). Every field below used to have + * its own defaulting overload — {@code leadChannel}/{@code outage}/{@code leadSeats}/ + * {@code peers}/{@code leadRollover} each got a shorter, convenience constructor that silently + * filled it in ({@code null}, {@code .none()}, or {@code List.of()}) when a caller did not pass + * it. That is exactly how {@code Fleetd.main}'s wiring of {@link LeadRollover} could have gone + * silently missing: drop one argument from the real call and it just lands on a shorter + * overload instead of failing to compile, and every existing test — none of which exercises + * {@code Fleetd.main} itself — stays green while the live daemon quietly answers + * {@code NOT_CONFIGURED} to {@code fleet_handover} forever. Collapsing every overload into one + * required-everything constructor turns that mistake into a compile error instead: this + * project's own antidote for a defaulted parameter surviving as an untested decision (see + * {@code FleetdCompletionResolverWiringTest} / {@code FleetdLeadRolloverWiringTest}'s own + * javadoc for the same lesson applied to a different seam). A caller that genuinely wants a + * feature off must now say so explicitly at the call site — {@code null}, + * {@link OutageSource#none()}, {@link LeadSeatSource#none()}, {@code List.of()} are all still + * perfectly fine values, just never an implicit default reached by omission. * - * @param outage required — pass {@link OutageSource#none()} for a caller that does not want the - * feature, never a defaulting overload (the same rule {@code quarantine} follows). - */ - public FleetMcp(MessageService messages, PeerLauncher workers, SessionManager sessions, - ConnectionIdentity identity, MemberPresence presence, PrimaryRegistry primaryRegistry, - CallerResolver callers, Metrics metrics, CapacitySource capacity, HealthCoverageSource healthCoverage, - QuarantineSource quarantine, LeadChannel leadChannel, OutageSource outage) { - this(messages, workers, sessions, identity, presence, primaryRegistry, callers, metrics, capacity, - healthCoverage, quarantine, leadChannel, outage, LeadSeatSource.none()); - } - - /** - * As above, with fleetd #176 lead-seat facts (see {@link LeadSeatSource}). - */ - public FleetMcp(MessageService messages, PeerLauncher workers, SessionManager sessions, - ConnectionIdentity identity, MemberPresence presence, PrimaryRegistry primaryRegistry, - CallerResolver callers, Metrics metrics, CapacitySource capacity, HealthCoverageSource healthCoverage, - QuarantineSource quarantine, LeadChannel leadChannel, OutageSource outage, - LeadSeatSource leadSeats) { - this(messages, workers, sessions, identity, presence, primaryRegistry, callers, metrics, capacity, - healthCoverage, quarantine, leadChannel, outage, leadSeats, List.of()); - } - - /** - * As above, with fleetd #361 {@code coordinator.peers} (see {@link CoordinationSource}). - * - * @param leadSeats required — pass {@link LeadSeatSource#none()} for a caller that does not want - * the feature, never a defaulting overload (the same rule {@code quarantine} and - * {@code outage} follow). - * @param peers the coord-ids declared under {@code coordinator.peers}; empty when unset or - * when {@code leadChannel} is {@code null}. - */ - public FleetMcp(MessageService messages, PeerLauncher workers, SessionManager sessions, - ConnectionIdentity identity, MemberPresence presence, PrimaryRegistry primaryRegistry, - CallerResolver callers, Metrics metrics, CapacitySource capacity, HealthCoverageSource healthCoverage, - QuarantineSource quarantine, LeadChannel leadChannel, OutageSource outage, - LeadSeatSource leadSeats, List peers) { - this(messages, workers, sessions, identity, presence, primaryRegistry, callers, metrics, capacity, - healthCoverage, quarantine, leadChannel, outage, leadSeats, peers, null); - } - - /** - * As above, with fleetd #480 Unit C: the {@link LeadRollover} executor behind - * {@code fleet_handover}. This is what {@code Fleetd.main} actually wires up. - * - * @param leadRollover {@code null} whenever {@code leadRollover:} is not configured — an - * upgraded daemon must never silently acquire the ability to clear a lead's - * own pane (mirrors {@code Fleetd.leadRollover(...)}'s own construction - * gate). {@code fleet_handover} is registered unconditionally either way — - * see this class's javadoc and fleetd #474's charter tool-surface gate — - * and every action degrades to a clean refusal naming {@code NOT_CONFIGURED} + * @param callers resolves each call's {@link Principal}; {@code null} disables + * authorization. This surface needs its own enforcement: {@code /mcp} is a + * raw servlet on Jetty's context handler and never passes through + * Javalin's {@code before} filter, so the REST guard does not cover it. + * @param metrics registry for auth-failure counting; may be {@code null} + * @param quarantine CB-578 stage B facts for {@code fleet_profiles}; pass + * {@link QuarantineSource#none()} for a caller that does not want the + * feature + * @param leadChannel this daemon's lead-to-lead channel (CB-637); {@code null} whenever no + * {@code coordinator:} block is configured or its broker could not be + * reached at boot — cross-daemon lead messaging is simply off, and + * {@code fleet_send{coordId}} says so rather than failing obscurely + * @param outage fleetd #201 Unit 5 cool-off facts for {@code fleet_list}/ + * {@code fleet_profiles}; pass {@link OutageSource#none()} for a caller + * that does not want the feature + * @param leadSeats fleetd #176 lead-seat facts (see {@link LeadSeatSource}); pass + * {@link LeadSeatSource#none()} for a caller that does not want the + * feature + * @param peers fleetd #361 {@code coordinator.peers} (see {@link CoordinationSource}); + * the coord-ids declared there, or empty when unset or when + * {@code leadChannel} is {@code null} + * @param leadRollover fleetd #480 Unit C: the {@link LeadRollover} executor behind + * {@code fleet_handover}. {@code null} whenever {@code leadRollover:} is + * not configured — an upgraded daemon must never silently acquire the + * ability to clear a lead's own pane (mirrors + * {@code Fleetd.leadRollover(...)}'s own construction gate). + * {@code fleet_handover} is registered unconditionally either way — see + * this class's javadoc and fleetd #474's charter tool-surface gate — and + * every action degrades to a clean refusal naming {@code NOT_CONFIGURED} * instead of throwing. See {@link #handover}. */ public FleetMcp(MessageService messages, PeerLauncher workers, SessionManager sessions, diff --git a/fleetd/src/test/java/dev/ltms/fleet/mcp/FleetMcpAuthzTest.java b/fleetd/src/test/java/dev/ltms/fleet/mcp/FleetMcpAuthzTest.java index 305920b..38a26ab 100644 --- a/fleetd/src/test/java/dev/ltms/fleet/mcp/FleetMcpAuthzTest.java +++ b/fleetd/src/test/java/dev/ltms/fleet/mcp/FleetMcpAuthzTest.java @@ -26,6 +26,7 @@ import org.junit.jupiter.api.Test; import java.nio.file.Files; import java.nio.file.Path; +import java.util.List; import java.util.Map; import java.util.Set; import java.util.regex.Matcher; @@ -74,12 +75,16 @@ class FleetMcpAuthzTest { ConnectionIdentity identity = new ConnectionIdentity(new PaneLocator(herdr), _ -> 999_999); metrics = FleetMetrics.create(sessions, new InMemoryReplyInbox()); + // fleetd #480 correction round: FleetMcp has one constructor now (no defaulting + // overloads — see its javadoc), so every feature this test does not exercise is passed + // its explicit "off" value here rather than being omitted. mcp = new FleetMcp(messages, workers, sessions, identity, sessions.asPresence(), new PrimaryRegistry(null), enforce ? CallerResolver.withLeadsAndMembers(identity, false, null, Map::of, new MemberRegistry(null)) : null, metrics, FleetMcp.CapacitySource.none(), new FleetMcp.HealthCoverageSource(() -> "off"), - FleetMcp.QuarantineSource.none()); + FleetMcp.QuarantineSource.none(), null, FleetMcp.OutageSource.none(), + FleetMcp.LeadSeatSource.none(), List.of(), null); return mcp; } -- 2.52.0