diff --git a/fleetd/fleetd.example.yaml b/fleetd/fleetd.example.yaml index dbf3383..9829550 100644 --- a/fleetd/fleetd.example.yaml +++ b/fleetd/fleetd.example.yaml @@ -307,16 +307,22 @@ profiles: # 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 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. + # GOTCHA 3 (fleetd #176, corrected by fleetd #257) — `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 ANY OTHER + # `subscription: true` profile that shares this one's account (see THE SENTINEL, just below, + # next to `credentialId:`) — `fleet_list` reports that seat count under `leadSeats`; see + # `profile:` under THE FLEET below. `free` itself is NEVER reduced by `leadSeats`: `free` means + # "what the real placement gate (`CompositePeerLauncher#enforceMaxLoad`) will actually grant a + # fresh `fleet_spawn` right now", and that gate only ever compares live members against + # `maxLoad` — it has no notion of the lead's own seat. An earlier cut of this feature + # subtracted `leadSeats` from `free` on the theory it made `free` describe the true ceiling on + # the account, but no backend seat ceiling shared with the lead has ever actually been + # measured, and the subtraction just made `free` disagree with the one thing it is supposed to + # describe — the fleetd #257 fix. `maxLoad: 3` means 3 member slots, full stop; a lead sharing + # the account is a fact you can see in `leadSeats`, not a reason `free` undercounts spawns that + # will, in practice, succeed. # # 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 @@ -536,17 +542,18 @@ 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 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 - # daemon's side — so that lead's seat goes uncounted, exactly as before this ticket. + # entry here names a profile that shares a worker profile's account, `fleet_list` reports the + # lead's live seat(s) on that worker profile under `leadSeats` — informational only, as of fleetd + # #257 it is NEVER subtracted from `free` (see GOTCHA 3, next to `maxLoad:`, in THE WORKERS above, + # for why). "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 daemon's side — so that lead's seat never appears in `leadSeats`. # # `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 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 332e153..1e1d7a9 100644 --- a/fleetd/src/main/java/dev/ltms/fleet/mcp/FleetMcp.java +++ b/fleetd/src/main/java/dev/ltms/fleet/mcp/FleetMcp.java @@ -139,16 +139,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. + * subscription — a fact {@code fleet_list} reports alongside {@code free} via the + * {@code leadSeats} key. * *

{@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. + * — see {@code LeadLauncher}). 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. + * + *

fleetd #257: this count is reported, never subtracted from {@code free}. An earlier cut of + * this feature subtracted it, on the theory that it made {@code free} describe the real ceiling + * on the account — but the real spawn gate ({@code CompositePeerLauncher#enforceMaxLoad}) never + * read this count at all, so the subtraction made {@code free} disagree with the one thing it is + * supposed to describe: what a fresh {@code fleet_spawn} will actually get. No backend seat + * ceiling shared with the lead has been measured either — see {@code fleetd.example.yaml}'s + * {@code maxLoad} docs. {@code free} now always equals {@code max(0, maxLoad - live)}, and + * {@code leadSeats} is reported purely as a fact the caller may act on however it likes. */ public record LeadSeatSource(Function seatsFor) { /** Inert source — no profile is ever reported as sharing a seat with a lead. */ @@ -1137,10 +1145,17 @@ public final class FleetMcp { *

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. + * credential) via the {@code leadSeats} key, added only when the count is positive, for the + * same byte-identical-when-unused reason as the quarantine/cool-off keys above. + * + *

