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

Seeding ~/.claude.json can still lose an external writer's change #247

New Issue
Closed
opened 2026-09-03 06:47:57 +02:00 by ltms · 3 comments
ltms commented 2026-09-03 06:47:57 +02:00
Owner
Copy Link
Copy Source

Follow-up to #149, which is merged (2e5b63f). #149 made the workspace-trust seed safe inside fleetd: the read-modify-write is atomic (ATOMIC_MOVE) and guarded by a process-wide lock, so two concurrent spawns can no longer lose each other's entry.

What is still broken. The lock is in-process. ~/.claude.json is also written by the operator's own running Claude Code, which fleetd cannot see or coordinate with. So this can still happen:

fleetd            operator's Claude Code
------            ----------------------
read file  (v1)
                  read file  (v1)
                  write file (v1 + their change)  -> v2
ATOMIC_MOVE (v1 + our change)                     -> v3

The move is atomic, so the file is never torn. But v3 was built from v1, so their change is gone. The window is small (one read plus a small edit), and it only opens when a spawn happens at the same moment Claude Code saves. It is real, not theoretical: the file is written on ordinary events like adding a project or finishing an OAuth refresh.

Why it was not fixed in #149. Fixing it needs a cross-process mechanism, and none of the options is obviously right. I chose to merge the in-process fix and file this rather than hold a third review round.

Options, roughly in order of cost:

  1. Compare-and-swap on content. Re-read the file just before the move; if its bytes changed since our read, start over. Cheap, no new file, no new dependency. It shrinks the window a lot but does not close it — a write landing between the re-read and the move is still lost.
  2. An advisory lock file (~/.claude.json.lock, O_CREAT|O_EXCL or flock). This actually closes the window, but only if the other writer takes the same lock — and Claude Code does not. So on its own it buys nothing against the real writer. It is only worth doing if it is paired with (1).
  3. Do not write the file at all. Find another way to mark a worktree as trusted, so fleetd never touches a file it does not own. This is the only option that removes the class of bug rather than narrowing it. I do not know yet whether an alternative exists — that is the first thing to check.

Suggested first step: check whether option 3 is possible before building 1 or 2. If it is not, do 1, and say plainly in the code comment that the window is narrowed and not closed.

Acceptance criteria:

  • Either fleetd no longer writes ~/.claude.json, or the seed detects an external write and retries.
  • A test proves it: simulate a writer that changes the file between our read and our write, then assert that writer's change survives.
  • If any window remains, a comment at the call site says so and says how wide it is. No silent "fixed".
Follow-up to #149, which is merged (`2e5b63f`). #149 made the workspace-trust seed safe **inside fleetd**: the read-modify-write is atomic (`ATOMIC_MOVE`) and guarded by a process-wide lock, so two concurrent spawns can no longer lose each other's entry. **What is still broken.** The lock is in-process. `~/.claude.json` is also written by the operator's own running Claude Code, which fleetd cannot see or coordinate with. So this can still happen: ``` fleetd operator's Claude Code ------ ---------------------- read file (v1) read file (v1) write file (v1 + their change) -> v2 ATOMIC_MOVE (v1 + our change) -> v3 ``` The move is atomic, so the file is never torn. But `v3` was built from `v1`, so **their change is gone**. The window is small (one read plus a small edit), and it only opens when a spawn happens at the same moment Claude Code saves. It is real, not theoretical: the file is written on ordinary events like adding a project or finishing an OAuth refresh. **Why it was not fixed in #149.** Fixing it needs a cross-process mechanism, and none of the options is obviously right. I chose to merge the in-process fix and file this rather than hold a third review round. **Options, roughly in order of cost:** 1. **Compare-and-swap on content.** Re-read the file just before the move; if its bytes changed since our read, start over. Cheap, no new file, no new dependency. It shrinks the window a lot but does not close it — a write landing between the re-read and the move is still lost. 2. **An advisory lock file** (`~/.claude.json.lock`, `O_CREAT|O_EXCL` or `flock`). This actually closes the window, **but only if the other writer takes the same lock** — and Claude Code does not. So on its own it buys nothing against the real writer. It is only worth doing if it is paired with (1). 3. **Do not write the file at all.** Find another way to mark a worktree as trusted, so fleetd never touches a file it does not own. This is the only option that removes the class of bug rather than narrowing it. I do not know yet whether an alternative exists — that is the first thing to check. **Suggested first step:** check whether option 3 is possible before building 1 or 2. If it is not, do 1, and say plainly in the code comment that the window is narrowed and not closed. **Acceptance criteria:** - Either fleetd no longer writes `~/.claude.json`, or the seed detects an external write and retries. - A test proves it: simulate a writer that changes the file between our read and our write, then assert that writer's change survives. - If any window remains, a comment at the call site says so and says how wide it is. No silent "fixed".
ltms commented 2026-09-03 07:32:43 +02:00
Author
Owner
Copy Link
Copy Source

