From c796eac09cfdba71db5897c897fdd9520ccdb375 Mon Sep 17 00:00:00 2001 From: Dai Ha Date: Thu, 3 Sep 2026 12:55:08 +0700 Subject: [PATCH 1/4] fleetd #176: subtract the lead's own subscription seat from free maxLoad counted panes, never subscription seats: a subscription:true profile's lead is itself a live claude session on that same account, so free overstated capacity by the lead's own seat (measured free:1 with a real ceiling of 0, and free:3 on an idle fleet with a real ceiling of 2). Add FleetMcp.LeadSeatSource (same shape as QuarantineSource/ OutageSource) and Fleetd.leadSeatLookup, which derives the seat count from fleet.leaders..profile matched against the target profile by effectiveCredentialId() - no hardcoded "-1", and no new config key: profile: already exists for this exact "which account does this lead share" question. maxLoad itself is left untouched; only free (and a new, additive-only leadSeats field) changes. Exhaustion quarantine (cause 2 in the ticket) already forced free to 0 via the same BackendQuarantine capacityView already reads - confirmed by reading the exhaustionSink wiring, no code change needed there. --- fleetd/fleetd.example.yaml | 20 +++ .../src/main/java/dev/ltms/fleet/Fleetd.java | 65 ++++++++- .../dev/ltms/fleet/config/FleetConfig.java | 10 +- .../java/dev/ltms/fleet/mcp/FleetMcp.java | 80 +++++++++-- .../ltms/fleet/FleetdLeadSeatLookupTest.java | 129 ++++++++++++++++++ .../ltms/fleet/FleetdLeadSeatWiringTest.java | 43 ++++++ .../java/dev/ltms/fleet/mcp/FleetMcpTest.java | 62 +++++++++ 7 files changed, 398 insertions(+), 11 deletions(-) create mode 100644 fleetd/src/test/java/dev/ltms/fleet/FleetdLeadSeatLookupTest.java create mode 100644 fleetd/src/test/java/dev/ltms/fleet/FleetdLeadSeatWiringTest.java diff --git a/fleetd/fleetd.example.yaml b/fleetd/fleetd.example.yaml index 79a0d0a..da3b057 100644 --- a/fleetd/fleetd.example.yaml +++ b/fleetd/fleetd.example.yaml @@ -306,6 +306,15 @@ profiles: # GOTCHA 2 — `maxLoad` is the ONLY throttle you have here. There is no metering, no budget # and no refusal on cost; the cap on live members is the single thing standing between a # fan-out and your monthly limit. Set it deliberately and keep it small. + # + # GOTCHA 3 (fleetd #176) — `maxLoad` counts members, never the lead itself. The lead is a live + # `claude` session on this SAME account (a lead is never moved off-subscription, whatever its + # own profile says), so it already holds one seat before any member spawns. If a lead's + # `fleet.leaders..profile` names THIS profile (or one sharing its `credentialId`), + # `fleet_list`'s `free` for this profile subtracts that lead's live seat(s) automatically — see + # `profile:` under THE FLEET below. If no lead entry names this profile, fleetd has no way to + # know it shares this account, and `free` will overstate what a fresh `fleet_spawn` actually + # gets by exactly the seats the lead is quietly holding. # gitTokenEnv: GITEA_TOKEN # opt-in: let this profile's workers open their own PR (CB-302) # gitHostEnv: GITEA_HOST # defaults to GITEA_HOST; injected only with gitTokenEnv # exhaustedPattern: "usage limit has been reached" # opt-in: classify a usage-limit refusal (CB-578) @@ -503,6 +512,17 @@ fleet: # recognised: give it a `profile:` and the daemon launches the shortfall when fewer than # `instances` are live. Omit `profile:` and it is recognise-only, as before. # + # `profile:` has a SECOND job as of fleetd #176, even for a recognise-only lead you never want + # auto-launched: it is also how fleetd learns which account this lead's own session shares. A + # `subscription: true` profile bills the operator's Claude account, and the lead itself is always + # a live `claude` session on that same account — `maxLoad` never counted that seat. If a lead + # entry here names a profile (or one sharing its `credentialId`) that matches a worker profile, + # `fleet_list`'s `free` for that worker profile subtracts the lead's live seat(s) automatically. + # Setting `profile:` on an already-running, recognise-only lead is safe — the daemon only launches + # the SHORTFALL below `instances`, so naming a profile here does not, by itself, start anything. + # Omit it and fleetd has no way to derive the sharing — there is no other reliable signal on the + # daemon's side — so that lead's seat goes uncounted, exactly as before this ticket. + # # `tab:` (CB-579) is REQUIRED and is the only field identity depends on — the exact label of the # tab hosting the lead, matched case-insensitively. Label the tab yourself and put that same # string here, and the pane is recognised on the next rescan. Reopen the tab later, or the session diff --git a/fleetd/src/main/java/dev/ltms/fleet/Fleetd.java b/fleetd/src/main/java/dev/ltms/fleet/Fleetd.java index 8d4cae3..ffb8153 100644 --- a/fleetd/src/main/java/dev/ltms/fleet/Fleetd.java +++ b/fleetd/src/main/java/dev/ltms/fleet/Fleetd.java @@ -640,7 +640,8 @@ public final class Fleetd { new FleetMcp.OutageSource(profile -> { var configured = config.get().profiles().get(profile); return configured == null ? null : configured.effectiveCredentialId(); - }, outagePolicy)); + }, outagePolicy), + new FleetMcp.LeadSeatSource(leadSeatLookup(() -> config.get().profiles(), leaders, leads))); // 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 @@ -765,6 +766,68 @@ public final class Fleetd { .orElse(null); } + /** + * fleetd #176: per-profile factory for {@link FleetMcp.LeadSeatSource} — how many seats a + * profile's own live LEAD session(s) hold on the same Claude subscription. + * + *

