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() {