Skip to content

fix(daemon): never kill a live daemon for being slow, and survive it when one is replaced - #142

Merged
broven merged 3 commits into
mainfrom
fix/daemon-replaced-mid-call
Oct 19, 2026
Merged

broven merged 3 commits into
mainfrom
fix/daemon-replaced-mid-call

Conversation

@broven

@broven broven commented Oct 19, 2026

Copy link
Copy Markdown
Owner

What happened

On 2026-10-19 an agent-driven browser call on this machine died with:

ExecutorUnavailable: ensureExecutor failed for session '2378': CDP BrowserwrightDaemon.ensureExecutor failed:
ensureExecutor failed: RuntimeError('upstream readiness: no browserwright extension is connected to the
daemon (session 2378). Open the browser where the extension is installed and ensure it is enabled; ...')

Every word of that was wrong. The extension was connected and healthy the whole
time. A machine-global daemon that was alive but momentarily slow (heavy
machine load; it had been idle for ~53 minutes) was SIGTERMed two seconds after
three concurrent CLI calls missed two 1.5s /__ping__ probes. It was never
wedged: it was still accepting ws clients, and it shut down gracefully on the
signal.

Root cause — three defects, all confirmed by tracing every signal path

  1. A probe timeout was read as "someone else owns the port."
    daemon_lifecycle._classify folded probe kinds timeout/garbage into
    FOREIGN, and ensure() spawns a replacement serve for FOREIGN. ADR-0013
    rule 2 requires an unprovable holder to be needs-human, never an automatic
    kill or replacement target — and its "two consecutive agreeing probes" guard
    is blind here, because two timeouts agree with each other.
  2. The port reclaim treated one negative 1.0s ping as proof of staleness.
    _reclaim_stale_daemon_ports — run by every fresh serve — had no second
    probe, no agreement rule and no window. Its identity gates were strict
    (lsof + browserwright cmdline + same runtime dir); its liveness evidence
    was one ping. That is the path that killed the daemon.
  3. The reclaim was invisible by construction, which is why this took hours
    to find: it ran 39 lines before _wire_logging(), so its warnings reached
    only logging.lastResort, and ensure() spawns serve with
    stdout=DEVNULL, stderr=DEVNULL. An ensure-spawned serve could kill a daemon
    and leave no trace anywhere. (The first hypothesis — "zero reclaim lines in
    nine days of logs proves the path never ran" — was wrong for exactly this
    reason.)

And the asymmetry underneath all three: the explicit verbs refused to signal
anything they could not ping, so a genuinely wedged daemon could not be stopped
by any verb, while the implicit path killed on the weakest possible evidence.

What changed

426ac5b — silence is not proof of a stranger

  • Verdict state UNRESPONSIVE for timeout/garbage; FOREIGN keeps its
    meaning ("something answered HTTP, just not our pong"). ensure() takes no
    action for it — no spawn, no stop.
  • Escalation before concluding: _ipc.probes_until_answered re-asks inside a
    bounded 6s window, so a merely busy daemon answers and is diagnosed
    up/stale. Only continuous silence for the whole window is concluded.
  • The reclaim gates on that same escalated evidence and records its decision as
    a LIFECYCLE reclaim line (pid, endpoint, ports, per-probe kind@latency,
    reason, initiator) through _ipc.log_lifecycle's direct file append.
    _wire_logging() now runs first and is idempotent.
  • The asymmetry is flipped: stop and restart's incumbent stop may act on
    proven identity when the ping fails (_stale.unresponsive_daemon: lsof +
    browserwright cmdline + our own runtime dir), with death measured against the
    process table. The "never signal a process we cannot prove is browserwright"
    rule is unchanged.

71ae538 — survive a replaced daemon, and stop blaming the extension

  • The daemon names its own reason: DaemonShuttingDown (a marker carried through
    the JSON-RPC error envelope), raised on arrival and inside the readiness
    wait, set first thing in _graceful_shutdown. The extension text is untouched
    and still used when the extension really is what is missing.
  • The client survives a replacement: CDPTransportClosed separates "the socket
    died with no reply" from "the daemon answered with an error", and
    _call_until_daemon_returns retries only the two seams that run strictly
    before a request can reach the executor — the ensureExecutor verb and the
    /exec connect — re-resolving the endpoint on every attempt.
  • The send/recv pair still has no retry, and that is the rule this is built
    around:
    once a request frame may be on the wire, replaying it would run the
    caller's code twice.
  • ~9s of retries (13 × 0.75s, the cold-start precedent), capped by whatever the
    caller's --timeout has left (ADR-0014). A first-attempt "nothing is running"
    is still refused immediately.
  • Exhausted retries name the daemon, say the browser and extension are not
    involved, append a one-shot /__ping__ observation (ADR-0013 rule 3), and end
    with fix="retry the command".

992a238… — "no LaunchAgent" is a supported ownership mode

  • restart no longer requires a plist: without one it stops the incumbent,
    spawns a detached serve, and proves the launchd path's own post-condition
    (new pid ≠ old pid, /__status__ reports the expected version), naming five
    distinct ways to fail. install/uninstall and the launchd path are
    unchanged. (Before this, upgrade-global — which calls restart — died at
    the restart step on a plist-less machine.)
  • Durable bind config: daemon/config.py falls back to $BS_HOME/config.toml
    (resolved through the product's own home, so tests/conftest.py's BS_HOME
    redirect keeps the suite off a developer's real config). daemon_url
    deliberately still requires a named file, so an auto-discovered config can
    never re-point every client.
  • A signal-driven stop now records itself (LIFECYCLE signal sig=SIGTERM …),
    without duplicating the CLI's LIFECYCLE stop when a verb asked for it.
  • ADR-0012 gains an ownership amendment; CONTEXT.md gains daemon ownership
    and the signal lifecycle event.

Verification

  • mise run test: daemon 1034 passed / 105 skipped, skill 346 passed,
    evals 13/13, pi 5/5 (~49s). mise run lint: clean.

  • Red before green, verified three ways: reverting src/ fails every new
    assertion (31 in the first change alone); targeted mutations (drop the version
    proof, drop the exited-child observation, drop the stop-line suppression, drop
    the config fallback, drop the run_serve wiring) each fail exactly the test
    that pins them.

  • The incident itself, replayed — scripts/replay-daemon-silence-incident.py
    (hermetic; it neutralises daemon_endpoint/_spawn_detached/run_verb and
    redirects BS_HOME/TMPDIR):

    slow (3s) but alive      probe=timeout   ensure()=up             spawn=no  stop=no   (6.03s)
    accepts, never answers   probe=timeout   ensure()=unresponsive   spawn=no  stop=no   (9.01s)
    healthy                  probe=ours      ensure()=up             spawn=no  stop=no   (0.00s)
    nothing listening        probe=refused   ensure()=down           spawn=yes stop=no   (0.01s)
    

    Row 1 is the fix: the same endpoint still reads timeout to the one short
    probe the old code acted on, and ensure() no longer kills or replaces it.
    Row 4 is the guarantee that recovery still works.

What this does NOT do

  • A live daemon that is silent for the whole 6s escalated window is still
    reclaimed. The fix raises the evidence bar from "one negative 1s ping" to
    "7.5s of continuous silence", it does not make a frozen daemon immortal. Client
    survival (71ae538) is what makes the remaining case non-fatal.
  • A slow-but-alive daemon now costs that one call up to ~6s (or ~9s for
    confirm=True) instead of being killed. No other shape pays: a healthy daemon
    is 1–50ms, a dead one 0ms.
  • restart's proof window is still timeout=5s (unchanged from the launchd
    path), so a very slow cold start can report a failed restart while the new
    daemon is still coming up.
  • No release is cut here: facade_host durability and plist-less restart only
    reach a machine's global install once this is released.

Follow-ups (not in this PR)

  • On the affected machine: drop the LaunchAgent once a release contains this —
    write facade_host into $BS_HOME/config.toml, restart (now plist-less),
    verify both the loopback and the tailnet endpoint, then uninstall.

A live-but-slow daemon was SIGTERMed two seconds after it missed two 1.5s
/__ping__ probes (2026-10-19, this machine). Three concurrent CLI calls
timed out, `_classify` folded probe kind `timeout` into FOREIGN, `ensure()`
spawned a `serve`, and that serve's port reclaim treated one negative 1.0s
ping as proof the incumbent was stale. The daemon was never wedged: it was
still accepting ws clients and it shut down gracefully on the signal.

- verdict state UNRESPONSIVE: `timeout`/`garbage` are no longer FOREIGN.
  `ensure()` takes no action for it (same early return as UNDECIDED), so
  silence is never grounds to spawn or to signal. FOREIGN keeps its meaning:
  something answered HTTP, just not our pong.
- escalation before concluding: `_ipc.probes_until_answered` re-asks inside
  a bounded 6s window, so a merely busy daemon answers and is diagnosed
  up/stale. Only silence for the whole window is concluded. Cost is paid
  only by the pathological shape: refused->DOWN 0.00s, healthy 1-50ms,
  silent holder 7.5s single / 9.0s confirm.
- the reclaim gates on that same escalated evidence, and records its
  decision as a `LIFECYCLE reclaim` line (pid, endpoint, ports, per-probe
  kind@latency, reason, initiator) through `_ipc.log_lifecycle`'s direct
  file append. It used to leave no trace at all: it ran 39 lines before
  `_wire_logging()`, and ensure() spawns serve with stdout/stderr=devnull.
  `_wire_logging()` now runs first and is idempotent.
- the explicit paths (`stop`, and `restart`'s incumbent stop) may now act
  on proven identity when the ping fails: `_stale.unresponsive_daemon`
  requires lsof + a browserwright cmdline + our own runtime dir, and death
  is then measured against the process table. Before this, a wedged daemon
  could not be stopped by any verb while the implicit path killed on the
  weakest possible evidence.

Tests: 31 assertions fail against the pre-change source and pass after.
mise run test: 1012 passed / 105 skipped (daemon), 338 (skill), 5 (pi).
mise run lint: clean.
Three agent calls died on 2026-10-19 with "no browserwright extension is
connected ... open chrome://extensions" while the extension was connected and
healthy: the daemon was being replaced under the call and had pulled its own
relay away first. Every word of that error blamed the one layer that was fine.

- the daemon now names its own reason: `DaemonShuttingDown` (a marker carried
  through the JSON-RPC error envelope) is raised from `ensure_session_drivable`
  both on arrival and inside the readiness wait, and the flag is set first thing
  in `_graceful_shutdown`. The extension text is untouched and still used when
  the extension really is what is missing.
- the client survives the replacement: `CDPTransportClosed` separates "the
  socket died with no reply" from "the daemon answered with an error", and
  `_call_until_daemon_returns` retries only the two seams that run strictly
  before a request can reach the executor — the `ensureExecutor` verb and the
  `/exec` connect. Each attempt re-resolves the endpoint (the cached ws url is
  invalidated), so the retry dials the daemon that answers now.
- the send/recv pair still has NO retry, and that is the rule the change is
  built around: once a request frame may be on the wire, replaying it would run
  the caller's code a second time.
- ~9s of retries (13 x 0.75s, the cold-start precedent), capped by whatever the
  caller's `--timeout` has left (ADR-0014). A first-attempt "nothing is running"
  is still refused immediately; retrying it would only make every no-daemon call
  ~9s slower to say the same thing.
- exhausted retries name the daemon, state that the browser and extension are
  not involved, append a one-shot `/__ping__` observation (ADR-0013 rule 3), and
  end with `fix="retry the command"`.

Tests: 2 new daemon tests quote the exact incident message and fail at HEAD; 4
of the 8 new client tests fail at HEAD. Existing tests changed only where a
signature or a budget moved; `test_adr0013_diagnose_before_blame.py` now also
scans `daemon/errors.py`, which widens the banned-word net.
mise run test: 1012 passed / 107 skipped (daemon), 346 (skill), 5 (pi).
mise run lint: clean.
A readable, repeatable demonstration that the 2026-10-19 failure no longer
reproduces, and the cheapest way for a reviewer to see the new rule work:
same four endpoint shapes, one short probe each (the only evidence the old code
had) against what `ensure()` concludes.

    slow (3s) but alive      probe=timeout   ensure()=up             spawn=no  stop=no
    accepts, never answers   probe=timeout   ensure()=unresponsive   spawn=no  stop=no
    healthy                  probe=ours      ensure()=up             spawn=no  stop=no
    nothing listening        probe=refused   ensure()=down           spawn=yes stop=no

Hermetic by construction: it sets `BS_HOME`/`TMPDIR` aside before importing the
package and replaces `daemon_endpoint`, `_spawn_detached` and `run_verb` with
fakes, so it cannot touch the machine-global daemon. That is the lesson from
this incident's own investigation: an earlier version of this replay ran a dev
checkout (version 0.0.0) against the default endpoint, which the lifecycle
correctly read as a stale daemon and replaced — killing a live one.
@broven
broven merged commit f5d6c5b into main Oct 19, 2026
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant