Compare commits

..

1 Commits

Author SHA1 Message Date
Dai Ha 5a3ab5764c fleetd #247: CAS seedTrustDialog against the operator's own live Claude Code
CI / contract (pull_request) Successful in 54s
CI / build (pull_request) Successful in 1m49s
TRUST_JSON_LOCK only serialises seedTrustDialog calls this launcher makes
inside its own JVM. It cannot reach the one writer that actually shares the
target file on a real host: the operator's own live Claude Code, whose
CLAUDE_CONFIG_DIR is routinely the very configDir a profile is given, so the
file fleetd writes on every claude-code spawn is that session's own config.
A plain read-modify-write there is a routine lost update: fleetd reads v1,
the operator's session writes v2, fleetd's ATOMIC_MOVE lands v3 built from
v1 and v2 is gone, atomically.

Add a bounded compare-and-swap: before the move, re-read the target's exact
bytes and compare with what the update was built from; on a mismatch,
rebuild from the fresh bytes and retry (up to 5 attempts). Exhausting the
retries writes nothing and logs a WARN naming the file — the member shows
the trust dialog and fails to reach an injectable state instead, which is
visible and recoverable, unlike silently overwriting the operator's live
config. Also warn every time the seed is about to target the default
~/.claude.json (configDir unset), since that is the unsafe default.

