From c50f5b2d61c77adff02bbb6194e4bd35360c2548 Mon Sep 17 00:00:00 2001 From: Dai Ha Date: Thu, 3 Sep 2026 13:10:10 +0700 Subject: [PATCH] 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() {