fleetd #257: {@code leadSeatCount} is reported, never subtracted from {@code free}. + * {@code free} means "what a fresh {@code fleet_spawn} on this profile will actually get", and + * the real gate ({@code CompositePeerLauncher#enforceMaxLoad}) only ever compares {@code live} + * against {@code maxLoad} — it has no notion of a lead's own seat. Subtracting + * {@code leadSeatCount} here made {@code free} disagree with the gate it is supposed to + * describe: it could report {@code free: 0} while a spawn on that exact profile still + * succeeded. {@code leadSeats} stays in the row as a fact the caller can act on however it + * likes, but it no longer changes what {@code free} means. */ private static Map capacityView(String profile, Function liveCount, Function maxLoad, List roster, @@ -1155,7 +1170,12 @@ public final class FleetMcp { .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 - leadSeatCount)); + // fleetd #257: free must report what the real spawn gate (CompositePeerLauncher#enforceMaxLoad) + // will actually grant, and that gate never reads leadSeatCount — only maxLoad and live. Not + // subtracting the lead's seat here used to make free UNDERSTATE what a fresh fleet_spawn would + // get, so a lead believing free:0 gave up on a profile the gate would still spawn onto. + // leadSeatCount is still reported via the leadSeats key below, just never subtracted from free. + row.put("free", cap == null ? null : Math.max(0, cap - live)); row.put("reclaimable", reclaimable); if (leadSeatCount > 0) { row.put("leadSeats", leadSeatCount); @@ -1360,8 +1380,13 @@ public final class FleetMcp { + "member spawned without one shares its directory with others and never reports " + "an id, however long it runs (fleetd #249). An empty 'members' " + "means no members are spawned; it says nothing about peers. When capacity " - + "facts are configured, a 'capacity' row per profile also reports free: 0 for " - + "a quarantined profile's credential (see fleet_profiles), whatever its " + + "facts are configured, a 'capacity' row per profile reports 'free' — the " + + "slots a fresh fleet_spawn on that profile will actually be granted right " + + "now (max(0, maxLoad - live)), the same check the spawn gate itself runs. A " + + "'leadSeats' key, when present, reports how many of those live slots are a " + + "lead session sharing this profile's subscription — informational only, " + + "already NOT subtracted from 'free' (fleetd #257). It also reports free: 0 " + + "for a quarantined profile's credential (see fleet_profiles), whatever its " + "maxLoad/live — with credentialId and quarantinedForSeconds naming the " + "quarantine, so 'free: 0, busy' can be told apart from 'free: 0, refusing " + "for N seconds'.", 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 b6237b1..6100420 100644 --- a/fleetd/src/test/java/dev/ltms/fleet/mcp/FleetMcpTest.java +++ b/fleetd/src/test/java/dev/ltms/fleet/mcp/FleetMcpTest.java @@ -34,6 +34,7 @@ import java.util.Map; import java.util.Set; import java.util.concurrent.CompletableFuture; import java.util.concurrent.TimeUnit; +import java.util.function.Function; import static org.junit.jupiter.api.Assertions.*; @@ -810,13 +811,16 @@ class FleetMcpTest { } /** - * 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). + * fleetd #257: this is the exact shape measured on the Mac fleet — {@code maxLoad:3, live:2}, + * one lead sharing the subscription. The OLD formula ({@code max(0, cap - live - leadSeats)}) + * reported {@code free:0} here, disagreeing with the real spawn gate (which never read + * {@code leadSeats} and would still grant one more spawn — see + * {@code freeMatchesWhatTheRealPlacementGateActuallyGrants} below for that proof against the + * actual gate). {@code free} must report {@code 1}: {@code leadSeats} is carried as a fact, not + * subtracted. */ @Test - void leadSeatSubtractsFromFreeTheSameWayLiveDoes() { + void leadSeatIsReportedButNeverSubtractedFromFree() { FakeHerdr h = new FakeHerdr(); SessionManager sessions = new SessionManager(workerService(h, "http://gx00.gw:8000", Set.of("gx00.gw"))); FleetMcp.LeadSeatSource leadSeats = new FleetMcp.LeadSeatSource( @@ -829,17 +833,17 @@ class FleetMcpTest { 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); + assertTrue(out.contains("\"free\":1"), "free is maxLoad - live only, never minus leadSeats: " + out); + assertTrue(out.contains("\"leadSeats\":1"), "leadSeats is still reported, just not subtracted: " + 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}. + * fleetd #257: the OTHER measurement in the issue — a completely idle fleet with a lead sharing + * the subscription. {@code maxLoad:3, live:0} must report {@code free:3}, matching what the + * spawn gate (which has no notion of a lead's seat) would actually grant. */ @Test - void leadSeatLowersFreeOnAnOtherwiseIdleSubscriptionProfile() { + void leadSeatDoesNotLowerFreeOnAnOtherwiseIdleSubscriptionProfile() { FakeHerdr h = new FakeHerdr(); SessionManager sessions = new SessionManager(workerService(h, "http://gx00.gw:8000", Set.of("gx00.gw"))); FleetMcp.LeadSeatSource leadSeats = new FleetMcp.LeadSeatSource( @@ -851,10 +855,76 @@ class FleetMcpTest { 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("\"free\":3"), "the real gate never subtracts the lead's seat: " + out); assertTrue(out.contains("\"leadSeats\":1"), out); } + /** + * fleetd #257 — the defect, driven against the REAL placement gate, not a copy of its + * arithmetic. {@code free} must equal exactly how many more spawns + * {@link CompositePeerLauncher#spawn} (routed through the same {@link SessionManager} fleet_spawn + * itself uses) will grant on this profile right now: this test reads whatever number + * {@code fleet_list}'s real {@code listFleet} call reports, then drives that many spawns through + * the REAL composite launcher and asserts every one succeeds, and the next one — one past what + * fleet_list promised — is refused. A test that instead hand-computed {@code cap - live} and + * compared it to {@code free} would pass even if both sides shared the same wrong formula (this + * repo has been bitten by exactly that before); this one only passes when fleet_list's number and + * the gate's real behaviour actually agree. + * + *

