Robust daemon lifecycle management and safety improvements (#118)

## Summary

This PR comprehensively improves daemon lifecycle management, addressing
multiple issues around daemon startup, shutdown, and process safety.

### Core Changes
- **Fix update command creating multiple daemons** - Use `stopDaemon()`
and `killAllDaemons()` instead of HTTP POST to ensure old daemon is
properly killed before starting new one
- **Highlander mode** - Prevent multiple daemons from running
simultaneously by detecting and cleaning up existing instances at
startup
- **Per-PID runtime files** - Use `daemon.$PID.json` pattern instead of
single `daemon.json` to properly track multiple daemon instances and
enable reliable cleanup

### Safety Improvements
- **PID reuse protection** - Tri-state process identity checking
(`processIsRoborev`, `processNotRoborev`, `processUnknown`) prevents
killing unrelated processes if a PID is reused
- **Conservative unknown handling** - When process identity can't be
determined (permissions, missing tools), keep runtime file to avoid
orphaning running daemons
- **Loopback validation** - Strict address validation prevents SSRF via
malicious runtime files; only allows HTTP calls to 127.x.x.x, ::1, and
localhost
- **Invalid runtime cleanup** - Remove runtime files with invalid data
(pid <= 0, empty addr) during listing

### Reliability Improvements
- **IsDaemonAlive retry logic** - 2 attempts with 1s timeout to avoid
false negatives on slow or transiently failing daemons
- **Windows locale-independent detection** - Use `tasklist /FO CSV` and
`wmic` for process detection instead of parsing localized strings
- **PowerShell fallback** - Use `Get-CimInstance` on Windows systems
without `wmic` (common on newer Win11)
- **UTF-16 BOM handling** - Properly handle UTF-16LE/BE encoded output
from Windows tools

### TUI Improvements
- **Daemon reconnection** - TUI sessions now automatically reconnect if
daemon restarts on a different port. After 3 consecutive connection
failures, the TUI re-reads runtime files to find the new daemon address.

## Test plan
- [x] Run `roborev update --force` and verify only one daemon process
running
- [x] All daemon tests pass on macOS
- [x] TUI reconnection tests pass
- [ ] CI passes on Linux and Windows
- [ ] `pgrep -fl "roborev daemon"` shows only one process after update

🤖 Generated with [Claude Code](https://claude.com/claude-code)

---------

Co-authored-by: Claude Opus 4.5 <noreply@anthropic.com>

Author: wesm

cmd/roborev/main.go

@@ -5,7 +5,6 @@ import (
 	"bytes"
 	"context"
 	"encoding/json"
-	"errors"
 	"fmt"
 	"io"
 	"log"
@@ -82,7 +81,7 @@ func main() {
 
 // getDaemonAddr returns the daemon address from runtime file or default
 func getDaemonAddr() string {
-	if info, err := daemon.ReadRuntime(); err == nil {
+	if info, err := daemon.GetAnyRunningDaemon(); err == nil {
 		return fmt.Sprintf("http://%s", info.Addr)
 	}
 	return serverAddr
@@ -93,8 +92,8 @@ func getDaemonAddr() string {
 func ensureDaemon() error {
 	client := &http.Client{Timeout: 500 * time.Millisecond}
 
-	// First check runtime file for daemon address
-	if info, err := daemon.ReadRuntime(); err == nil {
+	// First check runtime files for any running daemon
+	if info, err := daemon.GetAnyRunningDaemon(); err == nil {
 		addr := fmt.Sprintf("http://%s/api/status", info.Addr)
 		resp, err := client.Get(addr)
 		if err == nil {
@@ -165,7 +164,7 @@ func startDaemon() error {
 	client := &http.Client{Timeout: 500 * time.Millisecond}
 	for i := 0; i < 30; i++ {
 		time.Sleep(100 * time.Millisecond)
-		if info, err := daemon.ReadRuntime(); err == nil {
+		if info, err := daemon.GetAnyRunningDaemon(); err == nil {
 			addr := fmt.Sprintf("http://%s", info.Addr)
 			resp, err := client.Get(addr + "/api/status")
 			if err == nil {
@@ -182,59 +181,41 @@ func startDaemon() error {
 // ErrDaemonNotRunning indicates no daemon runtime file was found
 var ErrDaemonNotRunning = fmt.Errorf("daemon not running (no runtime file found)")
 
-// stopDaemon stops the running daemon using PID from daemon.json.
-// Returns ErrDaemonNotRunning if the runtime file is missing.
-// For corrupted/malformed files (JSON parse errors), removes them and returns ErrDaemonNotRunning.
-// Propagates permission/IO errors to the caller.
+// stopDaemon stops any running daemons.
+// Returns ErrDaemonNotRunning if no daemon runtime files are found.
 func stopDaemon() error {
-	info, err := daemon.ReadRuntime()
+	runtimes, err := daemon.ListAllRuntimes()
 	if err != nil {
+		// Check if it's just a "not exist" type error
 		if os.IsNotExist(err) {
 			return ErrDaemonNotRunning
 		}
-		// Check if it's a JSON parse error (corrupted file)
-		// This includes SyntaxError, UnmarshalTypeError, and UnexpectedEOF (truncated files)
-		var syntaxErr *json.SyntaxError
-		var typeErr *json.UnmarshalTypeError
-		if errors.As(err, &syntaxErr) || errors.As(err, &typeErr) || errors.Is(err, io.ErrUnexpectedEOF) {
-			// Corrupted file - remove it and treat as not running
-			daemon.RemoveRuntime()
-			return ErrDaemonNotRunning
-		}
-		// Permission/IO error - propagate to caller
-		return fmt.Errorf("failed to read daemon runtime file: %w", err)
+		// Propagate other errors (permission, IO, etc.)
+		return fmt.Errorf("failed to list daemon runtimes: %w", err)
 	}
-	if info.PID <= 0 {
-		daemon.RemoveRuntime() // Clean up invalid runtime file
+	if len(runtimes) == 0 {
 		return ErrDaemonNotRunning
 	}
 
-	// Kill by specific PID (works regardless of process name)
-	if runtime.GOOS == "windows" {
-		exec.Command("taskkill", "/PID", fmt.Sprintf("%d", info.PID), "/F").Run()
-	} else {
-		// Send SIGTERM first for graceful shutdown
-		exec.Command("kill", "-TERM", fmt.Sprintf("%d", info.PID)).Run()
-		time.Sleep(500 * time.Millisecond)
-		// Then SIGKILL to ensure it's dead
-		exec.Command("kill", "-KILL", fmt.Sprintf("%d", info.PID)).Run()
-	}
-	// Clean up runtime file
-	daemon.RemoveRuntime()
-	time.Sleep(500 * time.Millisecond)
-	return nil
+	// Kill all found daemons, track failures
+	var lastErr error
+	for _, info := range runtimes {
+		if !daemon.KillDaemon(info) {
+			lastErr = fmt.Errorf("failed to kill daemon (pid %d)", info.PID)
+		}
+	}
+
+	return lastErr
 }
 
 // killAllDaemons kills any roborev daemon processes that might be running
 // This handles orphaned processes from old binaries or crashed restarts
 func killAllDaemons() {
 	if runtime.GOOS == "windows" {
-		// On Windows, use wmic to kill roborev.exe processes except our own PID
-		// taskkill /IM would kill the CLI itself, so we filter by PID
-		myPID := os.Getpid()
-		// Use wmic to find and kill daemon processes, excluding our PID
+		// On Windows, use wmic to find daemon processes by command line
+		// and kill only those running "daemon run"
 		exec.Command("wmic", "process", "where",
-			fmt.Sprintf("name='roborev.exe' and processid!=%d", myPID),
+			"commandline like '%roborev%daemon%run%'",
 			"call", "terminate").Run()
 	} else {
 		// On Unix, use pkill to kill all roborev daemon processes
@@ -2038,18 +2019,13 @@ official release over a dev build.`,
 				}
 			}
 
-			// Restart daemon if running
-			if daemonInfo, err := daemon.ReadRuntime(); err == nil && daemonInfo != nil {
+			// Restart daemon if any are running
+			if runtimes, err := daemon.ListAllRuntimes(); err == nil && len(runtimes) > 0 {
 				fmt.Print("Restarting daemon... ")
-				// Stop old daemon with timeout
-				stopURL := fmt.Sprintf("http://%s/api/shutdown", daemonInfo.Addr)
-				client := &http.Client{Timeout: 5 * time.Second}
-				if resp, err := client.Post(stopURL, "application/json", nil); err != nil {
-					fmt.Printf("warning: failed to stop daemon: %v\n", err)
-				} else {
-					resp.Body.Close()
-				}
-				time.Sleep(500 * time.Millisecond)
+				// Stop all running daemons
+				_ = stopDaemon()
+				// Kill any orphaned daemon processes
+				killAllDaemons()
 
 				// Start new daemon using "roborev daemon run"
 				newBinary := filepath.Join(binDir, "roborev")

cmd/roborev/main_test.go

@@ -8,10 +8,8 @@ import (
 	"bytes"
 	"context"
 	"encoding/json"
-	"errors"
 	"fmt"
 	"io"
-	"io/fs"
 	"net/http"
 	"net/http/httptest"
 	"os"
@@ -39,7 +37,22 @@ import (
 func setupMockDaemon(t *testing.T, handler http.Handler) (*httptest.Server, func()) {
 	t.Helper()
 
-	ts := httptest.NewServer(handler)
+	// Wrap handler to handle /api/status requests.
+	// IsDaemonAlive calls /api/status to verify daemon is alive.
+	// ensureDaemon also calls /api/status to check the daemon version.
+	// We need to return a proper response with version info to prevent
+	// ensureDaemon from trying to restart the daemon (which would kill the test).
+	wrappedHandler := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		if r.URL.Path == "/api/status" && r.Method == http.MethodGet {
+			json.NewEncoder(w).Encode(map[string]interface{}{
+				"version": version.Version,
+			})
+			return
+		}
+		handler.ServeHTTP(w, r)
+	})
+
+	ts := httptest.NewServer(wrappedHandler)
 
 	// Use ROBOREV_DATA_DIR to override data directory (works cross-platform)
 	// On Windows, HOME doesn't work since os.UserHomeDir() uses USERPROFILE
@@ -1531,16 +1544,20 @@ func TestDaemonStopInvalidPID(t *testing.T) {
 		}
 	}()
 
-	// Create daemon.json with PID 0
-	daemonInfo := daemon.RuntimeInfo{PID: 0, Addr: "127.0.0.1:7373"}
+	// Create daemon.json with PID 0 and an address on a port that's definitely not in use
+	// Port 59999 is unlikely to be in use and will get connection refused quickly
+	daemonInfo := daemon.RuntimeInfo{PID: 0, Addr: "127.0.0.1:59999"}
 	data, _ := json.Marshal(daemonInfo)
 	if err := os.WriteFile(filepath.Join(tmpDir, "daemon.json"), data, 0644); err != nil {
 		t.Fatalf("write daemon.json: %v", err)
 	}
 
+	// ListAllRuntimes validates files and removes invalid ones (pid <= 0),
+	// so it returns an empty list, and stopDaemon returns ErrDaemonNotRunning.
+	// The key is that the invalid file gets cleaned up.
 	err := stopDaemon()
 	if err != ErrDaemonNotRunning {
-		t.Errorf("expected ErrDaemonNotRunning for PID 0, got %v", err)
+		t.Errorf("expected ErrDaemonNotRunning (invalid file removed during listing), got %v", err)
 	}
 
 	// Verify daemon.json was cleaned up
@@ -1611,8 +1628,11 @@ func TestDaemonStopTruncatedFile(t *testing.T) {
 	}
 }
 
-// TestDaemonStopPermissionError verifies stopDaemon propagates permission errors
-func TestDaemonStopPermissionError(t *testing.T) {
+// TestDaemonStopUnreadableFileSkipped verifies stopDaemon skips unreadable files
+// With the new per-PID runtime file pattern, ListAllRuntimes continues scanning
+// even when some files are unreadable. This allows daemon discovery to work even
+// if some runtime files are temporarily inaccessible.
+func TestDaemonStopUnreadableFileSkipped(t *testing.T) {
 	if os.Geteuid() == 0 {
 		t.Skip("skipping permission test when running as root")
 	}
@@ -1652,16 +1672,10 @@ func TestDaemonStopPermissionError(t *testing.T) {
 	}
 
 	err := stopDaemon()
-	// Should propagate the permission error, not ErrDaemonNotRunning
-	if err == ErrDaemonNotRunning {
-		t.Error("expected permission error to be propagated, got ErrDaemonNotRunning")
-	}
-	if err == nil {
-		t.Error("expected error for permission denied, got nil")
-	}
-	// Check that the underlying error is a permission error (using errors.Is to unwrap)
-	if !errors.Is(err, fs.ErrPermission) {
-		t.Errorf("expected permission error, got: %v", err)
+	// With the new behavior, unreadable files are skipped during ListAllRuntimes.
+	// Since no readable daemon files exist, stopDaemon returns ErrDaemonNotRunning.
+	if err != ErrDaemonNotRunning {
+		t.Errorf("expected ErrDaemonNotRunning (unreadable file skipped), got: %v", err)
 	}
 }