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
2 changed files with 266 additions and 34 deletions
@@ -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
@@ -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());
}
}