From f15a5034b969fc882c8dc245200517b2e9ebffb8 Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Wed, 12 Aug 2026 15:18:51 -0700 Subject: [PATCH] docs(daemon): cost the deferred guard honestly, and say why it is unreachable MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two corrections to the note, both of which change what it tells the next person. The reason the guard's case cannot arise at 34s is structural, not a lucky margin: an `empty` verdict means the daemon answered, so it resolved fast by construction, and proven-dead means nothing is listening, so the probe and the ladder both short-circuit. The path that actually spends the budget is the wedge that never answers — and that one now ends in a hold, paying neither the ladder nor the fork. Long path and expensive tail are disjoint. Raising the budget is precisely what re-couples them, by extending how late an `empty` may arrive. And the guard was costed as a signature change through DaemonSpawner, which is wrong. createOutOfProcessLauncher is a factory called where `signal` is already in scope; a third parameter closed over there leaves the launcher's call signature untouched. Overstating the price invites skipping the guard rather than paying it. Also recorded: two terms this budget does not bound at all — the healthy branch, which never consults the clock and still ends in a cleanup and a fork, and the unbounded daemon-host copy on packaged Windows. --- src/main/daemon/AGENTS.md | 29 +++++++++++++++++++++++++++-- 1 file changed, 27 insertions(+), 2 deletions(-) diff --git a/src/main/daemon/AGENTS.md b/src/main/daemon/AGENTS.md index 9d06186e667..eb868288391 100644 --- a/src/main/daemon/AGENTS.md +++ b/src/main/daemon/AGENTS.md @@ -68,8 +68,33 @@ hands → probe once more → `rename` in one syscall → verify we kept it. `preserveDaemon()`, which opens a non-shared 20s handshake and could overrun the deadline it is meant to respect. - Note the launcher closure does not receive the startup abort signal, so the guard cannot - simply read `signal.aborted` without threading it through `DaemonSpawner`. + Why it is unreachable at 34s is structure, not margin, and the distinction is the point: the + hold decoupled long classification from the replace path. A verdict of `empty` means the + daemon *answered*, so it resolved fast by construction; `unknown` + proven-dead means nothing + is listening, so the probe settles in ~500ms and the ladder short-circuits on ESRCH. The path + that actually consumes the budget — a wedge that never answers — now ends in a hold, which + pays neither the ladder nor the fork. The long path and the expensive tail are disjoint. + Raising the budget is what re-couples them, by extending how late an `empty` may legally + arrive (~22s in at 34s; ~32s in at 44s). The raise creates the case; it does not merely + expose it. + + Costing the guard honestly: the launcher closure does not receive the startup abort signal, + but `createOutOfProcessLauncher` is a factory called from inside `initDaemonPtyProvider`, + where `signal` is in scope. A third factory parameter closed over there leaves + `DaemonLauncher`'s call signature — all `DaemonSpawner` knows about — unchanged. One + parameter, not a spawner change. Record alongside it that a closed-over startup signal is + meaningful only for the startup launch: `runRestartDaemon` reuses the same spawner and the + `respawn` closure re-enters the same launcher, and both would read a signal that never + aborts, because `servicesSettled` clears the fail-open timer once init succeeds. That is + correct — later restarts are not under the startup gate — but it reads like a bug without + the sentence. + + Two things erode that margin rather than consume it, and neither is bounded by this budget: + the `health === 'healthy'` branch never consults `classificationRemainingMs()` at all + (`resolveOccupancyOverIpc` passes no `budgetMs`, so it takes the 19s default) and also ends + in a cleanup and a fork; and packaged Windows follows the fork with a daemon-host directory + copy of unbounded size. Both stay under today only because reaching them requires a verdict + that arrives early. **Do not try to fix this by tuning the classification budget.** Ten review rounds each found a different timing band where a bounded classification kills a session an unbounded one keeps.