{@code liveCount} here is wired the same way {@code Fleetd.main} wires it in production: one + * function, read by both the {@link CompositePeerLauncher}'s {@code maxLoad} gate and + * {@code fleet_list}'s {@code CapacitySource}, off the SAME {@link SessionManager#roster()} — so + * the two paths cannot silently drift on what "live" means. + */ + @Test + void freeMatchesWhatTheRealPlacementGateActuallyGrants() { + FakeHerdr h = new FakeHerdr(); + FleetConfig.Profile wcfg = new FleetConfig.Profile( + "sonnet", "http://gx00.gw:8000", "coder", null, "FLEETD_WORKER_TOKEN", null, + "tab", "fleetd-workers", "worker: {profile} #{n}", null, + null, null, null, null, null, null, null, 3); + Map profiles = Map.of(wcfg.profile(), wcfg); + ClaudeCodeLauncher delegate = new ClaudeCodeLauncher( + new AgentControl(h), new WorkspaceControl(h), new SubscriptionGuard(Set.of("gx00.gw")), + profiles, wcfg.profile(), k -> "FLEETD_WORKER_TOKEN".equals(k) ? "tok" : null); + + java.util.concurrent.atomic.AtomicReference smRef = + new java.util.concurrent.atomic.AtomicReference<>(); + Function liveCount = profile -> (int) smRef.get().roster().stream() + .filter(s -> profile.equals(s.profile())).count(); + CompositePeerLauncher composite = new CompositePeerLauncher( + List.of(delegate), wcfg.profile(), profiles, PlacementPolicies.fixed(), liveCount); + SessionManager sm = new SessionManager(composite); + smRef.set(sm); + + // Two members already live — the exact shape measured in fleetd #257 (maxLoad:3, live:2). + assertFalse(FleetMcp.spawn(sm, "sonnet").isError(), "setup: first live member must spawn cleanly"); + assertFalse(FleetMcp.spawn(sm, "sonnet").isError(), "setup: second live member must spawn cleanly"); + + // A lead session shares this profile's subscription — leadSeats:1, same as the ticket. + FleetMcp.LeadSeatSource leadSeats = new FleetMcp.LeadSeatSource(p -> "sonnet".equals(p) ? 1 : 0); + String out = textOf(FleetMcp.listFleet(composite, sm, null, + new FleetMcp.CapacitySource(liveCount, p -> profiles.get(p).maxLoad(), profiles::keySet, () -> 0), + new FleetMcp.HealthCoverageSource(() -> "off"), FleetMcp.QuarantineSource.none(), + FleetMcp.OutageSource.none(), leadSeats, Map.of(), "", null)); + int reportedFree = extractInt(out, "free"); + + for (int i = 0; i < reportedFree; i++) { + McpSchema.CallToolResult res = FleetMcp.spawn(sm, "sonnet"); + assertFalse(res.isError(), "fleet_list promised free:" + reportedFree + "; spawn #" + (i + 1) + + " of that many was refused by the real gate: " + textOf(res)); + } + McpSchema.CallToolResult overflow = FleetMcp.spawn(sm, "sonnet"); + assertTrue(overflow.isError(), "fleet_list reported free:" + reportedFree + + " but the real placement gate granted at least one more spawn than that: " + textOf(overflow)); + } + + private static int extractInt(String json, String key) { + java.util.regex.Matcher m = java.util.regex.Pattern.compile("\"" + key + "\":(-?\\d+)").matcher(json); + assertTrue(m.find(), "no \"" + key + "\" field in: " + json); + return Integer.parseInt(m.group(1)); + } + /** A profile with no lead seats reported must be byte-identical to before this ticket. */ @Test void zeroLeadSeatsOmitsTheKeyAndLeavesFreeUnchanged() {