Compare commits

...

3 Commits

Author SHA1 Message Date
Dai Ha 656588f597 fleetd #661: add the pane-placement case to the validateAll reachability enumeration
CI / shell-tests (pull_request) Failing after 7s
CI / contract (pull_request) Successful in 53s
CI / build (pull_request) Failing after 1m40s
validateAllReachesEveryOneOfTodaysSixValidators only covered six of
the eight real validators; the new validator was reachability-tested
only from FleetConfigTest, in a different file from the one whose job
is to enumerate every validateAll-reachability case.

Add the pane-placement case to the enumeration, rename the method to
drop the hardcoded count (validateAllReachesEveryOneOfTodaysRealValidators),
and correct the surrounding claims to say seven of eight, naming
validateLeadRollover as the one case still missing (fleetd #668, not
fixed here).
2026-10-03 19:50:16 +02:00
Dai Ha e33377b2ca fleetd #661: fix LeadCount javadoc, dangling @link, and the validator-count word
CI / shell-tests (pull_request) Failing after 7s
CI / contract (pull_request) Successful in 1m3s
CI / build (pull_request) Failing after 1m50s
LeadLauncher.LeadCount's javadoc carried the same false member-space-
exclusion claim as the three comments fixed earlier in this ticket;
its neighbouring body comment in countLeads was already correct and
is unchanged.

FleetConfigValidateAllTest's canary test is renamed to drop the
number from its name (the count now lives only in the Set.of literal
and the javadoc, so the two cannot drift), which also fixes the
dangling {@link} to the old name and the stale 'seventh' wording.
2026-10-03 19:42:50 +02:00
Dai Ha f288cee2bb fleetd #661: refuse pane placement when a lead tab is configured
CI / shell-tests (pull_request) Failing after 6s
CI / contract (pull_request) Successful in 45s
CI / build (pull_request) Failing after 1m55s
A pane-placed member lands inside the focused tab rather than its own,
so it can land inside a lead's labelled tab and be read back as that
lead by LeadTabScanner, which does not exclude the member space in
production. Add FleetConfig.validatePanePlacementAgainstLeadTabs(),
wired automatically into validateAll() by the existing reflective
sweep, to refuse that combination at startup.

