Features: #316 and #318

11-Features.md

@@ -3886,3 +3886,57 @@ deliberately fail-closed: if `lsof` ever fails for the *primary's own* connectio
 refused until its next call resolves. That costs availability and it is the right trade, because the
 old behaviour spent it on a silent escalation instead. A new DEBUG line in `LsofPeerPidLookup` now
 names the previously-silent no-match case, so a refusal that does happen can be diagnosed.
+
+## The check that authorises deleting a worktree is taken after the worker stops
+
+**What.** When a member is released, fleetd re-reads whether its worktree has uncommitted work
+immediately before the `git worktree remove --force`, with the pane already closed. If the tree is
+dirty by then, the removal is skipped and the work is also snapshotted into `refs/wip/*`.
+
+**On.** Always on. It costs one extra `git status`, and only on a release that was about to delete
+something — a release that already decided to preserve, and every `SHUTDOWN`, pay nothing.
+
+**Why it exists.** The old code read `hasUncommitted` once, while the worker was still running, and
+used that one boolean after `launcher.stop` to authorise the force-delete. A worker that committed,
+was released, and then wrote one more file during teardown lost that file. Worse, the same stale
+boolean gated the snapshot, so both defences — CB-576's preserve and CB-578 stage C's `refs/wip`
+copy — failed together. There was no third layer: pane gone, registry entry gone, directory
+force-deleted.
+
+**One thing to know for maintenance.** The snapshot deliberately did **not** move after the stop.
+`notifyReleased` fires *before* `launcher.stop` on purpose (CB-516/CB-581), so a caller blocked in a
+`fleet_ask` rendezvous fails fast instead of waiting on the herdr RPC; moving the snapshot would
+force that notification to move too or to carry a `snapshotRef` that was never computed. The late
+snapshot's ref therefore reaches the log and not the listener — a known, accepted gap. Also note
+what is **not** proven: whether herdr's `pane.close` returning means the worker process is really
+dead is decided inside herdr, whose source is not in this repo. The re-check narrows the race; it is
+not proof the race is gone. And the fail-toward-preserving rule in `dirtyImmediatelyBeforeRemoval`'s
+`catch` is the whole point — flipping it to `return false` turns this guard into a cause of the data
+loss it prevents. `releasePreservesAWorktreeWhoseLateRecheckCannotBeRead` exists because that
+mutation once passed the entire suite.
+
+## A reply that arrives while a target is being released is requeued, not stranded
+
+**What.** `AmqpReplyInbox.release` now leaves a `RELEASED` tombstone in its `held` map instead of
+removing the key. A delivery that lands during or after the release sees the tombstone and is
+nacked with requeue, so a later owner or a connection drop can still recover it.
+
+**On.** Always on, wherever the AMQP inbox is used.
+
+**Why it exists.** `basicCancel` stops new dispatches but does not flush one already handed to the
+client's consumer work pool. That delivery reached `deliverCallback`, found the key gone, and
+created a brand-new map under it — one `release` had already walked past and would never read
+again. The message then sat delivered-but-unacked until the whole inbox closed: never requeued,
+never redelivered, and nothing peeks a released target again. A worker's real report disappeared
+with no log line naming it. #298 fixed the case where the delivery was *already* held; this is the
+case where it *arrives during* the release.
+
+**One thing to know for maintenance.** The tombstone closes the window rather than narrowing it,
+and the reason is specific: `ConcurrentHashMap` serializes `compute` and `computeIfAbsent` for the
+same key against each other, so the swap and a racing insert cannot interleave. That is why the
+tombstone lives in the same map rather than in a separate "released" set — a second structure would
+have to be kept in sync, which is the shape that keeps producing defects here. `RELEASED` is one
+shared mutable map instance, so every read site must compare it by reference before touching it:
+`peek`, `ack` and `deliverCallback` all do, and `own` clears a stale tombstone with the two-argument
+`remove` so it can never delete a real map. Both halves of the fix are separately load-bearing —
+reverting either one alone fails `AmqpReplyInboxReleaseRaceTest`.