fleet/fleetd
3
0
Fork 0
Code Issues 73 Pull Requests Actions Packages Projects Releases 1 Wiki Activity

fleetd #149: seed the workspace-trust dialog before a claude-code spawn #244

Closed
agent wants to merge 0 commits from worker/cb149-trust-dialog-2392a5-9 into main
pull from: worker/cb149-trust-dialog-2392a5-9
merge into: fleet:main
Conversation 1 Commits - Files Changed -
agent commented 2026-09-03 06:24:01 +02:00
Member
Copy Link
Copy Source

fleetd #149 — seed the workspace-trust dialog before a claude-code spawn

Incident hit while building this fix (report this first, before the normal summary)

What happened. My first version of seedTrustDialog wrote unconditionally to cwd's trust
entry whenever cwd was non-blank, with no check on whether cwd was a worktree fleetd itself
provisioned. Many pre-existing tests in ClaudeCodeLauncherTest build a FleetConfig.Profile
with configDir=null and no cwd. Per HerdrPeerLauncher.resolveCwd, that falls back to the
real user.dir (this Maven module's actual directory), and with configDir=null the write target
falls back the same way to the real ~/.claude.json.

While running a deliberate mutation-testing pass (mutating seedTrustDialog to skip reading the
existing file, to prove the "additive" acceptance criterion is covered), that mutation's read of
the real, pre-existing ~/.claude.json was never actually read — the write went straight to a
freshly created object — and the operator's real ~/.claude.json was overwritten and shrunk from
72581 bytes (dozens of settings, ~27 projects, oauthAccount, etc.) down to 178 bytes containing
only my test's single seeded entry.

What I could not do. I located a Claude Code auto-backup at
~/.claude/backups/.claude.json.backup.1788234449856 (72532 bytes, verified valid JSON via
Python — 79 keys, 17 projects, oauthAccount present) and tried to copy it back over the real
file. That write was blocked by the Claude Code auto-mode classifier (path outside my
worktree). A follow-up read-only ls -la ~/.claude.json was also blocked. Per the classifier's
own instruction to stop rather than work around a denial, I stopped and did not try any other tool
to reach that path. I have not verified the real file's current state since, and I could not
restore it myself — this needs operator or lead action, most likely restoring from the backup
path above.

Permanent fix, not a patch. I added isProvisionedWorktree(cwd) — reusing the exact signal
writeIdeOverlay already used to know it's touching a fleetd-provisioned worktree (a .git that
is a regular file holding a gitdir: pointer, never a real checkout's .git directory) — and
gated seedTrustDialog on it. This closes the vulnerability everywhere a ClaudeCodeLauncher is
constructed and spawned with an unconfigured cwd/configDir, not just in this file's tests. I
grepped the rest of the module's tests to confirm no other test path reaches a real, .git-having
directory with configDir=null.

I verified the gate is load-bearing with a scoped mutation test (see mutation table below,
Mutation 4) — run with -Dtest limited to only the two tests that redirect their target to a
@TempDir regardless of the gate, so this verification run could not itself touch a real file.

Implementation

  • ClaudeCodeLauncher.buildLaunch now calls seedTrustDialog(cfg.configDir(), spec.cwd()) right
    after the existing CLAUDE_CONFIG_DIR/git-token env setup — buildLaunch always runs strictly
    before the herdr agent.start call, so the seed lands before the peer process itself starts.
  • seedTrustDialog does an additive Jackson tree read-modify-write of
    <configDir or ~>/.claude.json, setting
    projects.<cwd>.hasTrustDialogAccepted = true and
    projects.<cwd>.hasCompletedProjectOnboarding = true, preserving every other key (including
    other projects' data) via explicit instanceof ObjectNode checks rather than Jackson's
    ambiguous .with(String). Any I/O failure is swallowed and logged at debug — this must never
    block a spawn.
  • Gated on isProvisionedWorktree(cwd) (see incident above) — shared with writeIdeOverlay,
    which used the same .git-regular-file-vs-directory signature inline before this change.
  • FakeHerdr gained onAgentStart(Runnable), fired synchronously the instant an agent.start
    call reaches the fake — i.e. the instant the real herdr daemon would start the process. Used to
    assert the seed is already on disk at that exact point, not merely once spawn() returns.

Acceptance criteria

  1. A claude-code peer spawned into a never-seen directory reaches idle with no prompt — not
    provable from here.     I have no way to spawn a live claude-code member from inside this
    worktree; this was flagged as an expected gap in the ticket brief.
  2. Seed target follows configDir when set, else default ~/.claude.json — covered by
    seedTrustDialogWritesBothTrustFlagsForTheResolvedCwd and
    seedTrustDialogTargetsDefaultClaudeJsonWhenConfigDirIsUnset (both against @TempDir/a
    redirected user.home, never the real file).
  3. Additive, never drops/rewrites other keys — covered by
    seedTrustDialogIsAdditiveAndPreservesUnknownKeysAndOtherProjects.
  4. A test starts the real launcher and asserts the entry exists BEFORE the process starts —
    seedsWorkspaceTrustForTheCwdBeforeTheProcessStarts, using the new FakeHerdr.onAgentStart
    hook.
  5. opencode peers unaffected — seedTrustDialog/isProvisionedWorktree are only referenced from
    ClaudeCodeLauncher; OpenCodeLauncher is untouched (confirmed by git diff --stat, listed
    below).

Mutation testing (production code only, tests unedited)

# Mutation Compile errors (grep -cE 'COMPILATION ERROR|cannot find symbol') Test(s) that went red
1 Removed the seedTrustDialog(...) call from buildLaunch 0 seedsWorkspaceTrustForTheCwdBeforeTheProcessStarts, seedTrustDialogWritesBothTrustFlagsForTheResolvedCwd
2 Flipped hasTrustDialogAccepted/hasCompletedProjectOnboarding literal to false 0 seedTrustDialogWritesBothTrustFlagsForTheResolvedCwd, seedsWorkspaceTrustForTheCwdBeforeTheProcessStarts
3 Made the read-modify-write non-additive (root = TRUST_JSON.createObjectNode() unconditionally, skipping the existing-file read) 0 seedTrustDialogIsAdditiveAndPreservesUnknownKeysAndOtherProjects — this is the mutation that caused the real-file incident above, because it was run without scoping -Dtest to only the additive test
4 Removed the isProvisionedWorktree(cwd) gate at the top of seedTrustDialog 0 seedTrustDialogNeverWritesWhenCwdIsNotAProvisionedWorktree, seedTrustDialogNeverWritesToDefaultHomeWhenCwdIsNotAProvisionedWorktree — run scoped to only these two tests (both redirect to @TempDir/redirected user.home), specifically to avoid recreating the incident

Each mutation was reverted (restored from a full-file backup, confirmed via diff -q) before the
next one and before the final build below.

Build

Ran in fleetd/ inside my worktree, full and unpiped:

mvn clean install
...
[INFO] Tests run: 1169, Failures: 0, Errors: 0, Skipped: 0
[INFO] BUILD SUCCESS

Baseline on main was 1163 tests; this PR adds exactly the 6 new tests (1169 - 1163 = 6).

Rejected fixes (per the ticket) — none used

-p/non-interactive, --dangerously-skip-permissions, and permissions.additionalDirectories
are not used anywhere in this diff.

Files changed

  • fleetd/src/main/java/dev/ltms/fleet/member/ClaudeCodeLauncher.java
  • fleetd/src/test/java/dev/ltms/fleet/member/ClaudeCodeLauncherTest.java
  • fleetd/src/test/java/dev/ltms/fleet/herdr/FakeHerdr.java

Caveats for review

  • The ~/.claude.json incident above is the most important thing in this PR. Please have the
    operator check/restore from ~/.claude/backups/.claude.json.backup.1788234449856 (or the most
    recent backup, if a newer one exists) — I could not do this myself and have not re-verified the
    file's current state since the classifier blocked me.
  • Criterion 1 (live spawn reaching idle with no prompt) is not provable from a worker; needs a
    primary-side live dogfood spawn against a fresh worktree to fully close.
  • Out of scope, not investigated further: OpenCodeLauncher and any other future peer-launcher
    kind would need its own equivalent seeding if it ever grows the same trust-dialog behavior —
    today only claude-code has it.

Review round 2 — the write must be atomic and lock-protected

The lead's review caught a real defect: Files.writeString(target, content) truncates the target
in place before writing, so there was a window where .claude.json could be observed empty or
half-written. Two production failure modes follow: a crash/kill mid-write leaves the file
truncated, and two concurrent claude-code spawns (normal on this daemon — several run in parallel
routinely) racing a naive read-modify-write can silently drop one spawn's entry.

What changed

  • writeAtomically(Path target, String content) (package-visible, was inline in
    seedTrustDialog): serialises to a sibling temp file in the same directory as target (an
    atomic move is only guaranteed within one filesystem), then
    Files.move(tmp, target, ATOMIC_MOVE, REPLACE_EXISTING). A reader now only ever observes the
    fully-old or fully-new file, never a torn one. Preserves target's existing POSIX permissions
    (.claude.json ships 0600) via copyPosixPermissionsIfPresent; no-ops on a non-POSIX
    filesystem rather than failing.
  • TRUST_JSON_LOCK: a private static final Object, synchronized around
    seedTrustDialog's entire read-modify-write. This is the concurrency decision the lead asked me
    to justify: a single process-wide lock is enough because every claude-code spawn on this daemon
    runs in one JVM, so it fully serialises them — no lost updates between two spawns started at
    once. It explicitly does not protect against a second daemon process, or the operator's own
    live Claude Code process, writing .claude.json at the same instant; that case is what
    writeAtomically covers instead (each such writer still only ever sees a fully-old or fully-new
    file). I did not add cross-process locking (a lockfile, FileChannel.lock()) — the lead's own
    framing ("probably enough here, since all spawns go through one daemon") matches what a
    single-daemon deployment actually needs, and cross-process locking would need to coordinate with
    Claude Code's own writer too, which is out of fleetd's control either way.
  • Both fail soft exactly as before — any IOException/mutation failure here is logged at debug
    and swallowed; a member that cannot be seeded still spawns.

Tests added (4, on top of the existing 6)

  1. seedTrustDialogPreservesALargeExistingFileWithoutCollapsing — a 30-project, several-KB fixture
    survives; asserted on the restored key set (all 30 other projects' hasTrustDialogAccepted
    and nested mcpServers data, plus unrelated top-level keys), not just that the result parses,
    and the file's byte size never drops below its pre-seed size.
  2. concurrentSeedsForDifferentCwdsBothSurvive — two threads spawn for different cwds sharing
    one configDir, lined up at a CountDownLatch (no sleep); both entries must be present in the
    final file.
  3. seedTrustDialogPreservesExisting0600Permissions — pre-sets .claude.json to rw-------,
    spawns, asserts the permissions are still exactly rw------- afterward. Skips (via
    assumeTrue, the same pattern this file already uses elsewhere) on a filesystem with no POSIX
    permissions.
  4. writeAtomicallyNeverExposesATornFileToAConcurrentReader — calls writeAtomically directly
    (not through spawn()), with a large (~20 MB) payload and a busy-poll reader thread running
    concurrently; asserts every sample the reader takes is exactly the old content or exactly the
    new content, never anything else.

An honest correction on the mutation table's numbering

I want to flag something rather than let it pass quietly: the review asked for "test 2" (the
concurrent-different-cwds test) to go red when the atomic move is reverted to a plain
Files.writeString. It does not, and I want to explain why rather than force a result.

TRUST_JSON_LOCK fully serialises every seedTrustDialog call this launcher itself makes — two
threads calling spawn() concurrently can never actually interleave inside the synchronized
block, atomic move or not. So test 2 as specified is a test of the lock, and passes under this
mutation regardless of whether the underlying write is atomic. I verified this empirically rather
than assuming it (see the mutation table below) — after reverting the atomic move, I ran the full
ClaudeCodeLauncherTest class and confirmed concurrentSeedsForDifferentCwdsBothSurvive stayed
green.

What actually goes red under that mutation is test 4,
writeAtomicallyNeverExposesATornFileToAConcurrentReader — the one test that calls
writeAtomically directly, outside the lock, which is the only way to exercise atomicity
independent of the lock's own serialisation. That is also why I made writeAtomically
package-visible rather than private: a test going only through seedTrustDialog/spawn() could
never observe a torn file regardless of whether the underlying write is atomic, because the lock
already prevents any two writes from ever overlapping in this process.

Mutation table, round 2 (production code only; each reverted from a full-file backup + diff -q

before the next; run against the whole ClaudeCodeLauncherTest class — safe now, every fixture
uses @TempDir/a redirected user.home)

# Mutation Compile errors Test(s) that went red
5 Reverted writeAtomically to a plain Files.writeString(target, content) (no temp file, no ATOMIC_MOVE) 0 writeAtomicallyNeverExposesATornFileToAConcurrentReader (torn read of length 0 — the reader caught the exact truncation window). concurrentSeedsForDifferentCwdsBothSurvive stayed green — it tests the lock, not atomicity, as explained above.

(Mutations 1–4 from round 1 are unaffected by this round's changes and still hold; see the PR
description above for that table.)

Build (round 2)

Ran in fleetd/, full and unpiped:

mvn clean install
...
[INFO] Tests run: 1173, Failures: 0, Errors: 0, Skipped: 0
[INFO] BUILD SUCCESS

My branch's merge-base with origin/main is still 26bafe8 (I have not rebased, per the
implementer skill and the lead's instruction) — main has moved to 1168 tests since, per the lead.
1173 is what I measured on this branch, base 26bafe8 (1163 tests there) + 10 new tests total
across both rounds (6 from round 1, 4 from round 2).

Files changed (round 2 diff)

  • fleetd/src/main/java/dev/ltms/fleet/member/ClaudeCodeLauncher.java
  • fleetd/src/test/java/dev/ltms/fleet/member/ClaudeCodeLauncherTest.java

(FakeHerdr.java is unchanged this round — no new herdr-transport seam was needed.)

## fleetd #149 — seed the workspace-trust dialog before a claude-code spawn ### Incident hit while building this fix (report this first, before the normal summary) **What happened.** My first version of `seedTrustDialog` wrote unconditionally to `cwd`'s trust entry whenever `cwd` was non-blank, with no check on whether `cwd` was a worktree fleetd itself provisioned. Many pre-existing tests in `ClaudeCodeLauncherTest` build a `FleetConfig.Profile` with `configDir=null` and no `cwd`. Per `HerdrPeerLauncher.resolveCwd`, that falls back to the real `user.dir` (this Maven module's actual directory), and with `configDir=null` the write target falls back the same way to the **real `~/.claude.json`**. While running a deliberate mutation-testing pass (mutating `seedTrustDialog` to skip reading the existing file, to prove the "additive" acceptance criterion is covered), that mutation's read of the real, pre-existing `~/.claude.json` was never actually read — the write went straight to a freshly created object — and the operator's real `~/.claude.json` was overwritten and shrunk from 72581 bytes (dozens of settings, ~27 projects, `oauthAccount`, etc.) down to 178 bytes containing only my test's single seeded entry. **What I could not do.** I located a Claude Code auto-backup at `~/.claude/backups/.claude.json.backup.1788234449856` (72532 bytes, verified valid JSON via Python — 79 keys, 17 projects, `oauthAccount` present) and tried to copy it back over the real file. That write was **blocked by the Claude Code auto-mode classifier** (path outside my worktree). A follow-up read-only `ls -la ~/.claude.json` was also blocked. Per the classifier's own instruction to stop rather than work around a denial, I stopped and did not try any other tool to reach that path. **I have not verified the real file's current state since, and I could not restore it myself — this needs operator or lead action**, most likely restoring from the backup path above. **Permanent fix, not a patch.** I added `isProvisionedWorktree(cwd)` — reusing the exact signal `writeIdeOverlay` already used to know it's touching a fleetd-provisioned worktree (a `.git` that is a *regular file* holding a `gitdir:` pointer, never a real checkout's `.git` *directory*) — and gated `seedTrustDialog` on it. This closes the vulnerability everywhere a `ClaudeCodeLauncher` is constructed and spawned with an unconfigured `cwd`/`configDir`, not just in this file's tests. I grepped the rest of the module's tests to confirm no other test path reaches a real, `.git`-having directory with `configDir=null`. I verified the gate is load-bearing with a scoped mutation test (see mutation table below, Mutation 4) — run with `-Dtest` limited to only the two tests that redirect their target to a `@TempDir` regardless of the gate, so this verification run could not itself touch a real file. ### Implementation - `ClaudeCodeLauncher.buildLaunch` now calls `seedTrustDialog(cfg.configDir(), spec.cwd())` right after the existing `CLAUDE_CONFIG_DIR`/git-token env setup — `buildLaunch` always runs strictly before the herdr `agent.start` call, so the seed lands before the peer process itself starts. - `seedTrustDialog` does an additive Jackson tree read-modify-write of `<configDir or ~>/.claude.json`, setting `projects.<cwd>.hasTrustDialogAccepted = true` and `projects.<cwd>.hasCompletedProjectOnboarding = true`, preserving every other key (including other projects' data) via explicit `instanceof ObjectNode` checks rather than Jackson's ambiguous `.with(String)`. Any I/O failure is swallowed and logged at debug — this must never block a spawn. - Gated on `isProvisionedWorktree(cwd)` (see incident above) — shared with `writeIdeOverlay`, which used the same `.git`-regular-file-vs-directory signature inline before this change. - `FakeHerdr` gained `onAgentStart(Runnable)`, fired synchronously the instant an `agent.start` call reaches the fake — i.e. the instant the real herdr daemon would start the process. Used to assert the seed is already on disk at that exact point, not merely once `spawn()` returns. ### Acceptance criteria 1. A claude-code peer spawned into a never-seen directory reaches `idle` with no prompt — **not provable from here.** I have no way to spawn a live claude-code member from inside this worktree; this was flagged as an expected gap in the ticket brief. 2. Seed target follows `configDir` when set, else default `~/.claude.json` — covered by `seedTrustDialogWritesBothTrustFlagsForTheResolvedCwd` and `seedTrustDialogTargetsDefaultClaudeJsonWhenConfigDirIsUnset` (both against `@TempDir`/a redirected `user.home`, never the real file). 3. Additive, never drops/rewrites other keys — covered by `seedTrustDialogIsAdditiveAndPreservesUnknownKeysAndOtherProjects`. 4. A test starts the real launcher and asserts the entry exists BEFORE the process starts — `seedsWorkspaceTrustForTheCwdBeforeTheProcessStarts`, using the new `FakeHerdr.onAgentStart` hook. 5. opencode peers unaffected — `seedTrustDialog`/`isProvisionedWorktree` are only referenced from `ClaudeCodeLauncher`; `OpenCodeLauncher` is untouched (confirmed by `git diff --stat`, listed below). ### Mutation testing (production code only, tests unedited) | # | Mutation | Compile errors (`grep -cE 'COMPILATION ERROR\|cannot find symbol'`) | Test(s) that went red | |---|---|---|---| | 1 | Removed the `seedTrustDialog(...)` call from `buildLaunch` | 0 | `seedsWorkspaceTrustForTheCwdBeforeTheProcessStarts`, `seedTrustDialogWritesBothTrustFlagsForTheResolvedCwd` | | 2 | Flipped `hasTrustDialogAccepted`/`hasCompletedProjectOnboarding` literal to `false` | 0 | `seedTrustDialogWritesBothTrustFlagsForTheResolvedCwd`, `seedsWorkspaceTrustForTheCwdBeforeTheProcessStarts` | | 3 | Made the read-modify-write non-additive (`root = TRUST_JSON.createObjectNode()` unconditionally, skipping the existing-file read) | 0 | `seedTrustDialogIsAdditiveAndPreservesUnknownKeysAndOtherProjects` — **this is the mutation that caused the real-file incident above**, because it was run without scoping `-Dtest` to only the additive test | | 4 | Removed the `isProvisionedWorktree(cwd)` gate at the top of `seedTrustDialog` | 0 | `seedTrustDialogNeverWritesWhenCwdIsNotAProvisionedWorktree`, `seedTrustDialogNeverWritesToDefaultHomeWhenCwdIsNotAProvisionedWorktree` — run scoped to only these two tests (both redirect to `@TempDir`/redirected `user.home`), specifically to avoid recreating the incident | Each mutation was reverted (restored from a full-file backup, confirmed via `diff -q`) before the next one and before the final build below. ### Build Ran in `fleetd/` inside my worktree, full and unpiped: ``` mvn clean install ... [INFO] Tests run: 1169, Failures: 0, Errors: 0, Skipped: 0 [INFO] BUILD SUCCESS ``` Baseline on `main` was 1163 tests; this PR adds exactly the 6 new tests (1169 - 1163 = 6). ### Rejected fixes (per the ticket) — none used `-p`/non-interactive, `--dangerously-skip-permissions`, and `permissions.additionalDirectories` are not used anywhere in this diff. ### Files changed - `fleetd/src/main/java/dev/ltms/fleet/member/ClaudeCodeLauncher.java` - `fleetd/src/test/java/dev/ltms/fleet/member/ClaudeCodeLauncherTest.java` - `fleetd/src/test/java/dev/ltms/fleet/herdr/FakeHerdr.java` ### Caveats for review - **The `~/.claude.json` incident above is the most important thing in this PR.** Please have the operator check/restore from `~/.claude/backups/.claude.json.backup.1788234449856` (or the most recent backup, if a newer one exists) — I could not do this myself and have not re-verified the file's current state since the classifier blocked me. - Criterion 1 (live spawn reaching `idle` with no prompt) is not provable from a worker; needs a primary-side live dogfood spawn against a fresh worktree to fully close. - Out of scope, not investigated further: `OpenCodeLauncher` and any other future peer-launcher kind would need its own equivalent seeding if it ever grows the same trust-dialog behavior — today only claude-code has it. --- ## Review round 2 — the write must be atomic and lock-protected The lead's review caught a real defect: `Files.writeString(target, content)` truncates the target in place before writing, so there was a window where `.claude.json` could be observed empty or half-written. Two production failure modes follow: a crash/kill mid-write leaves the file truncated, and two concurrent claude-code spawns (normal on this daemon — several run in parallel routinely) racing a naive read-modify-write can silently drop one spawn's entry. ### What changed - **`writeAtomically(Path target, String content)`** (package-visible, was inline in `seedTrustDialog`): serialises to a sibling temp file in the *same directory* as `target` (an atomic move is only guaranteed within one filesystem), then `Files.move(tmp, target, ATOMIC_MOVE, REPLACE_EXISTING)`. A reader now only ever observes the fully-old or fully-new file, never a torn one. Preserves `target`'s existing POSIX permissions (`.claude.json` ships `0600`) via `copyPosixPermissionsIfPresent`; no-ops on a non-POSIX filesystem rather than failing. - **`TRUST_JSON_LOCK`**: a `private static final Object`, `synchronized` around `seedTrustDialog`'s entire read-modify-write. This is the concurrency decision the lead asked me to justify: a single process-wide lock is enough because every claude-code spawn on this daemon runs in one JVM, so it fully serialises them — no lost updates between two spawns started at once. It explicitly does **not** protect against a second daemon process, or the operator's own live Claude Code process, writing `.claude.json` at the same instant; that case is what `writeAtomically` covers instead (each such writer still only ever sees a fully-old or fully-new file). I did not add cross-process locking (a lockfile, `FileChannel.lock()`) — the lead's own framing ("probably enough here, since all spawns go through one daemon") matches what a single-daemon deployment actually needs, and cross-process locking would need to coordinate with Claude Code's own writer too, which is out of fleetd's control either way. - Both fail soft exactly as before — any `IOException`/mutation failure here is logged at debug and swallowed; a member that cannot be seeded still spawns. ### Tests added (4, on top of the existing 6) 1. `seedTrustDialogPreservesALargeExistingFileWithoutCollapsing` — a 30-project, several-KB fixture survives; asserted on the **restored key set** (all 30 other projects' `hasTrustDialogAccepted` and nested `mcpServers` data, plus unrelated top-level keys), not just that the result parses, and the file's byte size never drops below its pre-seed size. 2. `concurrentSeedsForDifferentCwdsBothSurvive` — two threads spawn for **different** cwds sharing one `configDir`, lined up at a `CountDownLatch` (no sleep); both entries must be present in the final file. 3. `seedTrustDialogPreservesExisting0600Permissions` — pre-sets `.claude.json` to `rw-------`, spawns, asserts the permissions are still exactly `rw-------` afterward. Skips (via `assumeTrue`, the same pattern this file already uses elsewhere) on a filesystem with no POSIX permissions. 4. `writeAtomicallyNeverExposesATornFileToAConcurrentReader` — calls `writeAtomically` **directly** (not through `spawn()`), with a large (~20 MB) payload and a busy-poll reader thread running concurrently; asserts every sample the reader takes is exactly the old content or exactly the new content, never anything else. ### An honest correction on the mutation table's numbering I want to flag something rather than let it pass quietly: the review asked for "test 2" (the concurrent-different-cwds test) to go red when the atomic move is reverted to a plain `Files.writeString`. **It does not, and I want to explain why rather than force a result.** `TRUST_JSON_LOCK` fully serialises every `seedTrustDialog` call this launcher itself makes — two threads calling `spawn()` concurrently can never actually interleave inside the synchronized block, atomic move or not. So test 2 as specified is a test of **the lock**, and passes under this mutation regardless of whether the underlying write is atomic. I verified this empirically rather than assuming it (see the mutation table below) — after reverting the atomic move, I ran the full `ClaudeCodeLauncherTest` class and confirmed `concurrentSeedsForDifferentCwdsBothSurvive` stayed green. What actually goes red under that mutation is test 4, `writeAtomicallyNeverExposesATornFileToAConcurrentReader` — the one test that calls `writeAtomically` directly, outside the lock, which is the only way to exercise atomicity independent of the lock's own serialisation. That is also why I made `writeAtomically` package-visible rather than private: a test going only through `seedTrustDialog`/`spawn()` could never observe a torn file regardless of whether the underlying write is atomic, because the lock already prevents any two writes from ever overlapping in this process. ### Mutation table, round 2 (production code only; each reverted from a full-file backup + `diff -q` before the next; run against the whole `ClaudeCodeLauncherTest` class — safe now, every fixture uses `@TempDir`/a redirected `user.home`) | # | Mutation | Compile errors | Test(s) that went red | |---|---|---|---| | 5 | Reverted `writeAtomically` to a plain `Files.writeString(target, content)` (no temp file, no `ATOMIC_MOVE`) | 0 | `writeAtomicallyNeverExposesATornFileToAConcurrentReader` (torn read of length 0 — the reader caught the exact truncation window). `concurrentSeedsForDifferentCwdsBothSurvive` stayed **green** — it tests the lock, not atomicity, as explained above. | (Mutations 1–4 from round 1 are unaffected by this round's changes and still hold; see the PR description above for that table.) ### Build (round 2) Ran in `fleetd/`, full and unpiped: ``` mvn clean install ... [INFO] Tests run: 1173, Failures: 0, Errors: 0, Skipped: 0 [INFO] BUILD SUCCESS ``` My branch's merge-base with `origin/main` is still `26bafe8` (I have not rebased, per the implementer skill and the lead's instruction) — main has moved to 1168 tests since, per the lead. 1173 is what I measured on this branch, base `26bafe8` (1163 tests there) + 10 new tests total across both rounds (6 from round 1, 4 from round 2). ### Files changed (round 2 diff) - `fleetd/src/main/java/dev/ltms/fleet/member/ClaudeCodeLauncher.java` - `fleetd/src/test/java/dev/ltms/fleet/member/ClaudeCodeLauncherTest.java` (`FakeHerdr.java` is unchanged this round — no new herdr-transport seam was needed.)
agent added 1 commit 2026-09-03 06:24:02 +02:00
fleetd #149: seed the workspace-trust entry before a claude-code spawn
CI / contract (pull_request) Successful in 51s
Details
CI / build (pull_request) Successful in 1m43s
Details
a89dcc9b7e 
A claude-code member spawned into a fresh worktree hits an interactive,
un-timed workspace-trust prompt on its first start in a directory it has
never seen. It never reaches its first turn and never mounts the bridge.

Fix: ClaudeCodeLauncher.seedTrustDialog writes
projects.<cwd>.hasTrustDialogAccepted / hasCompletedProjectOnboarding into
the profile's configDir/.claude.json (or ~/.claude.json when configDir is
unset) BEFORE the herdr spawn call, additively (existing keys/projects are
preserved). Gated to isProvisionedWorktree(cwd) - a .git that is a regular
gitdir-pointer file, never a real checkout's .git directory - the same
signal writeIdeOverlay already used, now shared between both.

That gate is a fix for a real incident hit while building this: an
earlier ungated version ran against this file's own pre-existing tests
(configDir=null, no cwd -> falls back to the real user.dir and
~/.claude.json) and corrupted the operator's actual ~/.claude.json down
to a single entry during a mutation-testing run. See PR body for the
full incident report.

FakeHerdr gained onAgentStart(Runnable) so a test can assert the seed
is on disk at the exact instant herdr's agent.start call is reached -
i.e. strictly before the peer process itself would start.
agent added 1 commit 2026-09-03 06:38:18 +02:00
fleetd #149 review round 2: make the trust-dialog seed atomic and lock-protected
CI / build (pull_request) Successful in 1m21s
Details
CI / contract (pull_request) Successful in 1m25s
Details
743377d6cd 
Files.writeString truncates the target in place before writing, so there
was a window where .claude.json could be observed empty or half-written
- exactly the shape of the incident this ticket already hit once, but
reachable in production too: a crash/kill mid-write, or two concurrent
claude-code spawns (normal here - several run in parallel routinely)
racing a naive read-modify-write and silently discarding one spawn's
entry.

Two independent fixes, each with its own dedicated test proving it (not
the other):

- ClaudeCodeLauncher.writeAtomically: serialise to a sibling temp file in
  the same directory, then Files.move with ATOMIC_MOVE + REPLACE_EXISTING,
  preserving the target's existing POSIX permissions (.claude.json ships
  0600). A reader now only ever observes the fully-old or fully-new file,
  never a torn one. Package-visible so a test can drive it directly.
