fix(daemon): graceful watchdog exit (no orphaned apps) + built-in log size cap

CHANGELOG.md

@@ -10,6 +10,47 @@ Detailed per-release notes are on the
 ## [Unreleased]
 
 ### Added
+- **The daemon caps its own log file.** launchd never rotates the daemon's
+  `StandardOutPath`/`StandardErrorPath` (`~/.pilot/daemon.log`), which grew
+  without bound — 22 MB on one laptop. When stderr is a regular file the
+  daemon now checks it every minute and, past `-log-max-size` MB (default 50;
+  `0` disables), copy-truncates it into gzipped generations
+  `daemon.log.pilot.1.gz` … `daemon.log.pilot.N.gz` (`-log-max-backups`,
+  default 3). Truncation is safe for every writer — the file is opened
+  append-only and shared by child processes. Both flags can also be set in
+  `~/.pilot/config.json` (`log_max_size`, `log_max_backups`). No-op when the
+  output goes to journald, a pipe or a terminal.
+  - By default only a log Pilot set up is rotated: one inside `~/.pilot`,
+    where install.sh's launchd job and `pilotctl daemon start` put it (also
+    `$PILOT_HOME/.pilot`), and the Homebrew service's
+    `$(brew --prefix)/var/log/pilot-daemon.log` when the daemon is the one
+    `brew install pilotprotocol` installed (`brew services`, launchd or
+    systemd). If you send the daemon's output somewhere else, rotation stays
+    off unless you set `-log-max-size` explicitly, on the command line or in
+    `config.json`, so your own rotation (logrotate, newsyslog) keeps working
+    as before.
+  - A rotation interrupted by a crash or kill is finished by the next
+    rotation of that log. In `~/.pilot` this includes a rotation left under
+    the `pilot-<pid>.log` name of a daemon that `pilotctl daemon start`
+    launched earlier. Only those names are finished this way, so another
+    program's `<name>.pilot.1` is never taken for one. A rotation holds its
+    uncompressed copy (`<log>.pilot.1`) locked with `flock` until it is
+    compressed, so a copy that another running daemon is still working on
+    is left alone.
+  - If the log cannot be truncated (an append-only file, or a filesystem
+    that refuses it), no backup is lost. The round's copy is removed, the
+    generations are put back, and the next attempt waits twice as long as
+    the last, up to about an hour.
+  - It never touches files it did not create. The `.pilot` infix keeps its
+    backups apart from logrotate's and newsyslog's names (`daemon.log.1`,
+    `daemon.log.2.gz`, …). It skips a symlink or another user's file at one
+    of its own names, and creates its files with `O_EXCL|O_NOFOLLOW`. If
+    other users can create files in the log's directory, it only truncates
+    and keeps no backups. That is the case when the directory is writable
+    by others, is owned by another user, or is writable by a group other
+    than the user's own private group. A `~/.pilot` that is group-writable
+    only because of the umask 002 on Ubuntu, Debian or Fedora (the group
+    is the user's own, with no other members) keeps its backups.
 - **Opt out of automatic app-store updates with `PILOT_APP_UPDATE_OPT_OUT`.** The
   `pilot-updater` keeps installed apps current by periodically running
   `pilotctl appstore upgrade --all`. Set `PILOT_APP_UPDATE_OPT_OUT=true` in the
@@ -20,6 +61,17 @@ Detailed per-release notes are on the
   `PILOT_UPDATER_NO_APP_UPGRADE` as a back-compat alias.
 
 ### Fixed
+- **Watchdog restarts no longer orphan app-store apps.** When the inbound-path
+  watchdog gave up on a wedged transport it called `os.Exit(86)` directly,
+  skipping the graceful shutdown that stops plugins — so every installed app
+  (each in its own process group) was left running on every respawn. One
+  laptop accumulated 94 copies of each of 12 apps (~2.8 GB RSS) in a week.
+  The watchdog now asks the daemon's shutdown loop to stop the daemon and its
+  plugins, then exits with code 86 as before so launchd/systemd still respawn
+  it; a 15 s hard deadline forces the exit if the teardown hangs. Startup
+  failures after plugins have started now stop them before exiting, too.
+  (Complements the app-store's orphan reaping at spawn,
+  pilot-protocol/app-store#38.)
 - **The `pilotctl skills disable` opt-out now survives updates and explicit
   reconciles.** A forced reconcile — `pilotctl skills check`, `pilotctl update`,
   or an installer re-run — bypassed the disabled flag and re-injected skills a

cmd/daemon/logrotation.go

@@ -0,0 +1,97 @@
+// SPDX-License-Identifier: AGPL-3.0-or-later
+
+package main
+
+import (
+	"flag"
+	"os"
+	"path/filepath"
+	"strings"
+)
+
+// pilotDirs returns the directories the daemon's log is in when Pilot
+// itself set it up, which is where -log-max-size applies by default:
+// launchd's StandardErrorPath (~/.pilot/daemon.log, from install.sh) and
+// `pilotctl daemon start` (~/.pilot/pilot-<pid>.log, under $PILOT_HOME
+// when that is set). pilotLogFiles adds the Homebrew service's log. A log
+// an operator redirected anywhere else may already be rotated by other
+// means, so it is left alone unless -log-max-size is set explicitly
+// (flagExplicit).
+func pilotDirs() []string {
+	var dirs []string
+	if home, err := os.UserHomeDir(); err == nil {
+		dirs = append(dirs, filepath.Join(home, ".pilot"))
+	}
+	if h := os.Getenv("PILOT_HOME"); h != "" {
+		dirs = append(dirs, filepath.Join(h, ".pilot"))
+	}
+	return dirs
+}
+
+// pilotLogFiles returns the logs outside pilotDirs that Pilot itself set
+// up, where -log-max-size also applies by default: the `brew services`
+// log when this daemon is the one Pilot's Homebrew formula installed.
+func pilotLogFiles() []string {
+	exe, err := os.Executable()
+	if err != nil {
+		return nil
+	}
+	if log, ok := homebrewServiceLog(exe); ok {
+		return []string{log}
+	}
+	return nil
+}
+
+// homebrewServiceLog returns the file `brew services` sends the daemon's
+// stdout and stderr to when exe is the pilot-daemon of Pilot's Homebrew
+// formula (TeoSlayer/homebrew-pilot, Formula/pilotprotocol.rb):
+// <prefix>/Cellar/pilotprotocol/<version>/bin/pilot-daemon, which the
+// service runs through the <prefix>/opt/pilotprotocol link. The formula's
+// service block sets log_path and error_log_path to
+// var/"log/pilot-daemon.log", which launchd (StandardOutPath,
+// StandardErrorPath) or systemd (StandardOutput=append:) opens and
+// nothing rotates. Only that one file is in scope, not the rest of
+// <prefix>/var/log, which belongs to other formulae. Keep the names here
+// in step with the formula.
+func homebrewServiceLog(exe string) (string, bool) {
+	resolved, err := filepath.EvalSymlinks(exe)
+	if err != nil {
+		return "", false
+	}
+	bin := filepath.Dir(resolved)
+	formula := filepath.Dir(filepath.Dir(bin))
+	cellar := filepath.Dir(formula)
+	if filepath.Base(resolved) != "pilot-daemon" ||
+		filepath.Base(bin) != "bin" ||
+		filepath.Base(formula) != "pilotprotocol" ||
+		filepath.Base(cellar) != "Cellar" {
+		return "", false
+	}
+	return filepath.Join(filepath.Dir(cellar), "var", "log", "pilot-daemon.log"), true
+}
+
+// flagExplicit reports whether flag name was set on the command line or
+// in the config file. config.ApplyToFlags sets config values without
+// marking the flag as set, so the config map is checked directly, the
+// way ApplyToFlags reads it: the hyphenated key, else the underscored
+// one, and only the value types it applies.
+func flagExplicit(name string, fileConfig map[string]interface{}) bool {
+	explicit := false
+	flag.Visit(func(f *flag.Flag) {
+		if f.Name == name {
+			explicit = true
+		}
+	})
+	if explicit {
+		return true
+	}
+	val, ok := fileConfig[name]
+	if !ok {
+		val = fileConfig[strings.ReplaceAll(name, "-", "_")]
+	}
+	switch val.(type) {
+	case string, float64, bool:
+		return true
+	}
+	return false
+}

cmd/daemon/logrotation_test.go

@@ -0,0 +1,132 @@
+// SPDX-License-Identifier: AGPL-3.0-or-later
+
+package main
+
+import (
+	"flag"
+	"os"
+	"path/filepath"
+	"reflect"
+	"testing"
+)
+
+// TestFlagExplicit: -log-max-size counts as explicit — rotating a log
+// outside ~/.pilot — when given on the command line or in the config
+// file, under either key spelling and only with a value ApplyToFlags
+// would apply; the flag's default alone does not.
+func TestFlagExplicit(t *testing.T) {
+	saved := flag.CommandLine
+	t.Cleanup(func() { flag.CommandLine = saved })
+	parse := func(args ...string) {
+		t.Helper()
+		flag.CommandLine = flag.NewFlagSet("pilot-daemon", flag.ContinueOnError)
+		flag.Int("log-max-size", 50, "")
+		flag.Int("log-max-backups", 3, "")
+		if err := flag.CommandLine.Parse(args); err != nil {
+			t.Fatal(err)
+		}
+	}
+
+	for _, tc := range []struct {
+		name string
+		args []string
+		cfg  map[string]interface{}
+		want bool
+	}{
+		{"default", nil, nil, false},
+		{"other flag set", []string{"-log-max-backups", "5"}, map[string]interface{}{"log_max_backups": float64(5)}, false},
+		{"command line", []string{"-log-max-size", "50"}, nil, true},
+		{"command line 0", []string{"-log-max-size=0"}, nil, true},
+		{"config underscore", nil, map[string]interface{}{"log_max_size": float64(20)}, true},
+		{"config hyphen", nil, map[string]interface{}{"log-max-size": "20"}, true},
+		{"config null", nil, map[string]interface{}{"log_max_size": nil}, false},
+		{"config object", nil, map[string]interface{}{"log_max_size": map[string]interface{}{}}, false},
+		// ApplyToFlags reads the hyphenated key when present, even when
+		// its value is one it ignores.
+		{"config hyphen null shadows underscore", nil, map[string]interface{}{"log-max-size": nil, "log_max_size": float64(20)}, false},
+	} {
+		parse(tc.args...)
+		if got := flagExplicit("log-max-size", tc.cfg); got != tc.want {
+			t.Errorf("%s: flagExplicit = %v, want %v", tc.name, got, tc.want)
+		}
+	}
+}
+
+// TestPilotDirs: the default rotation scope is ~/.pilot, plus
+// $PILOT_HOME/.pilot where `pilotctl daemon start` keeps its logs when
+// PILOT_HOME is set.
+func TestPilotDirs(t *testing.T) {
+	home := t.TempDir()
+	t.Setenv("HOME", home)
+	t.Setenv("PILOT_HOME", "")
+	if got, want := pilotDirs(), []string{filepath.Join(home, ".pilot")}; !reflect.DeepEqual(got, want) {
+		t.Fatalf("pilotDirs = %v, want %v", got, want)
+	}
+
+	alt := t.TempDir()
+	t.Setenv("PILOT_HOME", alt)
+	want := []string{filepath.Join(home, ".pilot"), filepath.Join(alt, ".pilot")}
+	if got := pilotDirs(); !reflect.DeepEqual(got, want) {
+		t.Fatalf("pilotDirs = %v, want %v", got, want)
+	}
+}
+
+// TestHomebrewServiceLog: the pilot-daemon Pilot's Homebrew formula
+// installed maps to the log its `brew services` block sets,
+// <prefix>/var/log/pilot-daemon.log — however it was started: through
+// <prefix>/opt/pilotprotocol (the service's run path), the linked
+// <prefix>/bin, or the Cellar itself. Any other binary maps to nothing.
+func TestHomebrewServiceLog(t *testing.T) {
+	prefix := t.TempDir()
+	// macOS's temp dir is under the /var -> /private/var link; the
+	// result is built from the resolved executable path.
+	realPrefix, err := filepath.EvalSymlinks(prefix)
+	if err != nil {
+		t.Fatal(err)
+	}
+	file := func(rel string) string {
+		t.Helper()
+		p := filepath.Join(prefix, rel)
+		if err := os.MkdirAll(filepath.Dir(p), 0o755); err != nil {
+			t.Fatal(err)
+		}
+		if err := os.WriteFile(p, nil, 0o755); err != nil {
+			t.Fatal(err)
+		}
+		return p
+	}
+	link := func(target, rel string) string {
+		t.Helper()
+		p := filepath.Join(prefix, rel)
+		if err := os.MkdirAll(filepath.Dir(p), 0o755); err != nil {
+			t.Fatal(err)
+		}
+		if err := os.Symlink(target, p); err != nil {
+			t.Fatal(err)
+		}
+		return p
+	}
+
+	cellar := file("Cellar/pilotprotocol/1.13.9/bin/pilot-daemon")
+	opt := filepath.Join(link("../Cellar/pilotprotocol/1.13.9", "opt/pilotprotocol"), "bin", "pilot-daemon")
+	linked := link("../Cellar/pilotprotocol/1.13.9/bin/pilot-daemon", "bin/pilot-daemon")
+	want := filepath.Join(realPrefix, "var", "log", "pilot-daemon.log")
+	for _, exe := range []string{cellar, opt, linked} {
+		if got, ok := homebrewServiceLog(exe); !ok || got != want {
+			t.Errorf("homebrewServiceLog(%s) = (%q, %v), want (%q, true)", exe, got, ok, want)
+		}
+	}
+
+	for _, exe := range []string{
+		file(".pilot/bin/pilot-daemon"),                                      // install.sh
+		file("usr/local/bin/pilot-daemon"),                                   // a manual install
+		file("Cellar/otherformula/1.0/bin/pilot-daemon"),                     // another formula
+		file("Cellar/pilotprotocol/1.13.9/bin/pilotctl"),                     // another binary
+		file("Cellar/pilotprotocol/1.13.9/libexec/pilot-daemon"),             // not the formula's bin
+		filepath.Join(prefix, "Cellar/pilotprotocol/9.9.9/bin/pilot-daemon"), // missing
+	} {
+		if got, ok := homebrewServiceLog(exe); ok {
+			t.Errorf("homebrewServiceLog(%s) = %q, want no Homebrew service log", exe, got)
+		}
+	}
+}