Measured live on 2026-09-03 after deploying #149. The external writer is worse than I described when I filed this, and it changes which fix is right.

I spawned a real claude-code member into a fresh worktree and then read <configDir>/.claude.json:

has trust key (hasTrustDialogAccepted):       28 of 28 projects
has onboarding key (hasCompletedProjectOnboarding): 0 of 28 projects

ClaudeCodeLauncher.java:561-562 writes both keys, and our entry was created by the running jar minutes before I looked. So hasCompletedProjectOnboarding was written and is gone — not only from our entry, but from every entry in the file.

What that means

I filed this as a race: a save landing in the window between our read and our ATOMIC_MOVE, losing whatever changed in between. That framing is too kind. Claude Code normalises the whole file on save and drops keys it does not expect in that position. It is not losing a race with us; it is deliberately rewriting what we wrote, every time, with no window involved.

This rules out option 1

Compare-and-swap on content was the cheap option in this ticket, and it is now clearly the wrong one. CAS defends against a concurrent write landing between a read and a write. It does nothing when the other writer's steady-state behaviour is to remove your key. A retry loop would re-add hasCompletedProjectOnboarding and the next save would strip it again — an endless, invisible fight over a key that does nothing.

Option 2 (an advisory lock) was already weak, because Claude Code does not take our lock. This measurement does not rescue it.

What is left, and it is simpler than the original three options