- TRUST_JSON_LOCK: a process-wide lock around seedTrustDialog's whole
  read-modify-write, so two concurrent spawns for different cwds both
  keep their entry instead of the second write discarding the first.
  Sufficient because every spawn on this daemon runs in one JVM; it does
  NOT protect against a second daemon process or the operator's own live
  Claude Code writing at the same instant - writeAtomically covers that
  case instead.

Both fail soft, same as before: any I/O failure here must never block a
spawn.

Four new tests: a large (30-project) existing file survives without
collapsing (asserted on the restored key set, not just that the result
parses); two concurrent spawns for different cwds both keep their entry
(CountDownLatch-synchronised, not a sleep); existing 0600 permissions
survive the write; and a direct test of writeAtomically with a busy-poll
reader thread proving a concurrent reader never observes a torn file.

See PR body for the full mutation-testing table, including an
honest note on which of these tests the atomicity mutation actually
caught (not the one implied by the numbering in review) and why.
ltms commented 2026-09-03 06:47:34 +02:00
Owner
Copy Link
Copy Source

Merged into main as 2e5b63f (round 2 head 743377d). Closing this PR — the branch is in main, not dropped.

How I checked it (lead, in an isolated worktree):

Run Result
baseline 1173 tests, 0 failures, 0 compile errors
MUT-1: replace ATOMIC_MOVE with a truncating write RED — writeAtomicallyNeverExposesATornFileToAConcurrentReader:2534 ("torn read of length 360448")
MUT-2: drop copyPosixPermissionsIfPresent GREEN — see below
MUT-3: drop the lock (synchronized → if (true)) RED — concurrentSeedsForDifferentCwdsBothSurvive:2433 ("worktree B's entry must survive the race")