{@code maxLoad} counts only members; the lead itself is a live {@code claude} session that + * is never moved off-subscription ({@code LeadLauncher} strips {@code ANTHROPIC_BASE_URL}/ + * {@code AUTH_TOKEN} from a lead's env whatever its profile says), so a {@code subscription: + * true} profile's real ceiling is lower than its configured {@code maxLoad} by exactly the + * number of lead seats sharing that same account. + * + *

The derivation, and why this route was chosen over a new config key. The link is + * {@code fleet.leaders..profile} — the field the operator already sets to name which + * {@code profiles:} entry a lead runs on (CB-557; see {@code fleetd.example.yaml}) — matched + * against the profile passed in here via {@link FleetConfig.Profile#effectiveCredentialId()}, + * the same grouping key {@link dev.ltms.fleet.placement.BackendQuarantine} already uses to say + * two profiles share one account. Nothing new is added to the config schema: this reuses a field + * that already exists and already means "the profile this lead's own session runs on". A lead + * entry that names no {@code profile:} (recognise-only, CB-558) says nothing about which account + * it shares, and there is no other reliable signal on the daemon's side to derive that from — so + * such a lead contributes no seats, exactly as before this ticket. Making that lead's seat count + * requires the operator to add one line (`profile: sonnet` under its {@code fleet.leaders} entry) + * — a config statement, not a code change, and the smallest one available given the field + * already exists for a closely related purpose. + * + *

Only counts leads {@code liveLeadTerminals} currently reports — CB-531's live tab scan (or + * the legacy {@code primary.terminal} pin) — never every configured lead: an entry whose + * {@code instances} nobody has actually started is not really competing for a seat, and must not + * shrink capacity for one that was never live. + * + * @param profiles the live profile map, normally {@code () -> config.get().profiles()} + * in {@code main} — hot, like every other {@code maxLoad}/ + * {@code credentialId} read {@link FleetMcp.CapacitySource} already does + * @param leaders {@code fleet.leaders}, read once at startup like the rest of that + * block ({@code fleetd.example.yaml} notes it is not hot) — passed as a + * plain map, never re-read from {@code config.get()} + * @param liveLeadTerminals terminal_id → lead name for every CURRENTLY recognised lead, normally + * the same supplier {@link dev.ltms.fleet.auth.CallerResolver#leads()} + * and {@code LeadCoordLoop} already consult + */ + static Function leadSeatLookup(Supplier> profiles, + Map leaders, Supplier> liveLeadTerminals) { + return profileName -> { + FleetConfig.Profile target = profiles.get().get(profileName); + if (target == null || !target.isSubscription()) { + return 0; + } + String targetCredential = target.effectiveCredentialId(); + int seats = 0; + for (String leadName : liveLeadTerminals.get().values()) { + FleetConfig.Leader lead = leaders.get(leadName); + if (lead == null || lead.profile() == null || lead.profile().isBlank()) { + continue; + } + FleetConfig.Profile leadProfile = profiles.get().get(lead.profile()); + if (leadProfile != null && targetCredential.equals(leadProfile.effectiveCredentialId())) { + seats++; + } + } + return seats; + }; + } + /** * fleetd #248 / fleetd#201 Unit 5: package-private factory for the per-target backend-error * pattern lookup {@link CompletionResolver} classifies a pane scrape against. Closes over the diff --git a/fleetd/src/main/java/dev/ltms/fleet/config/FleetConfig.java b/fleetd/src/main/java/dev/ltms/fleet/config/FleetConfig.java index 81300ac..e507886 100644 --- a/fleetd/src/main/java/dev/ltms/fleet/config/FleetConfig.java +++ b/fleetd/src/main/java/dev/ltms/fleet/config/FleetConfig.java @@ -946,7 +946,15 @@ public record FleetConfig( * survives restarts of the agent inside it — so identity is now the tab label alone. * * @param profile the {@code profiles:} entry to launch this lead on when one must - * be created; {@code null} ⇒ recognise-only, never create + * be created; {@code null} ⇒ recognise-only, never create. + *

fleetd #176: also the field {@code Fleetd.leadSeatLookup} reads + * to learn which account this lead's own live session shares — set it + * (safely, even on an already-running recognise-only lead: naming a + * profile here never starts anything beyond {@code instances}) so a + * {@code subscription: true} worker profile sharing its + * {@code effectiveCredentialId()} has this lead's seat subtracted from + * {@code fleet_list}'s {@code free}. {@code null} here also means this + * lead's seat cannot be derived and is not counted. * @param tab the exact tab label hosting this lead, matched case-insensitively; * the only field identity depends on. Required — a lead with no * {@code tab} can never be discovered, launched or not 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 f395ad4..e55ee4b 100644 --- a/fleetd/src/main/java/dev/ltms/fleet/mcp/FleetMcp.java +++ b/fleetd/src/main/java/dev/ltms/fleet/mcp/FleetMcp.java @@ -96,6 +96,8 @@ public final class FleetMcp { private final QuarantineSource quarantine; /** fleetd #201 Unit 5: SEPARATE from {@link #quarantine} — see {@link OutageSource}'s doc. */ private final OutageSource outage; + /** fleetd #176: SEPARATE from both of the above — see {@link LeadSeatSource}'s doc. */ + private final LeadSeatSource leadSeats; /** CB-637: this daemon's lead-to-lead channel; {@code null} when no coordinator is configured. */ private final LeadChannel leadChannel; @@ -135,6 +137,24 @@ public final class FleetMcp { } } + /** + * fleetd #176: the seats a profile's own live LEAD session(s) hold on the same Claude + * subscription — the third reason (alongside {@link QuarantineSource} and {@link OutageSource}) + * {@code free} can overstate what a fresh {@code fleet_spawn} would actually get. + * + *

{@code maxLoad} counts only members, never the lead itself. But a + * {@code subscription: true} profile bills the operator's own Claude account, and the lead is + * always a live {@code claude} session on that same account (it is never moved off-subscription + * — see {@code LeadLauncher}). So a fan-out that fills every member slot still leaves the lead's + * own seat unaccounted for, and the daemon reports a slot that was never really free. See + * {@code Fleetd.leadSeatLookup} for how the count is derived — from {@code fleet.leaders. + * .profile} and each profile's {@code effectiveCredentialId()}, never a hardcoded constant. + */ + public record LeadSeatSource(Function seatsFor) { + /** Inert source — no profile is ever reported as sharing a seat with a lead. */ + public static LeadSeatSource none() { return new LeadSeatSource(_ -> 0); } + } + /** * @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 @@ -149,7 +169,7 @@ public final class FleetMcp { 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()); + healthCoverage, quarantine, null, OutageSource.none(), LeadSeatSource.none()); } /** @@ -163,12 +183,12 @@ public final class FleetMcp { 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()); + 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}). This is what {@code Fleetd.main} actually wires up. + * (see {@link OutageSource}). * * @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). @@ -177,10 +197,28 @@ public final class FleetMcp { 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}). This is what + * {@code Fleetd.main} actually wires up. + * + * @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). + */ + 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.leadChannel = leadChannel; this.capacity = capacity; this.quarantine = Objects.requireNonNull(quarantine, "quarantine"); this.outage = Objects.requireNonNull(outage, "outage"); + this.leadSeats = Objects.requireNonNull(leadSeats, "leadSeats"); this.healthCoverage = healthCoverage; McpJsonMapper json = new JacksonMcpJsonMapperSupplier().get(); this.transport = HttpServletStreamableServerTransportProvider.builder() @@ -310,7 +348,7 @@ public final class FleetMcp { McpSchema.CallToolResult denied = deny(exchange, Authz.Action.READ, null); if (denied != null) return denied; return listFleet(workers, sessions, messages, capacity, healthCoverage, quarantine, outage, - callers == null ? Map.of() : callers.leads(), + leadSeats, callers == null ? Map.of() : callers.leads(), callerTerminal(exchange), leadChannel == null ? null : leadChannel.selfCoordId()); }; @@ -994,7 +1032,8 @@ public final class FleetMcp { CapacitySource capacity, HealthCoverageSource healthCoverage, QuarantineSource quarantine, OutageSource outage, Map leads, String selfTerm) { - return listFleet(workers, sessions, messages, capacity, healthCoverage, quarantine, outage, leads, selfTerm, null); + return listFleet(workers, sessions, messages, capacity, healthCoverage, quarantine, outage, + LeadSeatSource.none(), leads, selfTerm, null); } /** @@ -1011,7 +1050,7 @@ public final class FleetMcp { QuarantineSource quarantine, Map leads, String selfTerm, String selfCoordId) { return listFleet(workers, sessions, messages, capacity, healthCoverage, quarantine, OutageSource.none(), - leads, selfTerm, selfCoordId); + LeadSeatSource.none(), leads, selfTerm, selfCoordId); } /** As above, plus fleetd #201 Unit 5 cool-off facts (see {@link OutageSource}). */ @@ -1019,6 +1058,16 @@ public final class FleetMcp { CapacitySource capacity, HealthCoverageSource healthCoverage, QuarantineSource quarantine, OutageSource outage, Map leads, String selfTerm, String selfCoordId) { + return listFleet(workers, sessions, messages, capacity, healthCoverage, quarantine, outage, + LeadSeatSource.none(), leads, selfTerm, selfCoordId); + } + + /** As above, plus fleetd #176 lead-seat facts (see {@link LeadSeatSource}). */ + static McpSchema.CallToolResult listFleet(PeerLauncher workers, SessionManager sessions, MessageService messages, + CapacitySource capacity, HealthCoverageSource healthCoverage, + QuarantineSource quarantine, OutageSource outage, + LeadSeatSource leadSeats, Map leads, String selfTerm, + String selfCoordId) { try { Map live = workers.list().stream() .map(Agent.class::cast) @@ -1045,7 +1094,7 @@ public final class FleetMcp { } if (capacity.available()) result.put("capacity", profiles.stream() .map(profile -> capacityView(profile, capacity.liveCount(), capacity.maxLoad(), roster, messages, - capacity.clock().getAsLong(), quarantine, outage)).toList()); + capacity.clock().getAsLong(), quarantine, outage, leadSeats)).toList()); return text(json(result)); } catch (HerdrException e) { return error("herdr error listing the fleet: " + e.getMessage()); @@ -1084,20 +1133,33 @@ public final class FleetMcp { * {@code credentialId}/{@code coolingOffForSeconds}, but never {@code quarantinedForSeconds} — * that key is added only when exhaustion quarantine is ALSO active for this profile, since the * two checks are independent and either, both, or neither can be true. + * + *

fleetd #176: {@code maxLoad} counts panes, not subscription seats — it never counted the + * lead's own seat on a {@code subscription: true} profile's account. {@link LeadSeatSource} + * reports that count (0 for a non-subscription profile, or when no live lead shares its + * credential), and it is subtracted from {@code free} the same way {@code live} already is — + * {@code maxLoad} itself is left untouched, so the row still reports the configured cap. The + * {@code leadSeats} key is added only when the count is positive, for the same + * byte-identical-when-unused reason as the quarantine/cool-off keys above. */ private static Map capacityView(String profile, Function liveCount, Function maxLoad, List roster, MessageService messages, long nowNanos, QuarantineSource quarantine, - OutageSource outage) { + OutageSource outage, LeadSeatSource leadSeats) { Integer cap = maxLoad.apply(profile); int live = liveCount.apply(profile); + int leadSeatCount = leadSeats.seatsFor().apply(profile); int reclaimable = (int) roster.stream().filter(s -> profile.equals(s.profile())) .filter(s -> (s.state() == MemberSession.State.READY || s.state() == MemberSession.State.DONE)) .filter(s -> messages == null || (!messages.hasAcceptedDelivery(s.terminalId()) && !messages.hasInboxMessage(s.terminalId()))) .count(); Map row = new LinkedHashMap<>(); row.put("profile", profile); row.put("maxLoad", cap); row.put("live", live); - row.put("free", cap == null ? null : Math.max(0, cap - live)); row.put("reclaimable", reclaimable); + row.put("free", cap == null ? null : Math.max(0, cap - live - leadSeatCount)); + row.put("reclaimable", reclaimable); + if (leadSeatCount > 0) { + row.put("leadSeats", leadSeatCount); + } String credentialId = quarantine.credentialIdFor().apply(profile); if (credentialId != null) { quarantine.quarantine().remainingSeconds(credentialId).ifPresent(remaining -> { diff --git a/fleetd/src/test/java/dev/ltms/fleet/FleetdLeadSeatLookupTest.java b/fleetd/src/test/java/dev/ltms/fleet/FleetdLeadSeatLookupTest.java new file mode 100644 index 0000000..b1b4219 --- /dev/null +++ b/fleetd/src/test/java/dev/ltms/fleet/FleetdLeadSeatLookupTest.java @@ -0,0 +1,129 @@ +package dev.ltms.fleet; + +import dev.ltms.fleet.config.FleetConfig; +import org.junit.jupiter.api.DisplayName; +import org.junit.jupiter.api.Test; + +import java.util.Map; +import java.util.function.Function; + +import static org.junit.jupiter.api.Assertions.assertEquals; + +/** + * fleetd #176: {@link Fleetd#leadSeatLookup} is the factory {@code Fleetd.main} wires into {@code + * FleetMcp.LeadSeatSource} so {@code fleet_list}'s {@code free} can subtract the seat(s) a + * {@code subscription: true} profile's own live LEAD session holds on that same account — + * {@code maxLoad} never counted the lead, only members. {@code FleetdLeadSeatWiringTest} proves + * {@code main} still passes this factory's result in; this class proves the factory's own matching + * logic: subscription-only, credential-matched, and counting only CURRENTLY LIVE leads. + */ +class FleetdLeadSeatLookupTest { + + private static FleetConfig.Profile subscriptionProfile(String name, String credentialId) { + return new FleetConfig.Profile(name, null, "claude-sonnet-5", null, null, null, + "tab", "fleet", "w #{n}", null, null, null, null, null, null, null, + null, 3, true, null, credentialId, null); + } + + private static FleetConfig.Profile offSubscriptionProfile(String name, String credentialId) { + return new FleetConfig.Profile(name, "http://gx00.gw:8000", "coder", null, "FLEETD_WORKER_TOKEN", + null, "tab", "fleet", "w #{n}", null, null, null, null, null, null, null, + null, 2, false, null, credentialId, null); + } + + private static FleetConfig.Leader leadOnProfile(String profile) { + return new FleetConfig.Leader(profile, "lead: primary", 1, "lead:", 10, "claude", "claude-sonnet-5"); + } + + @Test + @DisplayName("a live lead sharing the target profile's credential counts as one seat") + void liveLeadSharingCredentialCountsAsOneSeat() { + Map profiles = Map.of("sonnet", subscriptionProfile("sonnet", null)); + Map leaders = Map.of("primary", leadOnProfile("sonnet")); + Function lookup = Fleetd.leadSeatLookup(() -> profiles, leaders, + () -> Map.of("term_primary", "primary")); + + assertEquals(1, lookup.apply("sonnet")); + } + + @Test + @DisplayName("no live lead names this profile ⇒ zero seats, exactly as before this ticket") + void noLiveLeadOnTheProfileCountsAsZero() { + Map profiles = Map.of("sonnet", subscriptionProfile("sonnet", null)); + Map leaders = Map.of("primary", leadOnProfile("sonnet")); + Function lookup = Fleetd.leadSeatLookup(() -> profiles, leaders, Map::of); + + assertEquals(0, lookup.apply("sonnet")); + } + + @Test + @DisplayName("a lead entry with no `profile:` (recognise-only) contributes no seats — cannot be derived") + void recogniseOnlyLeadWithNoProfileContributesNothing() { + Map profiles = Map.of("sonnet", subscriptionProfile("sonnet", null)); + FleetConfig.Leader recogniseOnly = new FleetConfig.Leader(null, "lead: primary", 1, "lead:", 10, + "claude", "claude-sonnet-5"); + Map leaders = Map.of("primary", recogniseOnly); + Function lookup = Fleetd.leadSeatLookup(() -> profiles, leaders, + () -> Map.of("term_primary", "primary")); + + assertEquals(0, lookup.apply("sonnet")); + } + + @Test + @DisplayName("a non-subscription profile never has a lead seat subtracted, whatever the credential match") + void nonSubscriptionProfileIsNeverAdjusted() { + Map profiles = Map.of( + "terra", offSubscriptionProfile("terra", "shared-openai"), + "sonnet", subscriptionProfile("sonnet", "shared-openai")); + Map leaders = Map.of("primary", leadOnProfile("sonnet")); + Function lookup = Fleetd.leadSeatLookup(() -> profiles, leaders, + () -> Map.of("term_primary", "primary")); + + assertEquals(0, lookup.apply("terra"), "terra is not subscription:true, so it must never be adjusted"); + } + + @Test + @DisplayName("credential mismatch ⇒ no seat counted, even though both profiles are subscription:true") + void differentCredentialIsNotCounted() { + Map profiles = Map.of( + "sonnet", subscriptionProfile("sonnet", "claude-account-a"), + "opus", subscriptionProfile("opus", "claude-account-b")); + Map leaders = Map.of("primary", leadOnProfile("opus")); + Function lookup = Fleetd.leadSeatLookup(() -> profiles, leaders, + () -> Map.of("term_primary", "primary")); + + assertEquals(0, lookup.apply("sonnet"), "different accounts must never be conflated into one seat count"); + } + + @Test + @DisplayName("two live instances of the same lead count as two seats") + void twoLiveInstancesOfTheSameLeadCountAsTwoSeats() { + Map profiles = Map.of("sonnet", subscriptionProfile("sonnet", null)); + Map leaders = Map.of("primary", leadOnProfile("sonnet")); + Function lookup = Fleetd.leadSeatLookup(() -> profiles, leaders, + () -> Map.of("term_a", "primary", "term_b", "primary")); + + assertEquals(2, lookup.apply("sonnet")); + } + + @Test + @DisplayName("an unconfigured target profile resolves to zero, not a thrown exception") + void unconfiguredTargetProfileIsZero() { + Function lookup = Fleetd.leadSeatLookup(Map::of, Map.of(), Map::of); + + assertEquals(0, lookup.apply("ghost")); + } + + @Test + @DisplayName("live leads are read through the supplier on every call, not snapshotted") + void liveLeadsAreReadThroughOnEveryCall() { + Map profiles = Map.of("sonnet", subscriptionProfile("sonnet", null)); + Map leaders = Map.of("primary", leadOnProfile("sonnet")); + java.util.Map live = new java.util.HashMap<>(); + Function lookup = Fleetd.leadSeatLookup(() -> profiles, leaders, () -> live); + + assertEquals(0, lookup.apply("sonnet")); + live.put("term_primary", "primary"); + assertEquals(1, lookup.apply("sonnet")); + } +} diff --git a/fleetd/src/test/java/dev/ltms/fleet/FleetdLeadSeatWiringTest.java b/fleetd/src/test/java/dev/ltms/fleet/FleetdLeadSeatWiringTest.java new file mode 100644 index 0000000..29ecdc3 --- /dev/null +++ b/fleetd/src/test/java/dev/ltms/fleet/FleetdLeadSeatWiringTest.java @@ -0,0 +1,43 @@ +package dev.ltms.fleet; + +import java.nio.file.Files; +import java.nio.file.Path; +import org.junit.jupiter.api.DisplayName; +import org.junit.jupiter.api.Test; + +import static org.junit.jupiter.api.Assertions.assertTrue; + +/** + * fleetd #176: {@code Fleetd.main} builds its {@code FleetMcp} from a 14-argument constructor whose + * last argument is a {@code FleetMcp.LeadSeatSource} wrapping {@link Fleetd#leadSeatLookup}. That + * argument is exactly the kind of wiring fleetd #248 warned about: dropping it (or swapping it for + * the inert {@code FleetMcp.LeadSeatSource.none()}) compiles with 0 errors and leaves every test + * that builds its own {@code FleetMcp}/{@code CapacitySource} directly — every test that predates + * this ticket — green, because none of them go through {@code main} at all. + * + *

{@link FleetdLeadSeatLookupTest} proves the factory's own matching logic; this class is the + * plain source-text assertion that proves {@code main} still passes its result in, mirroring + * {@code FleetdCompletionResolverWiringTest}'s approach for the same class of gap. + * + *

This test checks source text, not runtime behaviour. It never constructs a + * {@code FleetMcp} and never runs {@code main}. + */ +class FleetdLeadSeatWiringTest { + + private static String fleetdSource() throws Exception { + return Files.readString(Path.of("src/main/java/dev/ltms/fleet/Fleetd.java")); + } + + @Test + @DisplayName("[SOURCE TEXT] FleetMcp's construction call still passes a LeadSeatSource built from leadSeatLookup(...)") + void fleetMcpConstructionStillWiresLeadSeatLookup() throws Exception { + String source = fleetdSource(); + assertTrue(source.contains("new FleetMcp.LeadSeatSource(leadSeatLookup(() -> config.get().profiles(), " + + "leaders, leads))"), + "FleetMcp's construction call must still pass a LeadSeatSource built from " + + "Fleetd.leadSeatLookup(...). Dropping it or swapping in " + + "FleetMcp.LeadSeatSource.none() (fleetd #176's would-be silent regression, the same " + + "shape as fleetd #248's measured mutations) compiles with 0 errors and leaves every " + + "existing behavioural test green — this source check is what must go red instead."); + } +} diff --git a/fleetd/src/test/java/dev/ltms/fleet/mcp/FleetMcpTest.java b/fleetd/src/test/java/dev/ltms/fleet/mcp/FleetMcpTest.java index 600e3e7..d3c3973 100644 --- a/fleetd/src/test/java/dev/ltms/fleet/mcp/FleetMcpTest.java +++ b/fleetd/src/test/java/dev/ltms/fleet/mcp/FleetMcpTest.java @@ -767,6 +767,68 @@ class FleetMcpTest { assertFalse(out.contains("quarantinedForSeconds"), out); } + /** + * fleetd #176: this is the exact shape measured on the Mac fleet — {@code maxLoad:3, live:2}, + * where one of the "free" three is really the lead's own seat on the same subscription. The old + * formula ({@code max(0, cap - live)}) reported {@code free:1}; the real ceiling is {@code 0} + * (two members plus the lead's own seat already fill all three). + */ + @Test + void leadSeatSubtractsFromFreeTheSameWayLiveDoes() { + FakeHerdr h = new FakeHerdr(); + SessionManager sessions = new SessionManager(workerService(h, "http://gx00.gw:8000", Set.of("gx00.gw"))); + FleetMcp.LeadSeatSource leadSeats = new FleetMcp.LeadSeatSource( + profile -> "sonnet".equals(profile) ? 1 : 0); + + String out = textOf(FleetMcp.listFleet(workerService(h, "http://gx00.gw:8000", Set.of("gx00.gw")), + sessions, null, new FleetMcp.CapacitySource(profile -> 2, profile -> 3, + () -> Set.of("sonnet"), () -> 0), new FleetMcp.HealthCoverageSource(() -> "off"), + FleetMcp.QuarantineSource.none(), FleetMcp.OutageSource.none(), leadSeats, Map.of(), "", null)); + + assertTrue(out.contains("\"maxLoad\":3"), "maxLoad itself must be left untouched: " + out); + assertTrue(out.contains("\"live\":2"), out); + assertTrue(out.contains("\"free\":0"), "2 live + 1 lead seat fills all 3: " + out); + assertTrue(out.contains("\"leadSeats\":1"), out); + } + + /** + * fleetd #176: the OTHER measurement in the issue — a completely idle fleet still overstates + * {@code free} by the lead's own seat. {@code maxLoad:3, live:0} must report {@code free:2}, the + * real fan-out ceiling, not {@code 3}. + */ + @Test + void leadSeatLowersFreeOnAnOtherwiseIdleSubscriptionProfile() { + FakeHerdr h = new FakeHerdr(); + SessionManager sessions = new SessionManager(workerService(h, "http://gx00.gw:8000", Set.of("gx00.gw"))); + FleetMcp.LeadSeatSource leadSeats = new FleetMcp.LeadSeatSource( + profile -> "sonnet".equals(profile) ? 1 : 0); + + String out = textOf(FleetMcp.listFleet(workerService(h, "http://gx00.gw:8000", Set.of("gx00.gw")), + sessions, null, new FleetMcp.CapacitySource(profile -> 0, profile -> 3, + () -> Set.of("sonnet"), () -> 0), new FleetMcp.HealthCoverageSource(() -> "off"), + FleetMcp.QuarantineSource.none(), FleetMcp.OutageSource.none(), leadSeats, Map.of(), "", null)); + + assertTrue(out.contains("\"live\":0"), out); + assertTrue(out.contains("\"free\":2"), "an idle fleet's real ceiling is 3 minus the lead's own seat: " + out); + assertTrue(out.contains("\"leadSeats\":1"), out); + } + + /** A profile with no lead seats reported must be byte-identical to before this ticket. */ + @Test + void zeroLeadSeatsOmitsTheKeyAndLeavesFreeUnchanged() { + FakeHerdr h = new FakeHerdr(); + SessionManager sessions = new SessionManager(workerService(h, "http://gx00.gw:8000", Set.of("gx00.gw"))); + + String out = textOf(FleetMcp.listFleet(workerService(h, "http://gx00.gw:8000", Set.of("gx00.gw")), + sessions, null, new FleetMcp.CapacitySource(profile -> 0, profile -> 2, + () -> Set.of("terra"), () -> 0), new FleetMcp.HealthCoverageSource(() -> "off"), + FleetMcp.QuarantineSource.none(), FleetMcp.OutageSource.none(), FleetMcp.LeadSeatSource.none(), + Map.of(), "", null)); + + assertTrue(out.contains("\"free\":2"), out); + assertFalse(out.contains("leadSeats"), "no lead shares this profile's credential: " + out); + } + @Test void listReportsLeadsAndFlagsTheCallersOwnRow() { FakeHerdr h = new FakeHerdr(); From 01a840cc1431661b66fd3882748b8e041af67126 Mon Sep 17 00:00:00 2001 From: Dai Ha Date: Thu, 3 Sep 2026 13:06:11 +0700 Subject: [PATCH 2/4] #248 follow-up: drive the real backendErrorSink, not a copy of it BackendOutageFlowTest held a ~30-line hand-copy of the lambda in Fleetd.main, under a comment promising it mirrored production "EXACTLY". That promise was the defect. The test proved the copy, so any change to the real sink left the flow test green. #248 made Fleetd.backendErrorSink(...) public for exactly this reason. The test now calls it. Measured, same mutation in the real sink (an early return after sessions.onBackendError, dropping the cool-off and the lead nudge): old test (hand-copy): Tests run: 5, Failures: 0 -- blind new test (real sink): Tests run: 5, Failures: 4 -- catches it 0 compile errors in both runs, so both are real results. Production reverted and confirmed with diff -q. Full build: 1234 tests, 0 failures, 0 compile errors. --- .../fleet/inject/BackendOutageFlowTest.java | 43 +++++-------------- 1 file changed, 10 insertions(+), 33 deletions(-) diff --git a/fleetd/src/test/java/dev/ltms/fleet/inject/BackendOutageFlowTest.java b/fleetd/src/test/java/dev/ltms/fleet/inject/BackendOutageFlowTest.java index c82ed4e..cf2b1ea 100644 --- a/fleetd/src/test/java/dev/ltms/fleet/inject/BackendOutageFlowTest.java +++ b/fleetd/src/test/java/dev/ltms/fleet/inject/BackendOutageFlowTest.java @@ -2,6 +2,7 @@ package dev.ltms.fleet.inject; import com.fasterxml.jackson.databind.JsonNode; import com.fasterxml.jackson.databind.ObjectMapper; +import dev.ltms.fleet.Fleetd; import dev.ltms.fleet.config.FleetConfig; import dev.ltms.fleet.guard.SubscriptionGuard; import dev.ltms.fleet.herdr.AgentControl; @@ -146,40 +147,16 @@ class BackendOutageFlowTest { pushLoop = new ReplyPushLoop(registry, new AgentControl(leadClient), inbox, scheduler, 3, 50); AtomicReference pushLoopRef = new AtomicReference<>(pushLoop); - // --- mirrors Fleetd.main's backendErrorSink lambda EXACTLY: (1) mark BACKEND_ERROR, - // (2) resolve profile/credential via the roster, fail-loud + notify unmapped-target, - // (3) record in BackendOutagePolicy, (4) on a NEW incident, notify the lead. ------------ - BackendErrorSink backendErrorSink = (target, matchedLine, reason) -> { - sessions.onBackendError(target, reason); + // fleetd #248 follow-up: this used to be a 30-line hand-copy of Fleetd.main's + // backendErrorSink lambda, with a comment promising it mirrored production "EXACTLY". + // That promise is exactly the problem: a copy proves the copy. Editing or deleting the + // real sink left this whole flow test green, because it never touched the real sink. + // #248 made Fleetd.backendErrorSink public precisely so a cross-package test could + // drive the real object, so this now calls it. Every assertion below is about + // production code again. + BackendErrorSink backendErrorSink = + Fleetd.backendErrorSink(sessions, () -> profiles, outagePolicy, pushLoopRef::get); - String profileName = sessions.roster().stream() - .filter(session -> target.equals(session.terminalId())) - .findFirst() - .map(MemberSession::profile) - .orElse(null); - FleetConfig.Profile profile = profileName == null ? null : profiles.get(profileName); - if (profile == null) { - ReplyPushLoop loop = pushLoopRef.get(); - if (loop != null) { - loop.onBackendTargetUnmapped(target, reason); - } - return; - } - String credentialId = profile.effectiveCredentialId(); - Optional incident = outagePolicy.record(credentialId, target, reason); - incident.ifPresent(inc -> { - List affectedProfiles = profiles.values().stream() - .filter(p -> credentialId.equals(p.effectiveCredentialId())) - .map(FleetConfig.Profile::profile) - .sorted() - .toList(); - ReplyPushLoop loop = pushLoopRef.get(); - if (loop != null) { - loop.onBackendIncident(inc.id(), inc.targets(), credentialId, affectedProfiles, - (int) inc.remainingCoolOffSeconds()); - } - }); - }; BackendErrorPatternLookup patterns = target -> Pattern.compile("(?i)503 Service Unavailable"); resolver = new CompletionResolver(new AgentControl(herdr), rendezvous, ExhaustedPatternLookup.none(), ExhaustionSink.none(), patterns, backendErrorSink); From c50f5b2d61c77adff02bbb6194e4bd35360c2548 Mon Sep 17 00:00:00 2001 From: Dai Ha Date: Thu, 3 Sep 2026 13:10:10 +0700 Subject: [PATCH 3/4] fleetd #176 stage 2: make effectiveCredentialId() subscription-aware Stage 1's lead-seat matcher (leadSeatLookup) was correct but inert on the live host: the lead runs on profile 'opus', members on 'sonnet', both subscription:true with no explicit credentialId. Because effectiveCredentialId() fell back to the profile's own name, opus and sonnet never matched even though they share one Claude login, so the matcher charged zero seats. FleetConfig.Profile.effectiveCredentialId() now falls back to a shared sentinel (SUBSCRIPTION_CREDENTIAL_ID = "") instead of the profile name when subscription:true and credentialId is unset. An explicit credentialId still wins, so two separate Claude logins on one host can still be kept apart. This is also BackendQuarantine's and BackendOutagePolicy's grouping key and CompositePeerLauncher's spawn-time enforcement key, so the fix also links quarantine/cool-off across subscription profiles sharing an account -- intentional: one usage limit really does take out every profile on that login, mirroring credentialId: openai-shared already doing this for off-subscription profiles. Every caller was reviewed; none wants "this exact profile" over "this account". Tests added: - FleetdLeadSeatLookupTest: the live shape itself (lead on a DIFFERENT subscription profile than the target, same account, neither sets credentialId) -- the case stage 1's suite never covered - FleetMcpTest: quarantining one subscription profile's shared account zeroes free on another sharing it, via the same effectiveCredentialId()-driven wiring Fleetd.main uses Mutation-tested: reverting the subscription branch to the old fall-back-to-profile-name behavior sends both new tests RED with 0 compile errors; reverting the mutation restores byte-identical (diff -q) source and green tests. fleetd.example.yaml's fleetd #176 notes are rewritten for the sentinel semantics and when to override it with an explicit credentialId. --- fleetd/fleetd.example.yaml | 39 +++++++++++++--- .../dev/ltms/fleet/config/FleetConfig.java | 44 ++++++++++++++++--- .../ltms/fleet/FleetdLeadSeatLookupTest.java | 31 ++++++++++++- .../java/dev/ltms/fleet/mcp/FleetMcpTest.java | 42 ++++++++++++++++++ 4 files changed, 141 insertions(+), 15 deletions(-) diff --git a/fleetd/fleetd.example.yaml b/fleetd/fleetd.example.yaml index da3b057..dbf3383 100644 --- a/fleetd/fleetd.example.yaml +++ b/fleetd/fleetd.example.yaml @@ -310,11 +310,31 @@ profiles: # GOTCHA 3 (fleetd #176) — `maxLoad` counts members, never the lead itself. The lead is a live # `claude` session on this SAME account (a lead is never moved off-subscription, whatever its # own profile says), so it already holds one seat before any member spawns. If a lead's - # `fleet.leaders..profile` names THIS profile (or one sharing its `credentialId`), - # `fleet_list`'s `free` for this profile subtracts that lead's live seat(s) automatically — see - # `profile:` under THE FLEET below. If no lead entry names this profile, fleetd has no way to - # know it shares this account, and `free` will overstate what a fresh `fleet_spawn` actually - # gets by exactly the seats the lead is quietly holding. + # `fleet.leaders..profile` names THIS profile — or ANY OTHER `subscription: true` + # profile that shares this one's account (see THE SENTINEL, just below, next to + # `credentialId:`) — `fleet_list`'s `free` for this profile subtracts that lead's live + # seat(s) automatically; see `profile:` under THE FLEET below. If no lead entry names a + # profile sharing this account, fleetd has no way to know a lead holds a seat here, and `free` + # will overstate what a fresh `fleet_spawn` actually gets by exactly the seats the lead is + # quietly holding. + # + # THE SENTINEL (fleetd #176 stage 2, correcting an inert stage 1 fix): every `subscription: + # true` profile that leaves `credentialId` unset shares ONE implicit account-wide credential + # id with every other such profile on this host — because a subscription profile doesn't + # authenticate with a credential of its own, it authenticates as the operator's own Claude + # login, and there is exactly one of those. So on a typical host, `opus` (the lead's profile) + # and `sonnet` (the members' profile) are linked automatically, with NOTHING to set here — that + # is what makes GOTCHA 3 above work without also writing matching `credentialId:` values on + # both. This linkage is not just cosmetic: it is the same key `BackendQuarantine`/cool-off use, + # so a usage-limit hit on `opus` now quarantines `sonnet` too (and vice versa) — correct, since + # they are one Claude account, but worth knowing before you wonder why an unrelated-looking + # profile went quarantined. + # + # WHEN TO OVERRIDE — set explicit, DIFFERENT `credentialId:` values on two `subscription: true` + # profiles only when they are genuinely two separate Claude logins on the same host (a real, + # if unusual, setup). An explicit `credentialId` always wins over the sentinel, so this is the + # one way to keep two subscription profiles from being treated as one account for lead-seat + # counting AND for quarantine/cool-off grouping alike. # gitTokenEnv: GITEA_TOKEN # opt-in: let this profile's workers open their own PR (CB-302) # gitHostEnv: GITEA_HOST # defaults to GITEA_HOST; injected only with gitTokenEnv # exhaustedPattern: "usage limit has been reached" # opt-in: classify a usage-limit refusal (CB-578) @@ -516,8 +536,13 @@ fleet: # auto-launched: it is also how fleetd learns which account this lead's own session shares. A # `subscription: true` profile bills the operator's Claude account, and the lead itself is always # a live `claude` session on that same account — `maxLoad` never counted that seat. If a lead - # entry here names a profile (or one sharing its `credentialId`) that matches a worker profile, - # `fleet_list`'s `free` for that worker profile subtracts the lead's live seat(s) automatically. + # entry here names a profile that shares a worker profile's account, `fleet_list`'s `free` for + # that worker profile subtracts the lead's live seat(s) automatically. "Shares the account" is + # decided by matching `effectiveCredentialId()`, which (fleetd #176 stage 2 — see THE SENTINEL, + # next to `credentialId:`, in THE WORKERS above) means: an explicit, matching `credentialId:` on + # both, OR — the common case, needing NO extra config — both being `subscription: true` with + # `credentialId` left unset, since those all share one implicit account-wide id. A lead on `opus` + # and workers on `sonnet` link automatically this way; they do NOT need the same profile name. # Setting `profile:` on an already-running, recognise-only lead is safe — the daemon only launches # the SHORTFALL below `instances`, so naming a profile here does not, by itself, start anything. # Omit it and fleetd has no way to derive the sharing — there is no other reliable signal on the diff --git a/fleetd/src/main/java/dev/ltms/fleet/config/FleetConfig.java b/fleetd/src/main/java/dev/ltms/fleet/config/FleetConfig.java index e507886..f0e4662 100644 --- a/fleetd/src/main/java/dev/ltms/fleet/config/FleetConfig.java +++ b/fleetd/src/main/java/dev/ltms/fleet/config/FleetConfig.java @@ -644,14 +644,46 @@ public record FleetConfig( } /** - * The credential group this profile quarantines with (CB-578 stage B): the configured - * {@link #credentialId} when set, else this profile's own name — so an unconfigured profile - * quarantines alone, exactly as it did before this field existed. Two profiles that set the - * same non-blank {@code credentialId} share one quarantine: a {@code BACKEND_EXHAUSTED} - * classification on either one quarantines both. + * The shared credential id every {@code subscription: true} profile falls back to when it + * sets no explicit {@link #credentialId} (fleetd #176 stage 2, correcting an inert first cut + * of that ticket). A subscription profile has no credential of its own to fall back to its + * name for: it authenticates as the operator's own Claude login, and a host has exactly one + * of those, whatever names the operator gives the profiles running on it. Falling back to the + * profile's own name (the way an ordinary off-subscription profile does) would keep two + * subscription profiles on one login apart from each other, which is the opposite of what + * "one account" means. + * + *

Measured live and what it broke: a lead on profile {@code opus}, members on profile + * {@code sonnet}, same Claude login, neither setting {@code credentialId}. Before this + * sentinel, {@code opus.effectiveCredentialId()} was {@code "opus"} and {@code sonnet + * .effectiveCredentialId()} was {@code "sonnet"} — so fleetd #176's lead-seat matcher (and, + * this sentinel now also fixes, {@code CompositePeerLauncher}'s quarantine/cool-off spawn + * refusal and {@code BackendOutagePolicy}'s incident grouping) silently never linked them: the + * fix shipped, and stayed inert on the one host it was written for. + */ + public static final String SUBSCRIPTION_CREDENTIAL_ID = ""; + + /** + * The credential group this profile quarantines with (CB-578 stage B; extended fleetd #176 + * stage 2 — see {@link #SUBSCRIPTION_CREDENTIAL_ID}): the configured {@link #credentialId} + * when set — that always wins, so an operator with two separate Claude logins on one host can + * still keep them apart. Otherwise, a {@code subscription: true} profile falls back to + * {@link #SUBSCRIPTION_CREDENTIAL_ID} rather than its own name; an ordinary off-subscription + * profile falls back to its own name, exactly as it did before this field existed, so an + * unconfigured off-subscription profile still quarantines alone. + * + *

A {@code BACKEND_EXHAUSTED} (or repeated backend-error) classification on any profile + * sharing the result quarantines/cools off every profile that shares it — including, now, + * every {@code subscription: true} profile with no explicit {@code credentialId}. That is + * intended, not incidental: one Claude subscription hitting a usage limit really does take out + * every profile running on it, the same way {@code credentialId: openai-shared} already lets + * two OpenAI-backed profiles share one quarantine. */ public String effectiveCredentialId() { - return (credentialId == null || credentialId.isBlank()) ? profile : credentialId; + if (credentialId != null && !credentialId.isBlank()) { + return credentialId; + } + return isSubscription() ? SUBSCRIPTION_CREDENTIAL_ID : profile; } /** True when this profile's workers are granted a forge token to open their own PR (CB-302). */ diff --git a/fleetd/src/test/java/dev/ltms/fleet/FleetdLeadSeatLookupTest.java b/fleetd/src/test/java/dev/ltms/fleet/FleetdLeadSeatLookupTest.java index b1b4219..d81b89a 100644 --- a/fleetd/src/test/java/dev/ltms/fleet/FleetdLeadSeatLookupTest.java +++ b/fleetd/src/test/java/dev/ltms/fleet/FleetdLeadSeatLookupTest.java @@ -83,7 +83,8 @@ class FleetdLeadSeatLookupTest { } @Test - @DisplayName("credential mismatch ⇒ no seat counted, even though both profiles are subscription:true") + @DisplayName("explicit, different credentialIds still separate two subscription profiles (post fleetd #176 " + + "stage 2 sentinel)") void differentCredentialIsNotCounted() { Map profiles = Map.of( "sonnet", subscriptionProfile("sonnet", "claude-account-a"), @@ -92,7 +93,33 @@ class FleetdLeadSeatLookupTest { Function lookup = Fleetd.leadSeatLookup(() -> profiles, leaders, () -> Map.of("term_primary", "primary")); - assertEquals(0, lookup.apply("sonnet"), "different accounts must never be conflated into one seat count"); + assertEquals(0, lookup.apply("sonnet"), "different accounts must never be conflated into one seat count " + + "— an explicit credentialId on both sides must still win over the subscription sentinel, so an " + + "operator with two separate Claude logins on one host can keep them apart"); + } + + /** + * fleetd #176 stage 2 — the exact live shape that shipped inert: a lead on subscription profile + * {@code opus}, members on a DIFFERENTLY NAMED subscription profile {@code sonnet}, same Claude + * login, and NEITHER profile sets {@code credentialId}. Every other test in this class puts the + * lead on the SAME profile name as the target, which happened to keep working even with the old + * fall-back-to-profile-name {@code effectiveCredentialId()} — this is the one that did not, and + * its absence is what let the stage-1 fix ship without ever catching the bug it was filed for. + */ + @Test + @DisplayName("[LIVE SHAPE] lead on a DIFFERENT subscription profile, same account, neither sets " + + "credentialId ⇒ still counts as a seat") + void leadOnADifferentSubscriptionProfileSameAccountStillCountsAsASeat() { + Map profiles = Map.of( + "opus", subscriptionProfile("opus", null), + "sonnet", subscriptionProfile("sonnet", null)); + Map leaders = Map.of("primary", leadOnProfile("opus")); + Function lookup = Fleetd.leadSeatLookup(() -> profiles, leaders, + () -> Map.of("term_primary", "primary")); + + assertEquals(1, lookup.apply("sonnet"), "opus and sonnet are both subscription:true with no explicit " + + "credentialId, so they share one Claude login and the lead's live seat on opus must be charged " + + "against sonnet too — this is the live host's actual shape (fleetd #176 stage 2)"); } @Test diff --git a/fleetd/src/test/java/dev/ltms/fleet/mcp/FleetMcpTest.java b/fleetd/src/test/java/dev/ltms/fleet/mcp/FleetMcpTest.java index d3c3973..b6237b1 100644 --- a/fleetd/src/test/java/dev/ltms/fleet/mcp/FleetMcpTest.java +++ b/fleetd/src/test/java/dev/ltms/fleet/mcp/FleetMcpTest.java @@ -651,6 +651,48 @@ class FleetMcpTest { assertEquals(2, out.split("\"free\":0", -1).length - 1, out); } + /** + * fleetd #176 stage 2 (correcting the inert stage 1): two {@code subscription: true} profiles, + * {@code opus} and {@code sonnet}, neither setting an explicit {@code credentialId} — the exact + * shape measured on the live Mac fleet. This is INTENDED, not a regression: a real Claude usage + * limit on the one login behind both profiles really does take out every profile running on it, + * the same way {@code credentialId: openai-shared} already lets two OpenAI-backed profiles share + * one quarantine (see {@code everyProfileSharingTheQuarantinedCredentialReportsZeroFree} above). + * The {@code credentialIdFor} function here is built the same way {@code Fleetd.main} wires it — + * {@code profile -> profiles.get(profile).effectiveCredentialId()} — so this proves the actual + * config-driven behaviour, not just {@code capacityView}'s arithmetic with a hand-picked string. + */ + @Test + void quarantiningOneSubscriptionProfileZeroesFreeOnTheOtherSharingTheSameAccount() { + FakeHerdr h = new FakeHerdr(); + SessionManager sessions = new SessionManager(workerService(h, "http://gx00.gw:8000", Set.of("gx00.gw"))); + Map profiles = Map.of( + "opus", new FleetConfig.Profile("opus", null, "claude-opus-4", null, null, null, + "tab", "fleet", "w #{n}", null, null, null, null, null, null, null, + null, 3, true, null, null, null), + "sonnet", new FleetConfig.Profile("sonnet", null, "claude-sonnet-5", null, null, null, + "tab", "fleet", "w #{n}", null, null, null, null, null, null, null, + null, 3, true, null, null, null)); + BackendQuarantine quarantine = new BackendQuarantine(() -> 0L, TimeUnit.MINUTES.toNanos(20)); + quarantine.quarantine(FleetConfig.Profile.SUBSCRIPTION_CREDENTIAL_ID); + FleetMcp.QuarantineSource source = new FleetMcp.QuarantineSource(profile -> { + FleetConfig.Profile configured = profiles.get(profile); + return configured == null ? null : configured.effectiveCredentialId(); + }, quarantine); + + String out = textOf(FleetMcp.listFleet(workerService(h, "http://gx00.gw:8000", Set.of("gx00.gw")), + sessions, null, new FleetMcp.CapacitySource(profile -> 0, profile -> 3, + () -> Set.of("opus", "sonnet"), () -> 0), new FleetMcp.HealthCoverageSource(() -> "off"), + source, Map.of(), "")); + + assertEquals(2, out.split("\"free\":0", -1).length - 1, + "opus and sonnet share one Claude login with neither setting credentialId, so quarantining " + + "opus's account must also zero sonnet's free — this is intended, not a side effect: " + + out); + assertEquals(2, out.split("\"credentialId\":\"" + FleetConfig.Profile.SUBSCRIPTION_CREDENTIAL_ID + "\"", -1) + .length - 1, out); + } + /** fleetd #201 Unit 5: cool-off forces {@code free:0} but never adds {@code quarantinedForSeconds}. */ @Test void coolingOffProfileReportsZeroFreeButNeverQuarantinedForSeconds() { From 21c539f22ed16a3c75d73894d672ab397f234dfe Mon Sep 17 00:00:00 2001 From: Dai Ha Date: Thu, 3 Sep 2026 13:18:50 +0700 Subject: [PATCH 4/4] #113: derive the config guard from the record tree, both directions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The existing guard walks KNOWN_TOP_LEVEL_KEYS and anchors its regex at column 0, so it sees only top-level keys. Every nested key was outside its scope and nothing said so, which is the shape #113 collects: a checker narrower than it looks, whose green run stops anyone looking. Two derived guards replace the assumption: everyNestedConfigKeyIsDocumentedInTheExample walks FleetConfig's record components (17 records, 83 distinct key names) and requires each to be documented in the example. everyLiveKeyInTheExampleBindsToARecordComponent resolves every live key path in the example against the record tree, so a documented key that binds to nothing fails here instead of being silently ignored in production. Neither carries a list, so a key added to any nested record is covered the moment it compiles (criterion 2). Both mutations run through the real caller, not the helper (criterion 1, which asks for exactly that): removed every mention of paneProbeIntervalSeconds from the example -> FAILS, naming health.paneProbeIntervalSeconds added a live bind.totallyMadeUpKnob to the example -> FAILS, naming bind.totallyMadeUpKnob 0 compile errors in both; both reverted and confirmed with diff -q. The first attempt at mutation 1 removed only the `key:` line and the run stayed green — correctly, because the key was still documented in prose. An incomplete mutation proves nothing, so it was redone. Denominators (criterion 3): both guards print how many keys they checked, and the floor for "did the walk descend?" is derived from KNOWN_TOP_LEVEL_KEYS.size() rather than being a literal. Scope is stated in the javadoc rather than implied: the guards do not check a key sits at the right path, do not parse commented prose for the reverse direction, and do not prove a parsed key is read by anything. paneProbeIntervalSeconds is parsed and read by nothing, and these guards pass it -- the example already says so in its own text. everyOptionalKnobDocumentedInTheExampleBinds keeps its hand-written list but is re-documented as a value-binding spot check, explicitly not a coverage guard; coverage now comes from the two derived tests. broker.uri is documented only in the example's prose convention (`# uri -> ...`), never as a copy-pasteable `uri:` key, because writing it out invites pasting a password into a file -- the thing uriEnv exists to avoid. The matcher accepts that convention rather than pushing the file toward doing it. Full build: 1236 tests, 0 failures, 0 compile errors. --- .../ltms/fleet/config/FleetConfigTest.java | 246 +++++++++++++++++- 1 file changed, 243 insertions(+), 3 deletions(-) diff --git a/fleetd/src/test/java/dev/ltms/fleet/config/FleetConfigTest.java b/fleetd/src/test/java/dev/ltms/fleet/config/FleetConfigTest.java index 2ba37ec..bf07edf 100644 --- a/fleetd/src/test/java/dev/ltms/fleet/config/FleetConfigTest.java +++ b/fleetd/src/test/java/dev/ltms/fleet/config/FleetConfigTest.java @@ -6,11 +6,18 @@ import dev.ltms.fleet.peer.MemberRole; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.io.TempDir; +import java.lang.reflect.ParameterizedType; +import java.lang.reflect.RecordComponent; +import java.lang.reflect.Type; import java.nio.file.Files; import java.nio.file.Path; +import java.util.ArrayList; +import java.util.HashSet; import java.util.List; import java.util.Map; import java.util.Set; +import java.util.TreeMap; +import java.util.regex.Matcher; import java.util.regex.Pattern; import static org.junit.jupiter.api.Assertions.*; @@ -1366,9 +1373,18 @@ class FleetConfigTest { } /** - * Every optional knob the example documents must bind under the exact spelling used there. - * Keep this list in step with {@code fleetd.example.yaml}: a rename that updates the record - * but not the example (or vice versa) fails here instead of silently no-op'ing in production. + * A spot check that the knobs listed below bind under the exact spelling the example uses — + * it asserts real VALUES arrive in the record, which no name-matching guard can do. + * + *

This is NOT a coverage guard, and must not be read as one (fleetd #113). The list + * inside it is hand-written, so it only ever covers what someone remembered to add. Coverage + * — "is every key the code reads documented, and does every documented key bind?" — comes + * from {@link #everyNestedConfigKeyIsDocumentedInTheExample} and + * {@link #everyLiveKeyInTheExampleBindsToARecordComponent}, both of which derive their key + * set from the record tree and therefore cannot drift. + * + *

Adding a knob here is optional. Leaving one out is not a coverage gap, because the two + * derived guards above already fail on an undocumented or unbindable key. */ @Test void everyOptionalKnobDocumentedInTheExampleBinds(@TempDir Path dir) throws Exception { @@ -1525,6 +1541,230 @@ class FleetConfigTest { return p.matcher(yaml).find(); } + /** + * The nested half of {@link #everyKnownTopLevelKeyIsDocumentedInTheExample} (fleetd #113). + * + *

That guard walks {@link FleetConfig#KNOWN_TOP_LEVEL_KEYS} and anchors its regex at + * column 0, so it sees ONLY top-level keys. Every nested key — {@code profiles..model}, + * {@code health.paneProbeIntervalSeconds} and a hundred others — is outside its scope, and + * neither its name nor its output says so. A green run then reads as "the example documents + * the schema" when most of the schema was never looked at. + * + *

This walks the record tree rather than a name list, so a key added to any nested record + * is covered the moment it compiles, with no edit here. That is the point: a hand-maintained + * second copy of a list always drifts from the thing it mirrors. + * + *

Scope, stated on purpose (fleetd #113 criterion 3 — every check reports what it + * did and did not look at): + *

    + *
  • It checks each key NAME appears somewhere in the example as a YAML key, live or + * commented out. It does NOT check the key sits at the right path.
    • + *
  • It does NOT check a documented key is read by anything. {@code paneProbeIntervalSeconds} + * is parsed into {@link FleetConfig.Health} and used nowhere, and this guard passes it. + * Proving a key is live code needs a call graph, which this is not.
    • + *
+ */ + @Test + void everyNestedConfigKeyIsDocumentedInTheExample() throws Exception { + Path example = Path.of("fleetd.example.yaml"); + assertTrue(Files.exists(example), "fleetd.example.yaml must ship next to the pom"); + String text = Files.readString(example); + + Map pathByName = configKeyPaths(); + + // The denominator. An under-counting walk passes every subset check vacuously, which is + // the exact shape fleetd #113 collects — so the walk has to prove it descended at all. + // The floor is DERIVED, not a literal: the nested walk must find substantially more keys + // than the top-level set the old guard used, or it has not gone below the first level. + int topLevel = FleetConfig.KNOWN_TOP_LEVEL_KEYS.size(); + assertTrue(pathByName.size() > topLevel * 2, + "the record walk found " + pathByName.size() + " config key(s) against " + + topLevel + " top-level key(s) — it has stopped descending into the " + + "nested records, so this guard would pass vacuously. Fix the walk " + + "before trusting a green run."); + + List undocumented = pathByName.entrySet().stream() + .filter(e -> !keyDocumentedAnywhere(text, e.getKey())) + .map(Map.Entry::getValue) + .sorted() + .toList(); + + assertTrue(undocumented.isEmpty(), () -> "checked " + pathByName.size() + + " config key(s) that FleetConfig can bind; " + undocumented.size() + + " appear nowhere in fleetd.example.yaml: " + undocumented + + " — document each one there, commented out if optional. fleetd.yaml is " + + "gitignored, so the example is the only committed description of the schema."); + } + + /** + * The other direction: a LIVE key in the example that {@link FleetConfig} cannot bind. That is + * a key an operator would copy into {@code fleetd.yaml} expecting it to do something, where it + * would be silently ignored. + * + *

Scope, stated on purpose: only live (uncommented) keys are checked. Most of the + * example is commented-out prose, and that prose contains lines like {@code # mode: token} + * that are indistinguishable from keys by text alone. Parsing them would produce false + * failures, so they are deliberately out of scope — and saying so here is the point, rather + * than letting a reader assume the whole file was validated. + */ + @Test + void everyLiveKeyInTheExampleBindsToARecordComponent() throws Exception { + Path example = Path.of("fleetd.example.yaml"); + String text = Files.readString(example); + + List> paths = liveKeyPaths(text); + assertTrue(paths.size() >= 20, + "only " + paths.size() + " live key path(s) were parsed out of the example — the " + + "parser is not seeing the file, so this guard would pass vacuously."); + + List unbindable = paths.stream() + .filter(path -> !pathBinds(path)) + .map(path -> String.join(".", path)) + .distinct() + .sorted() + .toList(); + + assertTrue(unbindable.isEmpty(), () -> "checked " + paths.size() + + " live key path(s) in fleetd.example.yaml; " + unbindable.size() + + " bind to nothing in FleetConfig: " + unbindable + + " — an operator copying one of these into fleetd.yaml gets silence, not an error."); + } + + /** + * Every configuration key {@link FleetConfig} can bind, at every depth, as + * {@code name -> a dotted path to one place it appears}. Derived from the record components, + * so it cannot drift from the code. + */ + private static Map configKeyPaths() { + Map out = new TreeMap<>(); + collectConfigKeys(FleetConfig.class, "", new HashSet<>(), out); + return out; + } + + private static void collectConfigKeys(Class type, String prefix, Set seen, + Map out) { + if (!type.isRecord() || !seen.add(type.getName())) { + return; + } + for (RecordComponent rc : type.getRecordComponents()) { + String path = prefix.isEmpty() ? rc.getName() : prefix + "." + rc.getName(); + out.putIfAbsent(rc.getName(), path); + Class nested = rc.getType(); + if (nested.isRecord()) { + collectConfigKeys(nested, path, seen, out); + } else if (Map.class.isAssignableFrom(nested) || List.class.isAssignableFrom(nested)) { + Class element = elementRecord(rc); + if (element != null) { + String childPrefix = Map.class.isAssignableFrom(nested) + ? path + "." : path + "[]"; + collectConfigKeys(element, childPrefix, seen, out); + } + } + } + } + + /** The record type inside a {@code Map} or {@code List} component, else null. */ + private static Class elementRecord(RecordComponent rc) { + if (rc.getGenericType() instanceof ParameterizedType pt) { + Type[] args = pt.getActualTypeArguments(); + if (args.length > 0 && args[args.length - 1] instanceof Class c && c.isRecord()) { + return c; + } + } + return null; + } + + /** + * True when {@code key} is documented in the example, in either of the two conventions that + * file actually uses: + *

    + *
  1. as a YAML key at any indentation, live or commented out ({@code key:}); or
    2. + *
  2. in a prose block that describes a section's sub-keys, one per line, as + * {@code # key → what it does}.
    3. + *
+ * + *

The second form is not decoration. {@code broker.uri} is documented ONLY that way, on + * purpose: writing it out as a copy-pasteable {@code uri: amqp://user:pass@host} invites an + * operator to paste a password into a file, which is the very thing {@code uriEnv} exists to + * avoid. A guard that demanded the key form would push the file toward doing that. So this + * encodes the convention the example really uses rather than imposing a new one. + */ + private static boolean keyDocumentedAnywhere(String yaml, String key) { + String quoted = Pattern.quote(key); + Pattern asYamlKey = Pattern.compile("(?m)^\\s*(?:#\\s*)?" + quoted + ":"); + Pattern asProseEntry = Pattern.compile("(?m)^\\s*#\\s*" + quoted + "\\s+\u2192"); + return asYamlKey.matcher(yaml).find() || asProseEntry.matcher(yaml).find(); + } + + /** Every live (uncommented) key in {@code yaml}, as a path from the document root. */ + private static List> liveKeyPaths(String yaml) { + Pattern keyLine = Pattern.compile("^(\\s*)([A-Za-z][A-Za-z0-9_]*):(\\s.*)?$"); + List stack = new ArrayList<>(); + List indents = new ArrayList<>(); + List> paths = new ArrayList<>(); + for (String line : yaml.split("\n", -1)) { + if (line.isBlank() || line.stripLeading().startsWith("#")) { + continue; + } + Matcher m = keyLine.matcher(line); + if (!m.matches()) { + continue; + } + int indent = m.group(1).length(); + while (!indents.isEmpty() && indents.get(indents.size() - 1) >= indent) { + indents.remove(indents.size() - 1); + stack.remove(stack.size() - 1); + } + indents.add(indent); + stack.add(m.group(2)); + paths.add(List.copyOf(stack)); + } + return paths; + } + + /** True when a dotted YAML path resolves to something {@link FleetConfig} can bind. */ + private static boolean pathBinds(List path) { + Class type = FleetConfig.class; + boolean nextSegmentIsAFreeFormName = false; + for (int i = 0; i < path.size(); i++) { + if (nextSegmentIsAFreeFormName) { + nextSegmentIsAFreeFormName = false; + continue; + } + RecordComponent rc = componentNamed(type, path.get(i)); + if (rc == null) { + return false; + } + Class t = rc.getType(); + if (t.isRecord()) { + type = t; + } else if (Map.class.isAssignableFrom(t)) { + Class element = elementRecord(rc); + if (element == null) { + return true; // Map: its entries are data, not schema + } + type = element; + nextSegmentIsAFreeFormName = true; + } else { + // A scalar or a list of scalars: nothing may legitimately nest under it. + return i == path.size() - 1; + } + } + return true; + } + + private static RecordComponent componentNamed(Class type, String name) { + if (type == null || !type.isRecord()) { + return null; + } + for (RecordComponent rc : type.getRecordComponents()) { + if (rc.getName().equals(name)) { + return rc; + } + } + return null; + } + @Test void placementDefaultsToFixedForExistingConfigs(@TempDir Path dir) throws Exception { Path f = dir.resolve("no-placement.yaml");