Stop writing the second key. We now know two things about hasCompletedProjectOnboarding: it is not retained, and it is not needed — the member reached idle with only hasTrustDialogAccepted present (see #149). Writing it accomplishes nothing and is the only part of our write that provably conflicts with the other writer.

That leaves hasTrustDialogAccepted, which does survive — all 28 entries carry it, so Claude Code preserves that key rather than stripping it. The narrow race on that one key is still theoretically real, but its blast radius is one boolean that both writers agree should be true, which is the benign case.

So the revised suggestion, replacing the three options above:

  1. Drop the hasCompletedProjectOnboarding write. One line.
  2. Keep the atomic write and the in-process lock — both are still correct and both are cheap.
  3. Add a comment at the call site recording why only one key is written, with this measurement, so nobody re-adds the second one later thinking it was an oversight.

Acceptance criteria, revised:

  • fleetd writes only hasTrustDialogAccepted.
  • A test pins that it writes exactly that key and not the other, and the test's message says why — otherwise this looks like a missing feature to the next reader.
  • A live spawn into a fresh worktree still reaches idle. That is the only thing the write exists to achieve.

The original option 3 — find a way not to write the file at all — is still the ideal, and still unexplored. But it is now much less urgent, because what remains is a single boolean the other writer already agrees with.

Measured live on 2026-09-03 after deploying #149. **The external writer is worse than I described when I filed this, and it changes which fix is right.** I spawned a real `claude-code` member into a fresh worktree and then read `<configDir>/.claude.json`: ``` has trust key (hasTrustDialogAccepted): 28 of 28 projects has onboarding key (hasCompletedProjectOnboarding): 0 of 28 projects ``` `ClaudeCodeLauncher.java:561-562` writes **both** keys, and our entry was created by the running jar minutes before I looked. So `hasCompletedProjectOnboarding` was written and is gone — not only from our entry, but from **every** entry in the file. ## What that means I filed this as a **race**: a save landing in the window between our read and our `ATOMIC_MOVE`, losing whatever changed in between. That framing is too kind. Claude Code **normalises the whole file on save** and drops keys it does not expect in that position. It is not losing a race with us; it is deliberately rewriting what we wrote, every time, with no window involved. ## This rules out option 1 Compare-and-swap on content was the cheap option in this ticket, and it is now clearly the wrong one. CAS defends against a concurrent write landing between a read and a write. It does nothing when the other writer's *steady-state behaviour* is to remove your key. A retry loop would re-add `hasCompletedProjectOnboarding` and the next save would strip it again — an endless, invisible fight over a key that does nothing. Option 2 (an advisory lock) was already weak, because Claude Code does not take our lock. This measurement does not rescue it. ## What is left, and it is simpler than the original three options **Stop writing the second key.** We now know two things about `hasCompletedProjectOnboarding`: it is not retained, and it is not needed — the member reached `idle` with only `hasTrustDialogAccepted` present (see #149). Writing it accomplishes nothing and is the only part of our write that provably conflicts with the other writer. That leaves `hasTrustDialogAccepted`, which **does** survive — all 28 entries carry it, so Claude Code preserves that key rather than stripping it. The narrow race on that one key is still theoretically real, but its blast radius is one boolean that both writers agree should be `true`, which is the benign case. So the revised suggestion, replacing the three options above: 1. Drop the `hasCompletedProjectOnboarding` write. One line. 2. Keep the atomic write and the in-process lock — both are still correct and both are cheap. 3. Add a comment at the call site recording *why* only one key is written, with this measurement, so nobody re-adds the second one later thinking it was an oversight. **Acceptance criteria, revised:** - fleetd writes only `hasTrustDialogAccepted`. - A test pins that it writes exactly that key and not the other, and the test's message says why — otherwise this looks like a missing feature to the next reader. - A live spawn into a fresh worktree still reaches `idle`. That is the only thing the write exists to achieve. The original option 3 — find a way not to write the file at all — is still the ideal, and still unexplored. But it is now much less urgent, because what remains is a single boolean the other writer already agrees with.
ltms commented 2026-09-03 11:11:12 +02:00
Author
Owner
Copy Link
Copy Source

This ticket names the wrong file, and that made the race look far less likely than it is. Measured on the live host today.

Option 3 is already built, and it does not solve the problem

ClaudeCodeLauncher already exports CLAUDE_CONFIG_DIR to the member from cfg.configDir() (line 284) and already seeds <configDir>/.claude.json rather than ~/.claude.json when that is set (lines 531-533). So "point fleetd at a file it owns" needs no new code — only config.

And the live config already does it. Every claude-code profile sets configDir:

profile       configDir
local         /Users/dai.ha/.ccs/instances/gx10
local-direct  /Users/dai.ha/.ccs/instances/gx10
opus          /Users/dai.ha/.ccs/instances/ltms
sonnet        /Users/dai.ha/.ccs/instances/ltms

(sol, terra, xf, gx are opencode and never seed a trust dialog.)

So in production fleetd has never written ~/.claude.json. That part of the premise is wrong.

But the file it does write is not one fleetd owns either

CLAUDE_CONFIG_DIR for the operator's own lead session is /Users/dai.ha/.ccs/instances/ltms — the same directory the sonnet and opus profiles point at. Its .claude.json is 125528 bytes with 35 projects, 8 of them fleetd worktree entries.

Its mtime moved to 16:03:21 today, minutes after a spawn, while the operator's Claude Code was running and writing the same file.

So configDir did not move the write onto a file nobody else touches. It moved it from one live file to another live file — the lead's own. The race described in this ticket is real and live; it is just not on ~/.claude.json. Naming that file made it look like a rare collision with an unrelated process. It is actually a collision with the session that is doing the spawning.

What this probably explains

~/.claude.json was corrupted today: it shrank to 20208 bytes and lost oauthAccount — a lost update, not a torn write, which is this ticket's exact signature.

The path there is #258: two test fixtures built a worktree-shaped @TempDir with configDir null, so the test suite — not production — wrote the real ~/.claude.json on every run. 116 dead JUnit entries had accumulated there. A test run reading v1 while the operator's Claude Code wrote v2 produces precisely the observed damage.

I cannot prove this. The corrupted copy is no longer on disk, so the evidence is gone. But it is the only mechanism I found that produces that signature, and both halves of it are independently confirmed: #258 (fixed, ac790e4) put the test path on the real file, and this ticket is why any writer of that file can lose a concurrent change.

Revised recommendation

Option 3 as written — "find another way to mark a worktree as trusted" — is not available. claude --help offers no flag to accept workspace trust. The dialog is skipped only in non-interactive mode (-p, or a non-TTY stdout), and a member runs interactively in a pane, so that does not apply. The per-project trust flag lives only in projects.<cwd>.hasTrustDialogAccepted in a global-per-config-dir file.

Pointing configDir at a directory fleetd genuinely owns is not a fix either: the member is given that same directory as its own CLAUDE_CONFIG_DIR, and it is set to the operator's instance dir precisely so the member inherits that instance's configuration.

So: do option 1 — re-read immediately before the ATOMIC_MOVE, and start over if the bytes changed since the read. And say plainly at the call site that this narrows the window and does not close it, because a write landing between the re-read and the move is still lost.

Two things the acceptance criteria should add, given the above:

  1. The comment must name the real file. Anyone reading "we might race the operator's ~/.claude.json" will assume it is rare. The truthful sentence is that fleetd writes the lead's own config file, on every claude-code spawn.
  2. Make the unsafe default loud. configDir unset is the only path to ~/.claude.json, and it is the default. Nothing warns. #258 shows what that costs when something reaches it by accident — 116 writes to an operator's file with a green build every time.
**This ticket names the wrong file, and that made the race look far less likely than it is. Measured on the live host today.** ## Option 3 is already built, and it does not solve the problem `ClaudeCodeLauncher` already exports `CLAUDE_CONFIG_DIR` to the member from `cfg.configDir()` (line 284) and already seeds `<configDir>/.claude.json` rather than `~/.claude.json` when that is set (lines 531-533). So "point fleetd at a file it owns" needs no new code — only config. And the live config already does it. Every claude-code profile sets `configDir`: ``` profile configDir local /Users/dai.ha/.ccs/instances/gx10 local-direct /Users/dai.ha/.ccs/instances/gx10 opus /Users/dai.ha/.ccs/instances/ltms sonnet /Users/dai.ha/.ccs/instances/ltms ``` (`sol`, `terra`, `xf`, `gx` are opencode and never seed a trust dialog.) **So in production fleetd has never written `~/.claude.json`.** That part of the premise is wrong. ## But the file it does write is not one fleetd owns either `CLAUDE_CONFIG_DIR` for the operator's own lead session is `/Users/dai.ha/.ccs/instances/ltms` — **the same directory the `sonnet` and `opus` profiles point at**. Its `.claude.json` is 125528 bytes with 35 projects, 8 of them fleetd worktree entries. Its mtime moved to `16:03:21` today, minutes after a spawn, while the operator's Claude Code was running and writing the same file. So `configDir` did not move the write onto a file nobody else touches. It moved it from one live file to another live file — the lead's own. **The race described in this ticket is real and live; it is just not on `~/.claude.json`.** Naming that file made it look like a rare collision with an unrelated process. It is actually a collision with the session that is doing the spawning. ## What this probably explains `~/.claude.json` was corrupted today: it shrank to 20208 bytes and lost `oauthAccount` — a **lost update**, not a torn write, which is this ticket's exact signature. The path there is #258: two test fixtures built a worktree-shaped `@TempDir` with `configDir` null, so the test suite — not production — wrote the real `~/.claude.json` on every run. 116 dead JUnit entries had accumulated there. A test run reading v1 while the operator's Claude Code wrote v2 produces precisely the observed damage. **I cannot prove this.** The corrupted copy is no longer on disk, so the evidence is gone. But it is the only mechanism I found that produces that signature, and both halves of it are independently confirmed: #258 (fixed, `ac790e4`) put the test path on the real file, and this ticket is why any writer of that file can lose a concurrent change. ## Revised recommendation Option 3 as written — "find another way to mark a worktree as trusted" — is **not available**. `claude --help` offers no flag to accept workspace trust. The dialog is skipped only in non-interactive mode (`-p`, or a non-TTY stdout), and a member runs interactively in a pane, so that does not apply. The per-project trust flag lives only in `projects.<cwd>.hasTrustDialogAccepted` in a global-per-config-dir file. Pointing `configDir` at a directory fleetd genuinely owns is not a fix either: the member is given that same directory as its own `CLAUDE_CONFIG_DIR`, and it is set to the operator's instance dir precisely so the member inherits that instance's configuration. So: **do option 1** — re-read immediately before the `ATOMIC_MOVE`, and start over if the bytes changed since the read. And say plainly at the call site that this narrows the window and does not close it, because a write landing between the re-read and the move is still lost. Two things the acceptance criteria should add, given the above: 1. **The comment must name the real file.** Anyone reading "we might race the operator's `~/.claude.json`" will assume it is rare. The truthful sentence is that fleetd writes the *lead's own* config file, on every claude-code spawn. 2. **Make the unsafe default loud.** `configDir` unset is the only path to `~/.claude.json`, and it is the default. Nothing warns. #258 shows what that costs when something reaches it by accident — 116 writes to an operator's file with a green build every time.
ltms commented 2026-09-03 11:30:49 +02:00
Author
Owner
Copy Link
Copy Source

Fixed. PR #261 is merged to main as 34480ce. Full build: 1261 tests, 0 failures.

What shipped

seedTrustDialog now does a compare-and-swap with a bounded retry, inside the existing TRUST_JSON_LOCK:

  1. Read the target file's bytes.
  2. Build the update from them.
  3. Immediately before the ATOMIC_MOVE, re-read the bytes and compare. If they changed, someone else wrote in between — throw the attempt away and rebuild from the fresh bytes.
  4. Up to 5 attempts. On exhausting them, write nothing and log a WARN naming the cwd and the file.

Step 4 is the important one. An unseeded member shows the trust dialog and fails to reach an injectable state. That is visible, logged and recoverable — you retry the spawn. Writing our stale copy over the operator's live config is silent and not recoverable. So the code fails toward the recoverable outcome.

There is also a new WARN every time the seed is about to target the default ~/.claude.json (configDir unset). That path is the only one that touches the operator's home file, and it is the default, so a profile that simply forgot to set configDir previously got no signal at all.

The ticket named the wrong file, and I should correct that

I filed this saying the file at risk was ~/.claude.json. That is not where the live race is.

The real race is against <configDir>/.claude.json. On this host that resolves to /Users/dai.ha/.ccs/instances/ltms/.claude.json — the lead's own live config file, being read and written by the very session doing the spawning. I confirmed it from the config:

profile kind configDir
local, local-direct claude-code /Users/dai.ha/.ccs/instances/gx10
opus, sonnet claude-code /Users/dai.ha/.ccs/instances/ltms
gx, sol, terra, xf opencode unset

So this was never an exotic collision. It was a lost update on every ordinary spawn of opus or sonnet. Every claude-code profile has a configDir; the four without one are all opencode, which never calls this method at all.

What this does NOT fix — stated plainly

The window is narrowed, not closed. A write landing between the final re-read and the ATOMIC_MOVE itself is still lost. There is no OS-level compare-and-swap on a plain file, only this cooperative narrowing.

That limit is written into the method's javadoc, not just here, so the next person to read the code does not assume the race is gone and stop looking.

Proof the guard is load-bearing

The worker removed the CAS check (if (false && !Arrays.equals(before, atMove)), i.e. move unconditionally, as before the fix) and both new race tests went red. The failure message is worth quoting because it says what was actually lost:

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.
==> expected: <true> but was: <false>

The file was then restored and verified byte-identical before committing.

Was this the cause of the corruption on 2026-09-03?

I still cannot say it was, and I am not going to claim it. No ~/.claude.json.CORRUPTED.* file exists on disk any more, so there is nothing left to examine. What I can say is that the mechanism was real, it was reachable on every opus/sonnet spawn, and it is now bounded.

Closing.

Fixed. PR #261 is merged to `main` as `34480ce`. Full build: 1261 tests, 0 failures. ## What shipped `seedTrustDialog` now does a compare-and-swap with a bounded retry, inside the existing `TRUST_JSON_LOCK`: 1. Read the target file's bytes. 2. Build the update from them. 3. **Immediately before the `ATOMIC_MOVE`, re-read the bytes and compare.** If they changed, someone else wrote in between — throw the attempt away and rebuild from the fresh bytes. 4. Up to 5 attempts. **On exhausting them, write nothing** and log a WARN naming the cwd and the file. Step 4 is the important one. An unseeded member shows the trust dialog and fails to reach an injectable state. That is visible, logged and recoverable — you retry the spawn. Writing our stale copy over the operator's live config is silent and not recoverable. So the code fails toward the recoverable outcome. There is also a new WARN every time the seed is about to target the default `~/.claude.json` (`configDir` unset). That path is the only one that touches the operator's home file, and it is the default, so a profile that simply forgot to set `configDir` previously got no signal at all. ## The ticket named the wrong file, and I should correct that I filed this saying the file at risk was `~/.claude.json`. That is not where the live race is. The real race is against `<configDir>/.claude.json`. On this host that resolves to `/Users/dai.ha/.ccs/instances/ltms/.claude.json` — **the lead's own live config file**, being read and written by the very session doing the spawning. I confirmed it from the config: | profile | kind | configDir | |---|---|---| | `local`, `local-direct` | claude-code | `/Users/dai.ha/.ccs/instances/gx10` | | `opus`, `sonnet` | claude-code | `/Users/dai.ha/.ccs/instances/ltms` | | `gx`, `sol`, `terra`, `xf` | opencode | unset | So this was never an exotic collision. It was a lost update on **every ordinary spawn** of `opus` or `sonnet`. Every `claude-code` profile has a `configDir`; the four without one are all `opencode`, which never calls this method at all. ## What this does NOT fix — stated plainly The window is narrowed, not closed. A write landing between the final re-read and the `ATOMIC_MOVE` itself is still lost. There is no OS-level compare-and-swap on a plain file, only this cooperative narrowing. That limit is written into the method's javadoc, not just here, so the next person to read the code does not assume the race is gone and stop looking. ## Proof the guard is load-bearing The worker removed the CAS check (`if (false && !Arrays.equals(before, atMove))`, i.e. move unconditionally, as before the fix) and both new race tests went red. The failure message is worth quoting because it says what was actually lost: ``` 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. ==> expected: <true> but was: <false> ``` The file was then restored and verified byte-identical before committing. ## Was this the cause of the corruption on 2026-09-03? I still cannot say it was, and I am not going to claim it. No `~/.claude.json.CORRUPTED.*` file exists on disk any more, so there is nothing left to examine. What I can say is that the mechanism was real, it was reachable on every `opus`/`sonnet` spawn, and it is now bounded. Closing.
ltms closed this issue 2026-09-03 11:30:54 +02:00
Sign in to join this conversation.
No Branch/Tag Specified
Branches Tags
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
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: fleet/fleetd#247
Delete Branch "%!s()"

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?