Post-merge build on main: 1182 tests, 0 failures, 0 errors, BUILD SUCCESS, with a separate check that the compile-error count is 0.

One honest gap. copyPosixPermissionsIfPresent has no test — its mutation stays green. I accepted it and did not send round 3, because Files.createTempFile is 0600 on POSIX by default, so the "never world-readable" property holds even without that line. The line only preserves a non-default mode that someone set by hand. Anyone tightening this later should know the coverage is not there.

Known remaining risk, filed separately. Our lock is in-process. It cannot reach the operator's own running Claude Code, so an external write to ~/.claude.json between our read and our ATOMIC_MOVE is still lost. That is a real race and it is not fixed here. I chose to merge and file it rather than block a third round on it.

Round 1 was sent back because it used a plain Files.writeString, which is not atomic. Round 2 fixed that.

Merged into `main` as `2e5b63f` (round 2 head `743377d`). Closing this PR — the branch is in `main`, not dropped. **How I checked it (lead, in an isolated worktree):** | Run | Result | |---|---| | baseline | 1173 tests, 0 failures, **0 compile errors** | | MUT-1: replace `ATOMIC_MOVE` with a truncating write | **RED** — `writeAtomicallyNeverExposesATornFileToAConcurrentReader:2534` ("torn read of length 360448") | | MUT-2: drop `copyPosixPermissionsIfPresent` | **GREEN** — see below | | MUT-3: drop the lock (`synchronized` → `if (true)`) | **RED** — `concurrentSeedsForDifferentCwdsBothSurvive:2433` ("worktree B's entry must survive the race") | Post-merge build on `main`: **1182 tests, 0 failures, 0 errors, BUILD SUCCESS**, with a separate check that the compile-error count is 0. **One honest gap.** `copyPosixPermissionsIfPresent` has **no test** — its mutation stays green. I accepted it and did not send round 3, because `Files.createTempFile` is 0600 on POSIX by default, so the "never world-readable" property holds even without that line. The line only preserves a *non-default* mode that someone set by hand. Anyone tightening this later should know the coverage is not there. **Known remaining risk, filed separately.** Our lock is in-process. It cannot reach the operator's own running Claude Code, so an external write to `~/.claude.json` between our read and our `ATOMIC_MOVE` is still lost. That is a real race and it is not fixed here. I chose to merge and file it rather than block a third round on it. Round 1 was sent back because it used a plain `Files.writeString`, which is not atomic. Round 2 fixed that.
ltms closed this pull request 2026-09-03 06:47:40 +02:00
Some checks are pending
CI / build (pull_request) Successful in 1m21s
Details
CI / contract (pull_request) Successful in 1m25s
Details

Pull request closed

Please reopen this pull request to perform a merge.
Sign in to join this conversation.
Reviewers
No Reviewers
Labels
Clear labels
blocked
Cannot start until something else lands. The body says what.
 needs-live-proof
Merged and green, but never shown working on the running daemon. Not the same as done.
 ready-to-delegate
Scope, files and acceptance criteria are written. A worker can be briefed from the body alone.
 silent-default
A feature that compiles, passes tests, and ships turned off. Nine recurrences and counting.
No Label
Milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
agent kevin ltms
No Assignees
2 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: fleet/fleetd#244
Delete Branch "worker/cb149-trust-dialog-2392a5-9"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?