+106
@@ -3122,6 +3122,25 @@ file, and it is the default, so a profile that simply forgot the setting used to
|
||||
final re-read and the move itself is still lost. There is no operating-system compare-and-swap on a
|
||||
plain file, only this cooperative narrowing. Do not read the CAS as making the race gone.
|
||||
|
||||
**The third gotcha: under `memberHerdrSocket` the seed used to write the wrong home (fleetd #285).**
|
||||
When `memberHerdrSocket:` is set, the member pane runs as a *different OS user with its own `$HOME`*.
|
||||
The seeding code could not see that at all — it was a `static` method, so it had no access to the
|
||||
launcher's own `memberHerdrSocketConfigured()` check. With `configDir` unset it therefore wrote
|
||||
**fleetd's own `~/.claude.json`** — the operator's real file — while believing it was seeding the
|
||||
member's. The member never got a seed, and the operator's file was edited for nothing.
|
||||
|
||||
The sibling method one line below, `writeCharterFile`, had already solved this: it refuses the spawn
|
||||
when it cannot place a file where a different-uid member can read it. The trust seed now does the
|
||||
same. Under `memberHerdrSocket` it requires **both** `configDir` (so there is a member-readable
|
||||
target at all) **and** `worktreeGroup` (so the `0600` file it writes can be shared), and refuses the
|
||||
spawn — naming which key is missing — before touching any file. When both are set, the written file
|
||||
is chgrp'd and chmod'd to `rw-r-----` for that group, so the member's OS user can actually open it.
|
||||
|
||||
The refusal is deliberate rather than a degradation. A member with no readable trust seed is not
|
||||
"slightly worse": it sits on the interactive dialog forever and never calls `fleet_reply`, which is
|
||||
the exact failure this whole feature exists to prevent. **With `memberHerdrSocket` absent — every
|
||||
live fleet today — nothing about this changed.**
|
||||
|
||||
## A worktree tells you which of its config files are stubs
|
||||
|
||||
**What.** A provisioned worktree neutralizes `.mcp.json`, `opencode.json` and `.autoenv`: the copy in
|
||||
@@ -3410,3 +3429,90 @@ repo's recorded pointer is deliberately never updated (committing it is on the n
|
||||
`git submodule update --init` in a worker's worktree checks out a **months-old** snapshot. During
|
||||
this very backfill a worker reported that an entry "does not exist anywhere in the repo" when it had
|
||||
been on the page for hours. Paste the relevant text into the brief instead of pointing at the file.
|
||||
|
||||
## A dead member's seat comes back
|
||||
|
||||
**What.** A member whose backend errored (`BACKEND_ERROR`) or whose turn failed (`FAILED`) no longer
|
||||
counts against its profile's `maxLoad`. Its row stays in `fleet_list` so you can still see what
|
||||
happened, but a fresh `fleet_spawn` on that profile is granted.
|
||||
|
||||
**On.** Always on; no configuration.
|
||||
|
||||
**Why it exists.** Nothing ever removed these sessions, and the live counter had no state filter at
|
||||
all — it counted every roster entry. That counter is the one the real spawn gate reads
|
||||
(`CompositePeerLauncher.enforceMaxLoad`), so one backend error took a seat and never gave it back.
|
||||
Only an explicit `fleet_stop` on that exact pane, or a daemon restart, freed it.
|
||||
|
||||
The harm ran in the direction that hurts. On a `maxLoad: 1` profile — `opus` and `sol` on this host —
|
||||
a single backend error put the profile out of service for good, and `fleet_spawn` answered *"at
|
||||
maxLoad: 1 live >= 1 cap; refusing spawn — no fallback to another profile"*. The lead saw a capacity
|
||||
refusal with no reason to suspect a dead seat. It also outlived the cooldown that was supposed to be
|
||||
the remedy: quarantine and cool-off both expire on their own, the dead seat did not, so the profile
|
||||
was still refusing long after the credential recovered.
|
||||
|
||||
**The gotcha: `free` and `reclaimable` count different things, and a freed seat belongs in only one
|
||||
of them.** `reclaimable` means "this member holds a seat and has no open bridge work — stop it and
|
||||
you get the seat back". A terminal session holds no seat any more, so its seat is already in `free`.
|
||||
Counting it as `reclaimable` too would report the same seat twice, and `free + reclaimable` would
|
||||
read as more capacity than `maxLoad` allows. The ticket asked for exactly that, and that half of the
|
||||
ticket was wrong. The dead session is still visible without it: its roster row carries
|
||||
`state: "backend_error"` or `"failed"`, which is what tells the lead to stop it.
|
||||
|
||||
Both places that report `reclaimable` — the per-member flag and the per-profile count, which travel
|
||||
in the same `fleet_list` response — now call one shared predicate, so they cannot drift apart. A test
|
||||
runs it over every `MemberSession.State` value, so a state added later cannot slip through
|
||||
unconsidered.
|
||||
|
||||
**What this does not do.** Terminal sessions are still never reaped. They accumulate in the roster
|
||||
until stopped or until the daemon restarts. That is now cosmetic rather than a capacity loss, and it
|
||||
is deliberate: the roster entry is the only record of what went wrong.
|
||||
|
||||
## A member's teardown no longer leaks a worktree or a branch
|
||||
|
||||
**What.** Two cleanup paths in `SessionManager` were one-sided, and both are closed. Stopping a
|
||||
member whose worktree cannot be removed now completes the stop and logs a WARN instead of throwing.
|
||||
A spawn that fails after its worktree was created now deletes the orphaned branch as well as the
|
||||
worktree.
|
||||
|
||||
**On.** Always on; no configuration.
|
||||
|
||||
**Why it exists.** Every other cleanup step in `release()` was wrapped in a try/catch — the last one,
|
||||
removing the worktree, was not. By the time it ran, the session was already out of the registry and
|
||||
the pane already stopped, so a throw there escaped `release()` with the teardown in fact complete.
|
||||
There was no retry path: a second stop on that pane is a no-op. The caller saw a failed stop for a
|
||||
session that was gone, and the directory leaked with nothing left to point at it. `git worktree
|
||||
remove --force` can throw for ordinary reasons — a stale index lock, a slow filesystem, its own 30
|
||||
second timeout — so this was not exotic.
|
||||
|
||||
The second leak is the same shape as the one fixed one layer down earlier: `GitWorktrees` already
|
||||
deleted both the worktree **and** the branch when provisioning failed *inside* `add()`. The catch
|
||||
that covers failures *after* `add()` returns — the parity overlay, the group share, the spawn itself
|
||||
— removed only the worktree. A spawn failure there is routine (a quarantined credential, a backend
|
||||
refusal), so every occurrence left an orphan `worker/<slug>-<nonce>` branch behind.
|
||||
|
||||
**The gotcha: only the failed-provisioning path deletes a branch.** A **normal** release deliberately
|
||||
keeps the branch, so a worker's committed work can still be recovered after its pane is gone. Getting
|
||||
these two backwards would destroy real work, so the tests pin the separation from both sides: the
|
||||
normal-release test asserts the branch delete is *never* called, and the failed-spawn test asserts
|
||||
the deleted branch is the exact one `add()` created.
|
||||
|
||||
## A chained second question no longer kills its own ticket
|
||||
|
||||
**What.** A member that calls `fleet_ask` twice in one turn — asks, gets an answer, then asks again —
|
||||
keeps its ticket. Before, the second question silently ended the ticket, and the lead's `fleet_poll`
|
||||
returned nothing for a member that was still working.
|
||||
|
||||
**On.** Always on; no configuration.
|
||||
|
||||
**Why it exists.** Each `fleet_ask` moves the async task to a new `turnId`. The thread answering the
|
||||
*first* question then finished its work against the *old* `turnId`. Whether that killed the ticket
|
||||
came down to which of two lines ran first — a race, not a decision. Answering now registers its
|
||||
waiter before it resolves, and drops the stale key explicitly rather than relying on the order.
|
||||
|
||||
**The gotcha: the guard that looks load-bearing is not.** The fix also added a state guard on the
|
||||
answering path, and the pull request claimed the ticket would still die without it. It would not: a
|
||||
mutation removing only that guard left the test green, because `ask()` already moves the task to the
|
||||
new `turnId` before the answering thread wakes. The guard is kept as defence in depth — it mirrors a
|
||||
sibling guard whose own comment warns that this ordering is not something to rely on — and the
|
||||
measurement is written into the code next to it, so nobody deletes it as dead code without
|
||||
re-checking the ordering, and nobody trusts it as the only protection either.
|
||||
|
||||
Reference in New Issue
Block a user