Context
DaemonClient.connect() has untested error paths. Current behavior:
- Daemon binary missing →
Process.run() throws, error bubbles up (probably fine).
- Daemon binary present but crashes immediately on spawn →
spawnDaemon() returns, then we poll the socket for 10s, then throw DaemonClientError.startupTimedOut. The user sees the timeout but not the crash reason.
- Daemon binary present but a stale
serve.sock file (pointing to a crashed daemon) is sitting at the path → handled by DaemonProbe.canConnect() returning false (connect fails), and DaemonListener.start() unlinks stale socket before bind. Probably fine but not tested.
- Concurrent connect() calls from two CLI processes → the second tries to spawn, our PR 1 race fix refuses the spawn, the second client's daemon process exits nonzero, the connect() keeps polling until timeout. User sees an unhelpful timeout error when a daemon IS running.
Proposed fixes
-
Surface spawn-process exit status: in waitForSocket, also check whether the spawned child has exited. If it has, throw DaemonClientError.daemonStartupFailed(exitCode:) with the exit code, not a timeout.
-
Faster race resolution: before spawning, do a tight re-check (DaemonProbe.canConnect() with short timeout) in case another client just started the daemon. This avoids the redundant spawn.
-
Tests (hard to make deterministic, but worth doing):
- Corrupt binary at path so
Process.run() fails → expect immediate error.
- Point
DaemonPaths overrides to a bad location so daemon can't create files → expect startup-failed error.
Context links
Context
DaemonClient.connect()has untested error paths. Current behavior:Process.run()throws, error bubbles up (probably fine).spawnDaemon()returns, then we poll the socket for 10s, then throwDaemonClientError.startupTimedOut. The user sees the timeout but not the crash reason.serve.sockfile (pointing to a crashed daemon) is sitting at the path → handled byDaemonProbe.canConnect()returning false (connect fails), andDaemonListener.start()unlinks stale socket before bind. Probably fine but not tested.Proposed fixes
Surface spawn-process exit status: in
waitForSocket, also check whether the spawned child has exited. If it has, throwDaemonClientError.daemonStartupFailed(exitCode:)with the exit code, not a timeout.Faster race resolution: before spawning, do a tight re-check (
DaemonProbe.canConnect()with short timeout) in case another client just started the daemon. This avoids the redundant spawn.Tests (hard to make deterministic, but worth doing):
Process.run()fails → expect immediate error.DaemonPathsoverrides to a bad location so daemon can't create files → expect startup-failed error.Context links