Make watch stop work safely on Windows (#74)

main.go

@@ -597,6 +597,10 @@ func runWatchSubcommand(subCmd, root string) {
 			return
 		}
 		if err := stopWatchDaemon(absRoot); err != nil {
+			if errors.Is(err, watch.ErrForeignDaemonPID) {
+				fmt.Println("Watch daemon not running (cleared stale PID file)")
+				return
+			}
 			fmt.Fprintf(os.Stderr, "Error stopping daemon: %v\n", err)
 			os.Exit(1)
 		}

main_more_test.go

@@ -758,3 +758,24 @@ func TestSubcommandDispatchViaBinary(t *testing.T) {
 		}
 	})
 }
+
+func TestRunWatchSubcommandStopForeignPID(t *testing.T) {
+	root := t.TempDir()
+
+	origRunning := watchIsRunning
+	origStop := stopWatchDaemon
+	defer func() {
+		watchIsRunning = origRunning
+		stopWatchDaemon = origStop
+	}()
+	watchIsRunning = func(string) bool { return true }
+	stopWatchDaemon = func(string) error { return watch.ErrForeignDaemonPID }
+
+	stdout, _ := captureMainStreams(t, func() { runWatchSubcommand("stop", root) })
+	if !strings.Contains(stdout, "cleared stale PID file") {
+		t.Fatalf("expected stale-PID message on foreign PID, got:\n%s", stdout)
+	}
+	if strings.Contains(stdout, "Watch daemon stopped") {
+		t.Fatalf("should not claim the daemon was stopped for a foreign PID:\n%s", stdout)
+	}
+}

watch/process_unix.go

@@ -31,3 +31,11 @@ func processAlive(pid int) bool {
 	}
 	return proc.Signal(syscall.Signal(0)) == nil
 }
+
+// terminateDaemon stops the daemon on Unix with SIGTERM so it can shut down
+// gracefully. This matches long-standing behavior and does not gate on
+// ownership (unlike Windows, where the kill is destructive): the root argument
+// is accepted only to share a signature with the Windows implementation.
+func terminateDaemon(_ string, proc *os.Process) error {
+	return proc.Signal(syscall.SIGTERM)
+}

watch/process_windows.go

@@ -3,13 +3,26 @@
 package watch
 
 import (
-	"errors"
+	"fmt"
+	"os"
+	"os/exec"
+	"strings"
 	"syscall"
 )
 
+// processCommandLine returns the full command line of the given PID via CIM
+// (the Windows analog of `ps`; wmic is deprecated/removed on newer Windows).
+// IsOwnedDaemon uses this to confirm a PID belongs to this repo's watch daemon.
 func processCommandLine(pid int) (string, error) {
-	_ = pid
-	return "", errors.New("process command line lookup not supported on windows")
+	if pid <= 0 {
+		return "", fmt.Errorf("invalid pid %d", pid)
+	}
+	psCmd := fmt.Sprintf(`(Get-CimInstance Win32_Process -Filter "ProcessId=%d").CommandLine`, pid)
+	out, err := exec.Command("powershell", "-NoProfile", "-NonInteractive", "-Command", psCmd).Output()
+	if err != nil {
+		return "", err
+	}
+	return strings.TrimSpace(string(out)), nil
 }
 
 // processAlive reports whether a process with the given PID is currently
@@ -33,3 +46,20 @@ func processAlive(pid int) bool {
 	}
 	return code == stillActive
 }
+
+// terminateDaemon stops the daemon on Windows. Windows has no SIGTERM, so it
+// terminates with Kill — but only after confirming proc is actually this repo's
+// watch daemon. Ownership is checked against proc.Pid (not by re-reading
+// watch.pid) so we validate the exact process we are about to kill. A stale
+// watch.pid may point to a PID the OS reused for an unrelated process; killing
+// that would be destructive, so we refuse when ownership is foreign or unknown.
+func terminateDaemon(root string, proc *os.Process) error {
+	switch daemonOwnershipForPID(root, proc.Pid) {
+	case ownershipOwned:
+		return proc.Kill()
+	case ownershipForeign:
+		return ErrForeignDaemonPID
+	default:
+		return ErrDaemonOwnershipUnknown
+	}
+}

watch/state.go

@@ -2,14 +2,26 @@ package watch
 
 import (
 	"encoding/json"
+	"errors"
 	"fmt"
 	"os"
 	"path/filepath"
 	"strings"
-	"syscall"
 	"time"
 )
 
