From 57b3011e70dccb37c324474d63a20c7facd18406 Mon Sep 17 00:00:00 2001 From: Doug Brown Date: Tue, 19 May 2026 04:58:16 +0000 Subject: [PATCH] fix: daemon startup reliability on install/restart - Validate PID file points to actual git-tend process (fixes stale PID after reboot/crash blocking startup). Adds isOwnProcess platform check. - Use service manager for restart: systemctl --user restart on Linux, launchctl kickstart on macOS. Fixes Restart=on-failure not respawning after clean SIGTERM exit. - Run systemctl --user daemon-reload before enable --now during install. - Differentiate 'daemon not running' vs 'no repos found' in status output. - Ensure state directory exists before writing greet.last. - Add test coverage for isOwnProcess and empty-repo status branch. --- README.md | 2 +- cmd/git-tend/greet.go | 3 ++ cmd/git-tend/greet_test.go | 3 +- cmd/git-tend/restart.go | 42 +++++++++++++++++---------- cmd/git-tend/status.go | 8 +++-- cmd/git-tend/status_test.go | 50 ++++++++++++++++++++++++++++++++ internal/daemon/daemon.go | 34 +++++++++++++++++++++- internal/daemon/daemon_test.go | 16 ++++++++++ internal/install/install.go | 11 +++++-- internal/install/install_test.go | 2 +- internal/notify/notify_test.go | 19 ++++++++++++ 11 files changed, 167 insertions(+), 23 deletions(-) create mode 100644 cmd/git-tend/status_test.go diff --git a/README.md b/README.md index 441fe1c..5c20853 100644 --- a/README.md +++ b/README.md @@ -53,7 +53,7 @@ Make sure `~/.local/bin` is on your `PATH`, then install the daemon service: git-tend install ``` -On macOS this writes a launchd plist to `~/Library/LaunchAgents/com.dbrown.gittend.plist` and loads it. On Linux it writes a systemd user unit to `~/.config/systemd/user/git-tend.service` and enables it. +On macOS this writes a launchd plist to `~/Library/LaunchAgents/com.dougthings.gittend.plist` and loads it. On Linux it writes a systemd user unit to `~/.config/systemd/user/git-tend.service` and enables it. If `~/.config/git-tend/config.toml` doesn't exist, `install` writes a commented template. When stdin is a TTY it first prompts for which directories to scan (default `~/Code`, comma-separated for multiple). When run non-interactively (CI, piped install) it skips the prompt and writes the default. Either way the config path is printed so you can edit it. diff --git a/cmd/git-tend/greet.go b/cmd/git-tend/greet.go index 14af136..95e0cca 100644 --- a/cmd/git-tend/greet.go +++ b/cmd/git-tend/greet.go @@ -33,6 +33,9 @@ func runGreet(cmd *cobra.Command, args []string) error { } stateDir := paths.StateDir() + if err := os.MkdirAll(stateDir, 0755); err != nil { + return fmt.Errorf("creating state dir: %w", err) + } greetLastPath := filepath.Join(stateDir, "greet.last") today := time.Now().Format("2006-01-02") diff --git a/cmd/git-tend/greet_test.go b/cmd/git-tend/greet_test.go index 3577db0..801215b 100644 --- a/cmd/git-tend/greet_test.go +++ b/cmd/git-tend/greet_test.go @@ -9,6 +9,7 @@ import ( "github.com/spf13/cobra" + "github.com/sdougbrown/git-tend/internal/paths" "github.com/sdougbrown/git-tend/internal/status" ) @@ -17,7 +18,7 @@ func TestGreetDateStamp(t *testing.T) { t.Setenv("HOME", tmpHome) t.Setenv("NO_COLOR", "1") - stateDir := filepath.Join(tmpHome, "Library", "Application Support", "git-tend") + stateDir := paths.StateDir() os.MkdirAll(stateDir, 0755) configDir := filepath.Join(tmpHome, ".config", "git-tend") diff --git a/cmd/git-tend/restart.go b/cmd/git-tend/restart.go index ecfbc79..37884e4 100644 --- a/cmd/git-tend/restart.go +++ b/cmd/git-tend/restart.go @@ -1,15 +1,17 @@ package main import ( - "errors" "fmt" "os" + "os/exec" "path/filepath" + "runtime" "syscall" "time" "github.com/spf13/cobra" + "github.com/sdougbrown/git-tend/internal/install" "github.com/sdougbrown/git-tend/internal/paths" ) @@ -19,10 +21,9 @@ func init() { var restartCmd = &cobra.Command{ Use: "restart", - Short: "Stop the running daemon and let the service manager respawn it", - Long: `Send SIGTERM to the running daemon. The service manager (launchd on macOS, -systemd on Linux) respawns it against whatever binary the symlink/service file -points to. Use this after rebuilding to pick up new code, or any time you want + Short: "Restart the daemon via the service manager", + Long: `Restart the running daemon via systemd (Linux) or launchd (macOS). +Use this after rebuilding to pick up a new binary, or any time you want a fresh daemon process. Does not fetch or rebuild — that's on you.`, RunE: runRestart, } @@ -30,18 +31,29 @@ a fresh daemon process. Does not fetch or rebuild — that's on you.`, func runRestart(cmd *cobra.Command, args []string) error { pidPath := filepath.Join(paths.StateDir(), "daemon.pid") oldPid, err := readDaemonPid(pidPath) - if err != nil { + if err != nil && !os.IsNotExist(err) { return err } - proc, err := os.FindProcess(oldPid) - if err != nil { - return fmt.Errorf("finding process %d: %w", oldPid, err) + var smErr error + switch { + case install.IsMacOS(): + smErr = exec.Command("launchctl", "kickstart", "-kp", install.LaunchdLabel).Run() + case install.IsLinux(): + smErr = exec.Command("systemctl", "--user", "restart", "git-tend").Run() + default: + return fmt.Errorf("unsupported platform: %s", runtime.GOOS) } - if err := proc.Signal(syscall.SIGTERM); err != nil { - return fmt.Errorf("sending SIGTERM to pid %d: %w", oldPid, err) + + if smErr != nil { + return fmt.Errorf("service manager restart failed: %w (is the service loaded? try 'git-tend install')", smErr) + } + + if oldPid > 0 { + fmt.Printf("restarting daemon (was pid %d)...\n", oldPid) + } else { + fmt.Println("restarting daemon...") } - fmt.Printf("sent SIGTERM to pid %d, waiting for respawn...\n", oldPid) deadline := time.Now().Add(10 * time.Second) for time.Now().Before(deadline) { @@ -60,14 +72,14 @@ func runRestart(cmd *cobra.Command, args []string) error { return nil } - return fmt.Errorf("daemon did not respawn within 10s — is the service loaded? try 'git-tend install'") + return fmt.Errorf("daemon did not come back within 10s — is the service loaded? try 'git-tend install'") } func readDaemonPid(path string) (int, error) { data, err := os.ReadFile(path) if err != nil { - if errors.Is(err, os.ErrNotExist) { - return 0, fmt.Errorf("daemon not running (pid file missing at %s)", path) + if os.IsNotExist(err) { + return 0, fmt.Errorf("pid file missing at %s", path) } return 0, fmt.Errorf("reading pid file %s: %w", path, err) } diff --git a/cmd/git-tend/status.go b/cmd/git-tend/status.go index 57e6191..5876bce 100644 --- a/cmd/git-tend/status.go +++ b/cmd/git-tend/status.go @@ -26,8 +26,12 @@ func runStatus(cmd *cobra.Command, args []string) error { stateDir := paths.StateDir() sf := status.Read(filepath.Join(stateDir, "status.json")) - if sf == nil || len(sf.Repos) == 0 { - fmt.Println("No managed repos. Run 'git-tend daemon' first.") + if sf == nil { + fmt.Println("Daemon not running. Run 'git-tend install' or 'git-tend daemon'.") + return nil + } + if len(sf.Repos) == 0 { + fmt.Println("No managed repos found yet. If you just started the daemon, give it a minute to scan. Otherwise, make sure repos have a .gittend file inside them.") return nil } diff --git a/cmd/git-tend/status_test.go b/cmd/git-tend/status_test.go new file mode 100644 index 0000000..880b502 --- /dev/null +++ b/cmd/git-tend/status_test.go @@ -0,0 +1,50 @@ +package main + +import ( + "io" + "os" + "path/filepath" + "strings" + "testing" + + "github.com/spf13/cobra" + + "github.com/sdougbrown/git-tend/internal/status" +) + +func TestRunStatusEmptyRepos(t *testing.T) { + tmpHome := t.TempDir() + t.Setenv("HOME", tmpHome) + + stateDir := filepath.Join(tmpHome, ".local", "state", "git-tend") + os.MkdirAll(stateDir, 0755) + + sf := &status.StatusFile{ + Repos: map[string]status.RepoStatus{}, + } + status.Write(filepath.Join(stateDir, "status.json"), sf) + + r, w, err := os.Pipe() + if err != nil { + t.Fatal(err) + } + origStdout := os.Stdout + os.Stdout = w + + err = runStatus(&cobra.Command{}, nil) + if err != nil { + t.Fatalf("runStatus failed: %v", err) + } + + w.Close() + os.Stdout = origStdout + + data, err := io.ReadAll(r) + if err != nil { + t.Fatal(err) + } + + if !strings.Contains(string(data), "No managed repos found yet") { + t.Errorf("output = %q, want it to contain %q", string(data), "No managed repos found yet") + } +} diff --git a/internal/daemon/daemon.go b/internal/daemon/daemon.go index 43fc74b..aaf3223 100644 --- a/internal/daemon/daemon.go +++ b/internal/daemon/daemon.go @@ -6,8 +6,11 @@ import ( "fmt" "log/slog" "os" + "os/exec" "os/signal" "path/filepath" + "runtime" + "strings" "sync" "syscall" "time" @@ -123,7 +126,7 @@ func (d *Daemon) acquirePID() error { var existingPID int fmt.Sscanf(string(data), "%d", &existingPID) if existingPID > 0 { - if err := syscall.Kill(existingPID, 0); err == nil { + if err := syscall.Kill(existingPID, 0); err == nil && isOwnProcess(existingPID) { return fmt.Errorf("daemon already running, pid=%d", existingPID) } } @@ -132,6 +135,35 @@ func (d *Daemon) acquirePID() error { return os.WriteFile(d.pidPath, []byte(fmt.Sprintf("%d\n", os.Getpid())), 0644) } +// isOwnProcess checks whether pid belongs to a git-tend daemon. +// +// There is a narrow TOCTOU window after the caller's preceding Kill(pid, 0) +// succeeds: the process could exit and its PID be recycled before we read +// /proc//comm or run ps. In practice this is extremely unlikely for a +// long-lived daemon and is an acceptable limitation of a simple pidfile scheme. +func isOwnProcess(pid int) bool { + if pid == os.Getpid() { + return true + } + switch runtime.GOOS { + case "linux": + data, err := os.ReadFile(fmt.Sprintf("/proc/%d/comm", pid)) + if err != nil { + return false + } + return strings.TrimSpace(string(data)) == "git-tend" + case "darwin": + out, err := exec.Command("ps", "-p", fmt.Sprintf("%d", pid), "-o", "comm=").Output() + if err != nil { + return false + } + return filepath.Base(strings.TrimSpace(string(out))) == "git-tend" + default: + // Unknown platform: be conservative and treat any running PID as ours. + return true + } +} + func (d *Daemon) rescanRoots() { d.mu.Lock() defer d.mu.Unlock() diff --git a/internal/daemon/daemon_test.go b/internal/daemon/daemon_test.go index 4cfc521..c557fc3 100644 --- a/internal/daemon/daemon_test.go +++ b/internal/daemon/daemon_test.go @@ -90,6 +90,22 @@ func TestPIDRefusal(t *testing.T) { } } +func TestIsOwnProcess(t *testing.T) { + if !isOwnProcess(os.Getpid()) { + t.Error("isOwnProcess(os.Getpid()) should return true") + } + + cmd := exec.Command("sleep", "5") + if err := cmd.Start(); err != nil { + t.Fatal(err) + } + defer cmd.Process.Kill() + + if isOwnProcess(cmd.Process.Pid) { + t.Errorf("isOwnProcess(%d) should return false for a non-git-tend process", cmd.Process.Pid) + } +} + func TestDaemonTickIntegration(t *testing.T) { tempRoot := t.TempDir() diff --git a/internal/install/install.go b/internal/install/install.go index 6e2599f..febda96 100644 --- a/internal/install/install.go +++ b/internal/install/install.go @@ -12,12 +12,16 @@ import ( func IsMacOS() bool { return runtime.GOOS == "darwin" } func IsLinux() bool { return runtime.GOOS == "linux" } +const ( + LaunchdLabel = "com.dougthings.gittend" +) + const launchdPlistTemplate = ` Label - com.dbrown.gittend + ` + LaunchdLabel + ` ProgramArguments %s @@ -60,7 +64,7 @@ func launchdPlistPath() (string, error) { if err != nil { return "", err } - return filepath.Join(home, "Library", "LaunchAgents", "com.dbrown.gittend.plist"), nil + return filepath.Join(home, "Library", "LaunchAgents", LaunchdLabel+".plist"), nil } func systemdUnitPath() (string, error) { @@ -119,6 +123,9 @@ func LoadService() error { return exec.Command("launchctl", "load", plistPath).Run() } if IsLinux() { + if err := exec.Command("systemctl", "--user", "daemon-reload").Run(); err != nil { + return err + } return exec.Command("systemctl", "--user", "enable", "--now", "git-tend").Run() } return fmt.Errorf("unsupported platform: %s", runtime.GOOS) diff --git a/internal/install/install_test.go b/internal/install/install_test.go index b3ad15c..cd27573 100644 --- a/internal/install/install_test.go +++ b/internal/install/install_test.go @@ -30,7 +30,7 @@ func TestWriteLaunchdPlist(t *testing.T) { t.Fatal(err) } content := string(data) - for _, want := range []string{"com.dbrown.gittend", "KeepAlive", "RunAtLoad"} { + for _, want := range []string{LaunchdLabel, "KeepAlive", "RunAtLoad"} { if !strings.Contains(content, want) { t.Errorf("plist missing %q", want) } diff --git a/internal/notify/notify_test.go b/internal/notify/notify_test.go index ff17aee..82b054a 100644 --- a/internal/notify/notify_test.go +++ b/internal/notify/notify_test.go @@ -3,10 +3,15 @@ package notify import ( "os" "path/filepath" + "runtime" "testing" ) func TestNotifyMacOS(t *testing.T) { + if runtime.GOOS != "darwin" { + t.Skip("skipping macOS-only test") + } + tmpDir := t.TempDir() stubPath := filepath.Join(tmpDir, "osascript") calledPath := filepath.Join(tmpDir, "called") @@ -24,5 +29,19 @@ func TestNotifyMacOS(t *testing.T) { } func TestNotifyDoesNotCrash(t *testing.T) { + tmpDir := t.TempDir() + var stubName string + switch runtime.GOOS { + case "darwin": + stubName = "osascript" + case "linux": + stubName = "notify-send" + default: + t.Skip("unsupported platform") + } + stubPath := filepath.Join(tmpDir, stubName) + os.WriteFile(stubPath, []byte("#!/bin/sh\n"), 0755) + t.Setenv("PATH", tmpDir+":"+os.Getenv("PATH")) + Notify("test", "test body") }