From 3e46ca9ec7548cbfa4ab5cd5a7c5a53cd8418dfa Mon Sep 17 00:00:00 2001 From: Brennan Benson <79079362+brennanb2025@users.noreply.github.com> Date: Fri, 28 Aug 2026 07:46:36 -0700 Subject: [PATCH] docs(daemon): record publish scratch as the one swept namespace MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The directory's ownership rules ban sweepers absolutely ("No actor removes a name it did not create"), scoped in prose to the socket path but read by every agent working anywhere in src/main/daemon. This PR adds a sweeper without amending them, so review keeps re-raising "remove the reaper" — correctly, from the doc as written. Scope the traps to every canonical name, and record the exception with the two properties it actually rests on: publish scratch is never an actor's only copy, and it is deleted only on an ESRCH-proven-dead owner. Sweeping the bind name or .swap-/.hold- claims stays banned, since those can be an only copy. --- src/main/daemon/AGENTS.md | 38 +++++++++++++++++++++++++++++++++++--- 1 file changed, 35 insertions(+), 3 deletions(-) diff --git a/src/main/daemon/AGENTS.md b/src/main/daemon/AGENTS.md index 24d2d91d008..0586fe6f8aa 100644 --- a/src/main/daemon/AGENTS.md +++ b/src/main/daemon/AGENTS.md @@ -23,6 +23,9 @@ hands → probe once more → `rename` in one syscall → verify we kept it. ## Traps That Already Cost Us +These cover every canonical name another actor can act on — the socket endpoint above and the +`daemon-vN.pid` record — not just the socket path. + - **Never collapse "can't tell" into "dead."** Only `connected` means occupied; only `refused`/`missing` prove death. A timeout or `EPERM` proves nothing and must decline — treating it as death deletes an endpoint still serving every terminal on the host. @@ -36,9 +39,12 @@ hands → probe once more → `rename` in one syscall → verify we kept it. filesystems without a birth time report the epoch, and its granularity is often coarser than the events it must separate. Three attempts to patch around this produced three more defects; inode recycling is now settled by asking whether anything is _serving_. -- **Do not add a sweeper.** Deciding whether someone else's leftover is safe to delete is the - question this design retired; the last one produced five defects, including deleting a live - listener's only pathname. Every actor removes its own scratch name on each non-crash path. +- **Do not add a sweeper over names that can be an actor's only copy.** Deciding whether someone + else's leftover is safe to delete is the question this design retired; the last one produced + five defects, including deleting a live listener's only pathname. Every actor removes its own + scratch name on each non-crash path. This still bans a sweeper over the bind name and over + `.swap-`/`.hold-` claims, which briefly hold the only copy of a live daemon's endpoint, token, + or PID record. See the one documented exception below before adding another. - **Scratch namespaces must stay out of released builds' patterns.** Shipped versions sweep `^\.b[0-9a-f]{10}$` on age alone with no liveness check, which is why the bind name is `.p`. Deleting our sweeper does not un-ship theirs. @@ -48,3 +54,29 @@ hands → probe once more → `rename` in one syscall → verify we kept it. **Residual risk.** The final probe and the `rename` are two syscalls, and POSIX has no rename-if-target-is-inode-X. The harm is separately unreachable: a daemon never creates a session on an endpoint it no longer holds (`daemon-server.ts`), and it drains rather than serving on. + +## The One Namespace We Do Sweep: Publish Scratch + +`reapOrphanedDaemonPidPublishClaims` (`daemon-pid-publish-claim-reap.ts`) deletes +`daemon-vN.pid.publish--` names it did not create, at the launch funnel +(`DaemonSpawner.ensureRunning`). That is a deliberate exception to the rule above, not an +oversight, and it holds only because both premises behind the ban fail here: + +- **It is never anyone's only copy.** Before `linkSync` the claim is unpublished bytes no reader + looks for; after it, the canonical name holds the same inode. Losing one costs at most the + atomic path for that publish, which then degrades to the exclusive direct write. A bind name or + a `.swap-`/`.hold-` claim, by contrast, can be the only name a live daemon's endpoint or record + has — which is why sweeping those stays banned. +- **It deletes only on proven death, never on age.** The owner pid is read out of the name and + must return `ESRCH`. A live pid, this process, `EPERM`, and any other errno all preserve the + claim, so pid recycling only keeps junk around longer — the safe direction. This is the fence + the exception rests on: weaken it and the exception is gone with it. + +**Why not just let them leak:** a publisher killed between the write and the `finally` never runs +its own cleanup, and nothing else ever would. The reaper matches retired protocol versions on +purpose, because a `daemon-vN.pid` for a version no longer current is exactly what nothing heals. + +**Still in force here:** the name stays outside released builds' sweep patterns (pinned in +`daemon-spawner.test.ts`, "keeps every scratch namespace outside the released sweeper pattern"), +and the reaper must keep ignoring `.swap-`/`.hold-` (pinned in +`daemon-pid-publish-claim-reap.test.ts`, "touches nothing but publish scratch").