+// ErrForeignDaemonPID is returned by Stop when the PID in watch.pid is alive and
+// its command line was read but does NOT match this repo's watch daemon — i.e. a
+// stale PID the OS reused for an unrelated process. Callers can treat it as
+// "nothing of ours to stop" and safely discard the pid file.
+var ErrForeignDaemonPID = errors.New("watch.pid points to a live process that is not this repo's codemap watch daemon (stale or reused PID)")
+
+// ErrDaemonOwnershipUnknown is returned by Stop when the PID is alive but its
+// ownership could not be determined (the process command line was unavailable,
+// e.g. introspection was denied). We refuse to kill it AND keep the pid file, so
+// a real daemon is never orphaned or an unrelated process killed.
+var ErrDaemonOwnershipUnknown = errors.New("could not verify that watch.pid belongs to this repo's codemap watch daemon; refusing to stop it")
+
 // ReadState reads the daemon state from disk (for hooks to use).
 // Returns nil if state doesn't exist or if it's stale and daemon is not running.
 func ReadState(root string) *State {
@@ -57,34 +69,56 @@ func RemovePID(root string) {
 	os.Remove(pidFile)
 }
 
-// IsOwnedDaemon checks whether the PID file points to a codemap watch daemon
-// for this repository root.
-func IsOwnedDaemon(root string) bool {
-	pid, err := ReadPID(root)
-	if err != nil || pid <= 0 {
-		return false
-	}
+// daemonOwnership is the result of checking whether a specific PID is this
+// repo's watch daemon. It distinguishes "confirmed not ours" from "could not
+// determine" so callers never treat an unverifiable process as safe to discard.
+type daemonOwnership int
+
+const (
+	ownershipUnknown daemonOwnership = iota // could not read the process command line
+	ownershipOwned                          // command line matches this repo's watch daemon
+	ownershipForeign                        // command line retrieved and does NOT match
+)
 
+// daemonOwnershipForPID classifies whether pid is this repo's watch daemon.
+// It takes the PID explicitly (rather than re-reading watch.pid) so callers can
+// validate the exact process they are about to act on, avoiding a TOCTOU race
+// with a concurrent start/stop rewriting the pid file.
+func daemonOwnershipForPID(root string, pid int) daemonOwnership {
+	if pid <= 0 {
+		return ownershipForeign
+	}
 	cmdline, err := processCommandLine(pid)
 	if err != nil {
-		return false
+		return ownershipUnknown
 	}
 	cmdline = strings.TrimSpace(cmdline)
 	if cmdline == "" {
-		return false
+		return ownershipUnknown
 	}
 
 	absRoot, err := filepath.Abs(root)
 	if err != nil {
 		absRoot = root
 	}
-	if absRoot == "" {
-		return false
+	if absRoot != "" &&
+		strings.Contains(cmdline, "watch") &&
+		strings.Contains(cmdline, "daemon") &&
+		strings.Contains(cmdline, absRoot) {
+		return ownershipOwned
 	}
+	return ownershipForeign
+}
 
-	return strings.Contains(cmdline, "watch") &&
-		strings.Contains(cmdline, "daemon") &&
-		strings.Contains(cmdline, absRoot)
+// IsOwnedDaemon reports whether the PID file points to a codemap watch daemon
+// for this repository root. It is true only when ownership is positively
+// confirmed.
+func IsOwnedDaemon(root string) bool {
+	pid, err := ReadPID(root)
+	if err != nil || pid <= 0 {
+		return false
+	}
+	return daemonOwnershipForPID(root, pid) == ownershipOwned
 }
 
 // IsRunning checks if the daemon is running
@@ -108,12 +142,16 @@ func Stop(root string) error {
 	if err != nil {
 		return err
 	}
-	// NOTE: Windows has no SIGTERM, so this returns an error there (as it always
-	// has). `watch stop` on Windows is intentionally left non-destructive: safely
-	// killing would require verifying the PID still belongs to this repo's daemon
-	// (guarding against PID reuse), which needs a Windows command-line lookup that
-	// processCommandLine does not yet implement. Tracked as follow-up.
-	if err := proc.Signal(syscall.SIGTERM); err != nil {
+	// terminateDaemon is platform-specific: SIGTERM on Unix; on Windows it
+	// verifies the PID belongs to this repo's daemon (guarding against a reused
+	// stale PID) before killing, returning ErrForeignDaemonPID otherwise.
+	if err := terminateDaemon(root, proc); err != nil {
+		if errors.Is(err, ErrForeignDaemonPID) {
+			// The recorded PID isn't our daemon (stale or reused). Clear the
+			// bogus pid file so status stops reporting it, but never kill a
+			// process we can't confirm is ours.
+			RemovePID(root)
+		}
 		return err
 	}
 	// Clean up PID file