Also correct three stale comments that claimed a member-space
exclusion already blocked this path, in LeadTabScanner, FleetConfig's
validateLeadTabPrefixes javadoc, and LeadTabScannerTest.
2026-10-03 19:34:34 +02:00
6 changed files with 175 additions and 25 deletions
@@ -2688,9 +2688,10 @@ public record FleetConfig(
* tab labels — every member gets one rendered into its tab. Choose a lead {@code tabPrefix} that
* a member template matches and the daemon starts labelling its own members as leads, promoting
* the entire fleet to {@link dev.ltms.fleet.auth.Role#PRIMARY} with no message and no diff.
* The member-space exclusion in {@link dev.ltms.fleet.herdr.LeadTabScanner} already blocks the
* realistic path, but defence that depends on one workspace label holding is not defence enough
* for a privilege boundary.
* {@link #validatePanePlacementAgainstLeadTabs()} is the check that stops a pane-placed member
* from landing inside a lead's tab in the first place; this check is a second, independent
* guard that catches the hazard even when every profile places members correctly, by refusing
* a label that a scan would still misread as a lead.
*
* <p>CB-557 shrank this check rather than removing it. The default template is
* {@code "{role}: {profile} #{n}"} and {@code {role}} comes from a closed enum, so a
@@ -2737,6 +2738,45 @@ public record FleetConfig(
+ "lead tabs cannot be confused.");
}
/**
* Reject a profile that places its members by {@code "pane"} while any {@code fleet.leaders}
* entry names a {@code tab}. A pane-placed member lands inside the focused tab rather than its
* own, so it can land inside a lead's own labelled tab. {@link
* dev.ltms.fleet.herdr.LeadTabScanner} identifies a lead purely by that tab's label — it does
* not exclude the member space — so a member that ends up there would be read back as the lead
* and granted spawn/stop/send on the whole fleet.
*
* <p>Only a leader with a non-blank {@code tab} is in scope: one with no {@code tab} feeds
* nothing into {@link dev.ltms.fleet.herdr.LeadTabScanner}, so it creates no hazard here.
*
* @throws IllegalStateException when any {@code profiles:} entry is pane-placed while any
* {@code fleet.leaders} entry names a non-blank {@code tab}
*/
public void validatePanePlacementAgainstLeadTabs() {
if (fleet == null || fleet.leaders().isEmpty()) {
return;
}
boolean anyLeaderHasTab = fleet.leaders().values().stream()
.anyMatch(leader -> leader != null && leader.tab() != null && !leader.tab().isBlank());
if (!anyLeaderHasTab) {
return;
}
List<String> bad = new ArrayList<>();
profiles().entrySet().stream()
.filter(e -> !e.getValue().tabPlacement())
.map(Map.Entry::getKey)
.sorted()
.forEach(bad::add);
if (bad.isEmpty()) {
return;
}
throw new IllegalStateException("refusing to start: profile(s) " + bad
+ " use placement: pane while fleet.leaders names a tab. A pane-placed member can "
+ "land inside a lead's labelled tab and be read back as the lead, granted "
+ "spawn/stop/send on the whole fleet. Set placement: tab for each named profile, "
+ "or remove the tab from every fleet.leaders entry.");
}
/**
* Reject a present {@code leadRollover:} block with no (or a blank) {@code handoverPath}
* (fleetd #480). There is no sane non-null default for an operator-specific file path, unlike
@@ -37,8 +37,11 @@ import java.util.function.Supplier;
* <p><strong>Direction of trust.</strong> The label names the lead; it never <em>grants</em>
* anything a pane could take for itself. Three properties keep that honest:
* <ol>
* <li>Worker spaces are excluded wholesale ({@code excludedWorkspaceLabels}), so a worker cannot
* become a lead by being placed — as a split, say — inside a matching tab.</li>
* <li>{@code excludedWorkspaceLabels} can filter a workspace out of the scan, but this class does
* not by itself stop a worker from landing inside a matching tab — a caller may pass an empty
* set, and the daemon does. The guard against that is {@code
* FleetConfig.validatePanePlacementAgainstLeadTabs}: it refuses, at startup, any profile that
* places members by pane while a lead names a tab.</li>
* <li>A worker cannot rename a tab: {@code tab.rename} is reachable only through
* {@link WorkspaceControl}, which no {@code fleet_*} tool exposes. The label is writable by
* the human at the terminal and by nobody the bridge is defending against.</li>
@@ -201,8 +201,8 @@ public final class LeadLauncher {
/**
* How many live leads exist per configured name, and which of that name's labelled tabs are
* <em>not</em> live: a running agent in a tab labelled with that lead's exact {@code tab}
* (CB-579). Member workspaces are excluded, exactly as the scanner excludes them: a member must
* not be counted as a lead because it happens to sit in a matching tab.
* (CB-579). A member sitting in the same shared workspace is not counted as a lead because its
* tab carries a different label, not because any workspace is excluded from this count.
*
* <p>There used to be a second path here — a running agent on the terminal a
* {@code fleet.leaders.<name>.terminal} pin named, for a lead opened and pinned by hand. That
@@ -765,6 +765,92 @@ class FleetConfigTest {
"a label that collides with a convention nobody reads is not a problem");
}
// ── validatePanePlacementAgainstLeadTabs ────────────────────────────────────────────────────
/**
* The hazard this guard closes: a pane-placed member lands inside the focused tab rather than
* its own, so it can land inside a lead's labelled tab and be read back as that lead.
*/
@Test
void aPanePlacedProfileWithALeadTabRefusesToStart(@TempDir Path dir) throws Exception {
Path f = dir.resolve("pane-hazard.yaml");
Files.writeString(f, """
bind:
port: 8080
profiles:
gx10:
placement: pane
fleet:
leaders:
opus:
tab: "lead: opus"
""");
FleetConfig cfg = FleetConfig.load(f);
IllegalStateException e = assertThrows(IllegalStateException.class,
cfg::validatePanePlacementAgainstLeadTabs);
assertTrue(e.getMessage().contains("gx10"), "the message must name the offending profile");
}
@Test
void aPanePlacedProfileWithNoLeadTabIsAllowed(@TempDir Path dir) throws Exception {
Path f = dir.resolve("pane-no-tab.yaml");
Files.writeString(f, """
bind:
port: 8080
profiles:
gx10:
placement: pane
fleet:
leaders:
opus:
profile: gx10
""");
assertDoesNotThrow(() -> FleetConfig.load(f).validatePanePlacementAgainstLeadTabs(),
"a leader with no tab feeds nothing into the scanner, so pane placement is safe");
}
@Test
void aTabPlacedProfileWithALeadTabIsAllowed(@TempDir Path dir) throws Exception {
Path f = dir.resolve("tab-safe.yaml");
Files.writeString(f, """
bind:
port: 8080
profiles:
gx10:
placement: tab
fleet:
leaders:
opus:
tab: "lead: opus"
""");
assertDoesNotThrow(() -> FleetConfig.load(f).validatePanePlacementAgainstLeadTabs(),
"a member in its own tab cannot land inside a lead's tab");
}
/** Proves the reflective sweep behind {@code validateAll} really reaches this validator. */
@Test
void validateAllAlsoRefusesPanePlacementAgainstALeadTab(@TempDir Path dir) throws Exception {
Path f = dir.resolve("pane-hazard-sweep.yaml");
Files.writeString(f, """
bind:
port: 8080
profiles:
gx10:
placement: pane
fleet:
leaders:
opus:
tab: "lead: opus"
""");
FleetConfig cfg = FleetConfig.load(f);
IllegalStateException e = assertThrows(IllegalStateException.class, cfg::validateAll);
assertTrue(e.getMessage().contains("gx10"), "the message must name the offending profile");
}
// ── CB-530/CB-579: the leaders registry ─────────────────────────────────────────────────────
@Test
@@ -42,12 +42,13 @@ import static org.junit.jupiter.api.Assertions.assertTrue;
* has right now. This is the proof that a future, real seventh validator on {@link
* FleetConfig} would be swept automatically, without needing to add a real (unwanted)
* seventh validator just to exercise the claim.</li>
* <li>{@link #validateAllReachesEveryOneOfTodaysSixValidators()} proves {@link
* <li>{@link #validateAllReachesEveryOneOfTodaysRealValidators()} proves {@link
* FleetConfig#validateAll()} itself is wired to that same generic mechanism and genuinely
* reaches each of today's six real validators — reusing the exact minimal failing
* reaches seven of today's eight real validators — reusing the exact minimal failing
* configurations {@code FleetConfigTest} already established for each one directly, so a
* single call to {@code validateAll()} is shown to reproduce every one of those six
* failures.</li>
* single call to {@code validateAll()} is shown to reproduce every one of those seven
* failures. The eighth, {@link FleetConfig#validateLeadRollover()}, has no case here yet —
* a pre-existing gap tracked as fleetd #668.</li>
* </ol>
*
* <p>Together with the direct-{@code Fleetd.main}-invocation tests in {@code
@@ -62,8 +63,8 @@ import static org.junit.jupiter.api.Assertions.assertTrue;
* the generic sweep — claim 1 proves {@link FleetConfig#invokeAllValidators} is generic, and claim
* 2 proves {@code validateAll()} reaches today's six, and a hardcoded list satisfies both. So the
* reflective sweep is a convenience, not the guarantee. The guarantee is {@link
* #fleetConfigDeclaresExactlyTheseSixValidatorsToday()}: it fails the moment a seventh validator
* is declared, which forces whoever adds it to look at this file.
* #fleetConfigDeclaresExactlyTheseValidatorsToday()}: it fails the moment any validator is added
* or removed, which forces whoever changes the set to look at this file.
*/
class FleetConfigValidateAllTest {
@@ -209,18 +210,19 @@ class FleetConfigValidateAllTest {
+ "name) must all be skipped");
}
// ── Claim 2: FleetConfig.validateAll() is wired to that mechanism and reaches all six today ──
// ── Claim 2: FleetConfig.validateAll() is wired to that mechanism and reaches seven of eight today ──
/**
* Reflectively enumerates {@link FleetConfig}'s own public, no-arg, void {@code validateXxx()}
* methods (excluding {@code validateAll} itself) — the exact same filter {@link
* FleetConfig#invokeAllValidators} applies. This is not the mechanism proof (that is claim 1,
* above, on an unrelated class) — it is a visible denominator: today there are six, named
* here, so a reader adding a seventh sees this assertion name the new count rather than a
* silent pass at the old one.
* above, on an unrelated class) — it is a visible denominator: today there are eight, named in
* the {@code Set.of} below, so a reader adding or removing one sees this assertion name the new
* count rather than a silent pass at the old one. The count lives only in that set, not in this
* method's name, so the two cannot drift apart.
*/
@Test
void fleetConfigDeclaresExactlyTheseSixValidatorsToday() {
void fleetConfigDeclaresExactlyTheseValidatorsToday() {
Set<String> names = new TreeSet<>();
for (Method m : FleetConfig.class.getMethods()) {
if (java.lang.reflect.Modifier.isPublic(m.getModifiers())
@@ -233,7 +235,8 @@ class FleetConfigValidateAllTest {
}
assertEquals(new TreeSet<>(Set.of("validateAuthExposure", "validateLeadTabPrefixes",
"validateSubscriptionProfiles", "validateCharters", "validateMembers",
"validateModels", "validateLeadRollover")), names,
"validateModels", "validateLeadRollover", "validatePanePlacementAgainstLeadTabs")),
names,
"FleetConfig's public validate*() methods changed. Do TWO things, in this "
+ "order. First confirm validateAll() still delegates to "
+ "invokeAllValidators(this) — a hardcoded list there passes every other "
@@ -262,14 +265,17 @@ class FleetConfigValidateAllTest {
}
/**
* The heart of claim 2: for each of today's six real validators, a minimal file that fails
* The heart of claim 2: for seven of today's eight real validators, a minimal file that fails
* ONLY that one — the exact fixtures {@code FleetConfigTest} uses to test each validator
* directly — must also fail through {@link FleetConfig#validateAll()}. If a future edit to
* {@code validateAll()} silently dropped one validator from the sweep (e.g. a typo'd name
* filter), exactly one of these six would start passing when it must not.
* {@code validateAll()} silently dropped one of these seven from the sweep (e.g. a typo'd name
* filter), exactly one of them would start passing when it must not.
*
* <p>The eighth, {@link FleetConfig#validateLeadRollover()}, has no case here — a pre-existing
* gap tracked as fleetd #668, not fixed by this change.
*/
@Test
void validateAllReachesEveryOneOfTodaysSixValidators(@TempDir Path dir) throws Exception {
void validateAllReachesEveryOneOfTodaysRealValidators(@TempDir Path dir) throws Exception {
// validateAuthExposure: a non-loopback bind without token mode.
assertValidateAllRefuses(dir, "auth-exposure.yaml", """
bind:
@@ -342,6 +348,20 @@ class FleetConfigValidateAllTest {
allow:
- model: claude-sonnet-5
""", "rogue");
// validatePanePlacementAgainstLeadTabs: a pane-placed profile while a lead names a tab.
assertValidateAllRefuses(dir, "pane-placement.yaml", """
bind:
host: 127.0.0.1
port: 8765
profiles:
gx10:
placement: pane
fleet:
leaders:
opus:
tab: "lead: opus"
""", "gx10");
}
private static void assertValidateAllRefuses(Path dir, String fileName, String yaml,
@@ -232,8 +232,9 @@ class LeadTabScannerTest {
@Test
void everyPaneInALeadTabResolvesAsThatLead() {
// A human may split their own lead tab. Both panes are theirs, so both are that lead —
// nothing fleetd placed can land here (see the worker-space test above).
// A human may split their own lead tab. Both panes are theirs, so both are that lead.
// A pane-placed member landing here instead is refused at startup by
// FleetConfig.validatePanePlacementAgainstLeadTabs, not by this scanner.
TopologyHerdr herdr = twoLeads().pane("w1:p1b", "w1:t1", "term_opus_split");
assertEquals("opus-5.0",