This narrows the lost-update window, it does not close it: a write landing
between the final re-read and the ATOMIC_MOVE itself is still lost.
2026-09-03 16:20:20 +07:00
7 changed files with 313 additions and 231 deletions
+21 -28
View File
@@ -307,22 +307,16 @@ 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, 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.<name>.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.
# 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.<name>.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
@@ -542,18 +536,17 @@ 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` 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`.
# 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.
#
# `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
@@ -139,24 +139,16 @@ public final class FleetMcp {
/**
* fleetd #176: the seats a profile's own live LEAD session(s) hold on the same Claude
* subscription — a fact {@code fleet_list} reports alongside {@code free} via the
* {@code leadSeats} key.
* subscription — the third reason (alongside {@link QuarantineSource} and {@link OutageSource})
* {@code free} can overstate what a fresh {@code fleet_spawn} would actually get.
*
* <p>{@code maxLoad} counts only <em>members</em>, 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}). See {@code Fleetd.leadSeatLookup} for how the count is derived —
* from {@code fleet.leaders.<name>.profile} and each profile's {@code effectiveCredentialId()},
* never a hardcoded constant.
*
* <p>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.
* — 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.<name>
* .profile} and each profile's {@code effectiveCredentialId()}, never a hardcoded constant.
*/
public record LeadSeatSource(Function<String, Integer> seatsFor) {
/** Inert source — no profile is ever reported as sharing a seat with a lead. */
@@ -1145,17 +1137,10 @@ public final class FleetMcp {
* <p>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) 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.
*
* <p>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.
* 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<String, Object> capacityView(String profile, Function<String, Integer> liveCount,
Function<String, Integer> maxLoad, List<MemberSession> roster,
@@ -1170,12 +1155,7 @@ public final class FleetMcp {
.count();
Map<String, Object> row = new LinkedHashMap<>();
row.put("profile", profile); row.put("maxLoad", cap); row.put("live", live);
// 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("free", cap == null ? null : Math.max(0, cap - live - leadSeatCount));
row.put("reclaimable", reclaimable);
if (leadSeatCount > 0) {
row.put("leadSeats", leadSeatCount);
@@ -1380,13 +1360,8 @@ 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 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 "
+ "facts are configured, a 'capacity' row per profile 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'.",
@@ -19,6 +19,7 @@ import java.nio.file.Files;
import java.nio.file.Path;
import java.nio.file.StandardCopyOption;
import java.nio.file.attribute.PosixFileAttributeView;
import java.util.Arrays;
import java.util.EnumSet;
import java.util.List;
import java.util.Map;
@@ -519,6 +520,26 @@ public final class ClaudeCodeLauncher extends HerdrPeerLauncher {
* the first. Both exist because of a real incident: see {@link HerdrPeerLauncher#isProvisionedWorktree}'s javadoc
* and {@link #writeAtomically}'s javadoc.
*
* <p><b>Compare-and-swap against a writer the lock cannot reach (fleetd #247).</b>
* {@code TRUST_JSON_LOCK} only serialises calls this launcher itself makes inside this one JVM.
* It does nothing about a writer outside it — and on a host where the profile's
* {@code configDir} is the operator's own {@code CLAUDE_CONFIG_DIR}, the file this method writes
* IS the operator's own live Claude Code session's config file, being read and written by that
* session while it runs. Measured 2026-09-03: its mtime moved minutes after a spawn while that
* session was active. A plain read-modify-write there is a routine lost update, not a rare one:
* fleetd reads v1, the operator's session reads v1 and writes v2 with their own change, fleetd's
* {@code ATOMIC_MOVE} then lands v3 built from v1 — atomic, but v2's change is gone. So before
* the move this method re-reads {@code target}'s exact bytes and compares them with the bytes it
* built its update from; a mismatch means someone else wrote in between, and it discards its
* work and rebuilds from the fresh bytes, up to {@link #MAX_TRUST_JSON_CAS_ATTEMPTS} times.
* <b>Exhausting the retries writes nothing</b> — see the WARN at the end of the loop for why
* that, not a last write-anyway, is the safe failure: the member shows the trust dialog and
* fails to reach an injectable state, which is visible, logged and recoverable; overwriting the
* operator's live config with a stale copy is neither. This narrows the lost-update window, it
* does not close it — a write landing between the final re-read and the {@code ATOMIC_MOVE}
* itself is still lost, because there is no OS-level compare-and-swap on a plain file, only this
* cooperative narrowing of the gap.
*
* @param configDir the profile's {@code CLAUDE_CONFIG_DIR} ({@code cfg.configDir()}), or
* {@code null}/blank to target the default {@code ~/.claude.json}
* @param cwd the spawn's resolved working directory — the exact key Claude Code will look
@@ -528,55 +549,118 @@ public final class ClaudeCodeLauncher extends HerdrPeerLauncher {
if (!isProvisionedWorktree(cwd)) {
return;
}
Path target = (configDir == null || configDir.isBlank())
boolean unsetConfigDir = configDir == null || configDir.isBlank();
Path target = unsetConfigDir
? Path.of(System.getProperty("user.home"), ".claude.json")
: Path.of(configDir, ".claude.json");
if (unsetConfigDir) {
// fleetd #247: configDir unset is the ONLY path that targets ~/.claude.json — the
// operator's own home file, not a per-profile one — and it is the default, so a
// profile that simply forgot to set configDir gets no signal at all short of the
// operator noticing their own file changing. Say so loudly, every time it is about to
// happen, rather than only once ever: each occurrence is a live write to a real
// person's home config and deserves its own log line.
log.warn("seedTrustDialog: profile has no configDir set, so the workspace-trust seed "
+ "for cwd '{}' is about to write the operator's own default '{}' — set "
+ "configDir on this profile to target a per-member config file instead",
cwd, target);
}
synchronized (TRUST_JSON_LOCK) {
try {
if (target.getParent() != null) {
Files.createDirectories(target.getParent());
}
ObjectNode root = null;
if (Files.isRegularFile(target)) {
JsonNode existing = TRUST_JSON.readTree(target.toFile());
if (existing instanceof ObjectNode existingObject) {
root = existingObject;
for (int attempt = 1; attempt <= MAX_TRUST_JSON_CAS_ATTEMPTS; attempt++) {
byte[] before = Files.isRegularFile(target) ? Files.readAllBytes(target) : null;
ObjectNode root = parseTrustJsonOrEmpty(before);
JsonNode projectsNode = root.get("projects");
ObjectNode projects = projectsNode instanceof ObjectNode projectsObject
? projectsObject : TRUST_JSON.createObjectNode();
if (!(projectsNode instanceof ObjectNode)) {
root.set("projects", projects);
}
JsonNode projectNode = projects.get(cwd);
ObjectNode project = projectNode instanceof ObjectNode projectObject
? projectObject : TRUST_JSON.createObjectNode();
if (!(projectNode instanceof ObjectNode)) {
projects.set(cwd, project);
}
// fleetd #247: ONLY hasTrustDialogAccepted. We used to write
// hasCompletedProjectOnboarding beside it; do not put it back. Measured on
// 2026-09-03, minutes after a live spawn seeded this file: 28 of 28 project
// entries carried hasTrustDialogAccepted and 0 of 28 carried the onboarding key
// — including the 27 entries Claude Code wrote for itself. Claude Code
// normalises the whole file when it saves and drops that key every time, so
// writing it achieved nothing except making the next reader think it mattered.
// The member reached idle with the trust flag alone, which is the only outcome
// this seed exists for. If a future Claude Code needs the second flag the
// symptom returns as the trust dialog fleetd #149 describes — re-measure then,
// do not restore it on a guess.
project.put("hasTrustDialogAccepted", true);
String newContent = TRUST_JSON.writerWithDefaultPrettyPrinter().writeValueAsString(root);
trustJsonCasTestHook.run();
// fleetd #247 CAS: re-read immediately before the move and compare with what
// this attempt built its update from. A mismatch means another writer (most
// plausibly the operator's own live Claude Code — see this method's javadoc)
// landed a change in between; discard this attempt's work and rebuild from the
// fresh bytes rather than blindly overwriting it.
byte[] atMove = Files.isRegularFile(target) ? Files.readAllBytes(target) : null;
if (!Arrays.equals(before, atMove)) {
continue;
}
writeAtomically(target, newContent);
return;
}
if (root == null) {
root = TRUST_JSON.createObjectNode();
}
JsonNode projectsNode = root.get("projects");
ObjectNode projects = projectsNode instanceof ObjectNode projectsObject
? projectsObject : TRUST_JSON.createObjectNode();
if (!(projectsNode instanceof ObjectNode)) {
root.set("projects", projects);
}
JsonNode projectNode = projects.get(cwd);
ObjectNode project = projectNode instanceof ObjectNode projectObject
? projectObject : TRUST_JSON.createObjectNode();
if (!(projectNode instanceof ObjectNode)) {
projects.set(cwd, project);
}
// fleetd #247: ONLY hasTrustDialogAccepted. We used to write
// hasCompletedProjectOnboarding beside it; do not put it back. Measured on
// 2026-09-03, minutes after a live spawn seeded this file: 28 of 28 project
// entries carried hasTrustDialogAccepted and 0 of 28 carried the onboarding key
// — including the 27 entries Claude Code wrote for itself. Claude Code
// normalises the whole file when it saves and drops that key every time, so
// writing it achieved nothing except making the next reader think it mattered.
// The member reached idle with the trust flag alone, which is the only outcome
// this seed exists for. If a future Claude Code needs the second flag the
// symptom returns as the trust dialog fleetd #149 describes — re-measure then,
// do not restore it on a guess.
project.put("hasTrustDialogAccepted", true);
writeAtomically(target, TRUST_JSON.writerWithDefaultPrettyPrinter().writeValueAsString(root));
// fleetd #247: deliberately do NOT write here. A member that starts without the
// seed still starts — it may hit the trust dialog fleetd #149 describes and fail to
// reach an injectable state, but that failure is visible (herdr reports it, the
// spawn-readiness gate times out) and recoverable (retry the spawn). Writing our
// stale copy over whatever the other writer left would be silent and, if that other
// writer is the operator's own live session, could destroy real configuration —
// fail toward the recoverable outcome, not the silent one.
log.warn("seedTrustDialog: gave up seeding workspace-trust for cwd '{}' into '{}' "
+ "after {} attempts — another writer (most plausibly the operator's own "
+ "live Claude Code sharing this file) kept changing it faster than we could "
+ "re-read it, so nothing was written; the member may show the trust dialog "
+ "instead", cwd, target, MAX_TRUST_JSON_CAS_ATTEMPTS);
} catch (Exception e) {
log.debug("cannot seed workspace-trust entry for cwd '{}' into '{}'", cwd, target, e);
}
}
}
/** Bound on {@link #seedTrustDialog}'s fleetd #247 compare-and-swap retry loop. */
private static final int MAX_TRUST_JSON_CAS_ATTEMPTS = 5;
/**
* Test-only seam for fleetd #247: invoked once per CAS attempt inside {@link #seedTrustDialog}'s
* retry loop, after that attempt has read the target's bytes and built its replacement content,
* but immediately before the final re-read/compare that decides whether to write. A no-op in
* production. Package-visible (not {@code private}) so {@code ClaudeCodeLauncherTest} can install
* a hook here that writes to the target file, deterministically simulating a writer racing
* fleetd's own read-modify-write at the exact instant the CAS is meant to catch — the same race
* an external process (most plausibly the operator's own live Claude Code) creates, without
* depending on real thread scheduling to land the interleaving. A test that sets this MUST
* restore it to the no-op default in a {@code finally} block — it is shared, static state.
*/
static Runnable trustJsonCasTestHook = () -> {};
/**
* Parse {@code bytes} as a {@code .claude.json} tree, or hand back a fresh empty object when
* {@code bytes} is {@code null} (no file yet) or does not parse to a JSON object — the same
* missing-or-unreadable-is-empty fallback {@link #seedTrustDialog} always used, factored out so
* the fleetd #247 CAS loop can call it once per attempt.
*/
private static ObjectNode parseTrustJsonOrEmpty(byte[] bytes) throws IOException {
if (bytes == null) {
return TRUST_JSON.createObjectNode();
}
JsonNode existing = TRUST_JSON.readTree(bytes);
return existing instanceof ObjectNode existingObject ? existingObject : TRUST_JSON.createObjectNode();
}
/**
* Write {@code content} to {@code target} atomically: serialise to a sibling temp file in the
* <strong>same directory</strong> as {@code target} (an atomic move is only guaranteed within
@@ -34,7 +34,6 @@ 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.*;
@@ -811,16 +810,13 @@ class FleetMcpTest {
}
/**
* 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.
* 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 leadSeatIsReportedButNeverSubtractedFromFree() {
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(
@@ -833,17 +829,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\":1"), "free is maxLoad - live only, never minus leadSeats: " + out);
assertTrue(out.contains("\"leadSeats\":1"), "leadSeats is still reported, just not subtracted: " + out);
assertTrue(out.contains("\"free\":0"), "2 live + 1 lead seat fills all 3: " + out);
assertTrue(out.contains("\"leadSeats\":1"), out);
}
/**
* 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.
* 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 leadSeatDoesNotLowerFreeOnAnOtherwiseIdleSubscriptionProfile() {
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(
@@ -855,76 +851,10 @@ class FleetMcpTest {
FleetMcp.QuarantineSource.none(), FleetMcp.OutageSource.none(), leadSeats, Map.of(), "", null));
assertTrue(out.contains("\"live\":0"), out);
assertTrue(out.contains("\"free\":3"), "the real gate never subtracts the lead's seat: " + 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);
}
/**
* 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.
*
* <p>{@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<String, FleetConfig.Profile> 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<SessionManager> smRef =
new java.util.concurrent.atomic.AtomicReference<>();
Function<String, Integer> 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() {
@@ -26,6 +26,7 @@ import org.junit.jupiter.api.io.TempDir;
import org.slf4j.LoggerFactory;
import java.io.IOException;
import java.io.UncheckedIOException;
import java.nio.file.Files;
import java.nio.file.Path;
import java.nio.file.attribute.PosixFileAttributeView;
@@ -39,6 +40,7 @@ import java.util.UUID;
import java.util.concurrent.CountDownLatch;
import java.util.concurrent.TimeUnit;
import java.util.concurrent.atomic.AtomicBoolean;
import java.util.concurrent.atomic.AtomicInteger;
import java.util.concurrent.atomic.AtomicReference;
import java.util.function.Function;
import java.util.function.Supplier;
@@ -2594,4 +2596,150 @@ class ClaudeCodeLauncherTest {
+ tornRead.get());
assertEquals(newContent, Files.readString(target), "the final content must be the new content");
}
// --- fleetd #247: CAS against a writer TRUST_JSON_LOCK cannot reach --------------------------
//
// fleetd #149's lock only serialises seedTrustDialog calls THIS launcher makes inside this one
// JVM. It does nothing about the one writer that actually shares this file on a real host: the
// operator's own live Claude Code, whose CLAUDE_CONFIG_DIR is routinely the very configDir this
// profile is given. A plain read-modify-write there is a routine lost update — fleetd reads v1,
// the operator's session writes v2, fleetd's ATOMIC_MOVE lands v3 built from v1, and v2 is gone,
// atomically. The fix re-reads the file's exact bytes immediately before the move and compares
// them with what the update was built from, retrying from fresh bytes on a mismatch.
//
// Both tests below drive the race through the real seedTrustDialog/spawn() path (not a
// hand-rolled call to some extracted primitive) using trustJsonCasTestHook — a seam fired once
// per CAS attempt, at the exact point between the read and the final compare, so the race is
// deterministic instead of depending on real thread timing.
@Test
void seedTrustDialogRetriesAndPreservesAConcurrentExternalWritersChange(
@TempDir Path configDir, @TempDir Path worktree) throws Exception {
markAsProvisionedWorktree(worktree);
Path claudeJson = configDir.resolve(".claude.json");
Files.writeString(claudeJson, "{\"projects\":{}}");
// Fires exactly once, on the first CAS attempt — simulating the operator's own live Claude
// Code landing its own write to this SAME file in the gap between fleetd's read and write.
AtomicBoolean fired = new AtomicBoolean(false);
ClaudeCodeLauncher.trustJsonCasTestHook = () -> {
if (fired.compareAndSet(false, true)) {
try {
Files.writeString(claudeJson,
"{\"projects\":{\"/operator/own/project\":"
+ "{\"hasTrustDialogAccepted\":true}}}");
} catch (IOException e) {
throw new UncheckedIOException(e);
}
}
};
try {
FakeHerdr herdr = new FakeHerdr();
FleetConfig.Profile cfg = trustProfile(configDir.toString(), worktree.toString());
new ClaudeCodeLauncher(new AgentControl(herdr), new WorkspaceControl(herdr),
new SubscriptionGuard(Set.of("gx00.gw")), Map.of(cfg.profile(), cfg), cfg.profile(), _ -> null)
.spawn();
} finally {
ClaudeCodeLauncher.trustJsonCasTestHook = () -> {};
}
assertTrue(fired.get(), "the race hook must actually have fired during the spawn");
JsonNode root = new ObjectMapper().readTree(claudeJson.toFile());
assertTrue(root.path("projects").path("/operator/own/project")
.path("hasTrustDialogAccepted").asBoolean(false),
"the external writer's change, landed between fleetd's read and write, must SURVIVE "
+ "— this is the whole point of the CAS: without it, fleetd's stale-built "
+ "ATOMIC_MOVE would have silently discarded it. Final file: "
+ Files.readString(claudeJson));
assertTrue(root.path("projects").path(worktree.toString())
.path("hasTrustDialogAccepted").asBoolean(false),
"fleetd's own retry must still land its own trust entry, built from the fresh bytes");
}
/**
* Retry-exhaustion path: an external writer that changes the file on EVERY attempt (not just
* once) exhausts all {@code MAX_TRUST_JSON_CAS_ATTEMPTS} retries. fleetd must then write nothing
* at all — not even a partial/best-effort write — and log a WARN naming the file it gave up on.
*/
@Test
void seedTrustDialogWritesNothingAndWarnsWhenCasRetriesAreExhausted(
@TempDir Path configDir, @TempDir Path worktree) throws Exception {
markAsProvisionedWorktree(worktree);
Path claudeJson = configDir.resolve(".claude.json");
Files.writeString(claudeJson, "{\"marker\":\"start\"}");
AtomicInteger hookCalls = new AtomicInteger();
ClaudeCodeLauncher.trustJsonCasTestHook = () -> {
try {
Files.writeString(claudeJson,
"{\"marker\":\"race-" + hookCalls.incrementAndGet() + "\"}");
} catch (IOException e) {
throw new UncheckedIOException(e);
}
};
Logger logger = (Logger) LoggerFactory.getLogger(ClaudeCodeLauncher.class);
ListAppender<ILoggingEvent> appender = new ListAppender<>();
appender.start();
logger.addAppender(appender);
try {
FakeHerdr herdr = new FakeHerdr();
FleetConfig.Profile cfg = trustProfile(configDir.toString(), worktree.toString());
new ClaudeCodeLauncher(new AgentControl(herdr), new WorkspaceControl(herdr),
new SubscriptionGuard(Set.of("gx00.gw")), Map.of(cfg.profile(), cfg), cfg.profile(), _ -> null)
.spawn();
} finally {
logger.detachAppender(appender);
ClaudeCodeLauncher.trustJsonCasTestHook = () -> {};
}
assertEquals(5, hookCalls.get(), "the race hook must fire exactly once per CAS attempt");
String finalContent = Files.readString(claudeJson);
assertEquals("{\"marker\":\"race-" + hookCalls.get() + "\"}", finalContent,
"the file must be left exactly as the external writer last left it — fleetd must not "
+ "have written at all once retries are exhausted");
assertFalse(finalContent.contains("hasTrustDialogAccepted"),
"the trust entry must never appear — its presence would mean the CAS gave up and "
+ "wrote anyway instead of skipping the seed");
assertTrue(appender.list.stream().anyMatch(e -> e.getLevel() == Level.WARN
&& e.getFormattedMessage().contains("gave up seeding workspace-trust")
&& e.getFormattedMessage().contains(worktree.toString())
&& e.getFormattedMessage().contains(claudeJson.toString())),
"a WARN naming both the cwd and the file it gave up on must be logged: "
+ appender.list.stream().map(ILoggingEvent::getFormattedMessage).toList());
}
/**
* The loud-default WARN (fleetd #247): a profile with no {@code configDir} targets the
* operator's real {@code ~/.claude.json} (redirected here to a {@code @TempDir}), and every time
* that happens must be logged, not just detected.
*/
@Test
void seedTrustDialogWarnsEveryTimeItTargetsTheDefaultClaudeJson(
@TempDir Path fakeHome, @TempDir Path worktree) throws Exception {
markAsProvisionedWorktree(worktree);
String originalHome = System.getProperty("user.home");
System.setProperty("user.home", fakeHome.toString());
Logger logger = (Logger) LoggerFactory.getLogger(ClaudeCodeLauncher.class);
ListAppender<ILoggingEvent> appender = new ListAppender<>();
appender.start();
logger.addAppender(appender);
try {
FakeHerdr herdr = new FakeHerdr();
FleetConfig.Profile cfg = trustProfile(null, worktree.toString());
new ClaudeCodeLauncher(new AgentControl(herdr), new WorkspaceControl(herdr),
new SubscriptionGuard(Set.of("gx00.gw")), Map.of(cfg.profile(), cfg), cfg.profile(), _ -> null)
.spawn();
} finally {
logger.detachAppender(appender);
System.setProperty("user.home", originalHome);
}
assertTrue(appender.list.stream().anyMatch(e -> e.getLevel() == Level.WARN
&& e.getFormattedMessage().contains("no configDir set")
&& e.getFormattedMessage().contains(worktree.toString())),
"a WARN naming the cwd must fire when the profile sets no configDir: "
+ appender.list.stream().map(ILoggingEvent::getFormattedMessage).toList());
}
}
@@ -45,9 +45,6 @@ public final class FakeWorktrees implements Worktrees {
private final List<String> overlayShareOrder = new CopyOnWriteArrayList<>();
private final Set<String> existingPaths = ConcurrentHashMap.newKeySet();
private final Set<String> trackedPaths = ConcurrentHashMap.newKeySet();
/** Worktree paths that currently exist, mirroring GitWorktrees' {@code Files.exists} check for
* the already-gone case (CB-576 review, fleetd #116). */
private final Set<String> worktreePaths = ConcurrentHashMap.newKeySet();
private final AtomicLong snapshotSeq = new AtomicLong();
private volatile RuntimeException addFailure;
private volatile RuntimeException snapshotFailure;
@@ -118,15 +115,7 @@ public final class FakeWorktrees implements Worktrees {
}
// The branch already carries a unique nonce, so the derived path is distinct per acquire
// without an extra counter — keep it a pure function of the branch the test can predict.
String path = prefix + "/" + branch.replace('/', '_');
worktreePaths.add(path);
return path;
}
/** Model an operator / {@code git worktree prune} removing the worktree before release. */
public FakeWorktrees markGone(String worktreePath) {
worktreePaths.remove(worktreePath);
return this;
return prefix + "/" + branch.replace('/', '_');
}
@Override
@@ -136,11 +125,6 @@ public final class FakeWorktrees implements Worktrees {
@Override
public boolean hasUncommitted(String worktreePath) {
// A path that was never added, or was marked gone, is reported clean, mirroring
// GitWorktrees' already-gone guard — never an error, so teardown still completes.
if (!worktreePaths.contains(worktreePath)) {
return false;
}
return dirty;
}
@@ -258,38 +258,6 @@ class WorktreeSessionManagerTest {
}
}
/**
* CB-576 review (fleetd #116). A worktree that is already gone (operator cleanup,
* {@code git worktree prune}, an earlier half-completed release) must not break teardown.
* {@code hasUncommitted} reports the missing path clean (mirroring {@code GitWorktrees}), so
* {@code release} still runs {@code notifyReleased} (the CB-516 fast-fail for a blocked
* {@code fleet_send} caller) and {@code launcher.stop} (so the pane is not orphaned), and falls
* through to the already-gone-tolerant {@code remove}. Since CB-581 this all happens because the
* notify-and-stop work sits in {@code release}'s {@code finally}/post-try block rather than a
* checked branch — this test pins that shape by construction.
*/
@Test
void releaseStillStopsPaneAndNotifiesWhenWorktreeIsGone() {
FakeHerdr herdr = new FakeHerdr();
FakeWorktrees worktrees = new FakeWorktrees().withRepoRoot("/repo").withPrefix("/wt");
SessionManager sessions = new SessionManager(workerService(herdr), worktrees);
List<SessionManager.ReleaseDetail> released = new java.util.concurrent.CopyOnWriteArrayList<>();
sessions.onRelease(released::add);
MemberSession s = sessions.acquire("ltms-local", null, "/caller/proj", null,
new WorktreeRequest("cb-576g", null));
worktrees.markGone(s.worktree());
sessions.release(s.paneId());
assertEquals(1, released.size(),
"notifyReleased must still fire when the worktree is already gone (CB-516)");
assertEquals(s.terminalId(), released.getFirst().terminalId());
assertTrue(herdr.called("pane.close"),
"the pane must still be stopped when the worktree is already gone");
assertEquals(1, worktrees.removeCalls().size(),
"release still calls the already-gone-tolerant remove");
}
@Test
void drainAllPreservesWorktreeOfIdleSession() {
FakeHerdr herdr = new FakeHerdr();