From 7fb66e0cc3b211ba7a13bde2b5d10bdd6074696c Mon Sep 17 00:00:00 2001 From: Jordan Coin Jackson Date: Wed, 1 Jul 2026 21:11:46 -0400 Subject: [PATCH 1/3] Fix `watch status` on Windows (#72) IsRunning probed process liveness with os.Process.Signal(0), which is unsupported on Windows: Signal returns an error for any signal other than Kill, so IsRunning always reported false and `watch status` said "Watch daemon not running" even with a live PID in .codemap/watch.pid. Move liveness detection behind a platform-specific processAlive: - Unix keeps the Signal(0) probe. - Windows opens the process and checks GetExitCodeProcess for STILL_ACTIVE. Also make Stop fall back to Process.Kill when SIGTERM is unsupported (Windows), so `watch stop` actually terminates the daemon there. Adds a host-runnable processAlive test covering live, dead, and invalid PIDs. Fixes #72 Co-Authored-By: Claude Opus 4.8 (1M context) --- watch/process_unix.go | 16 ++++++++++++++++ watch/process_windows.go | 27 ++++++++++++++++++++++++++- watch/state.go | 17 ++++++++--------- watch/state_test.go | 20 ++++++++++++++++++++ 4 files changed, 70 insertions(+), 10 deletions(-) diff --git a/watch/process_unix.go b/watch/process_unix.go index dba26d7..dec4ff1 100644 --- a/watch/process_unix.go +++ b/watch/process_unix.go @@ -3,9 +3,11 @@ package watch import ( + "os" "os/exec" "strconv" "strings" + "syscall" ) func processCommandLine(pid int) (string, error) { @@ -15,3 +17,17 @@ func processCommandLine(pid int) (string, error) { } return strings.TrimSpace(string(out)), nil } + +// processAlive reports whether a process with the given PID is currently +// running. FindProcess always succeeds on Unix, so liveness is probed with +// signal 0. +func processAlive(pid int) bool { + if pid <= 0 { + return false + } + proc, err := os.FindProcess(pid) + if err != nil { + return false + } + return proc.Signal(syscall.Signal(0)) == nil +} diff --git a/watch/process_windows.go b/watch/process_windows.go index bdc19e3..d04c4dc 100644 --- a/watch/process_windows.go +++ b/watch/process_windows.go @@ -2,9 +2,34 @@ package watch -import "errors" +import ( + "errors" + "syscall" +) func processCommandLine(pid int) (string, error) { _ = pid return "", errors.New("process command line lookup not supported on windows") } + +// processAlive reports whether a process with the given PID is currently +// running. os.Process.Signal(0) is unsupported on Windows — Signal returns an +// error for any signal other than Kill — so IsRunning cannot use it there. +// Instead we open the process and check its exit code: a live process reports +// STILL_ACTIVE. +func processAlive(pid int) bool { + if pid <= 0 { + return false + } + const stillActive = 259 // STILL_ACTIVE + h, err := syscall.OpenProcess(syscall.PROCESS_QUERY_INFORMATION, false, uint32(pid)) + if err != nil { + return false + } + defer syscall.CloseHandle(h) + var code uint32 + if err := syscall.GetExitCodeProcess(h, &code); err != nil { + return false + } + return code == stillActive +} diff --git a/watch/state.go b/watch/state.go index 907ae85..4c30955 100644 --- a/watch/state.go +++ b/watch/state.go @@ -93,14 +93,9 @@ func IsRunning(root string) bool { if err != nil { return false } - // Check if process exists by sending signal 0 - proc, err := os.FindProcess(pid) - if err != nil { - return false - } - // On Unix, FindProcess always succeeds, so send signal 0 to check - err = proc.Signal(syscall.Signal(0)) - return err == nil + // Liveness is checked in a platform-specific way: Signal(0) on Unix is + // unsupported on Windows, so processAlive queries the OS directly there. + return processAlive(pid) } // Stop sends SIGTERM to the daemon process @@ -114,7 +109,11 @@ func Stop(root string) error { return err } if err := proc.Signal(syscall.SIGTERM); err != nil { - return err + // Windows does not support SIGTERM; fall back to a hard kill so + // `watch stop` actually terminates the daemon there. + if killErr := proc.Kill(); killErr != nil { + return killErr + } } // Clean up PID file RemovePID(root) diff --git a/watch/state_test.go b/watch/state_test.go index 695c45f..d548170 100644 --- a/watch/state_test.go +++ b/watch/state_test.go @@ -3,6 +3,7 @@ package watch import ( "encoding/json" "os" + "os/exec" "path/filepath" "testing" "time" @@ -144,3 +145,22 @@ func TestWriteInitialStateWritesReadableState(t *testing.T) { t.Fatalf("unexpected recent events in state: %#v", got.RecentEvents) } } + +func TestProcessAliveDetectsLiveAndDeadPIDs(t *testing.T) { + if !processAlive(os.Getpid()) { + t.Fatal("current process should be reported alive") + } + if processAlive(0) { + t.Fatal("pid 0 should not be reported alive") + } + // Spawn a short-lived process, wait for it to exit, then confirm it is dead. + cmd := exec.Command("go", "version") + if err := cmd.Start(); err != nil { + t.Skipf("cannot spawn helper process: %v", err) + } + pid := cmd.Process.Pid + _ = cmd.Wait() + if processAlive(pid) { + t.Fatalf("exited process %d should not be reported alive", pid) + } +} From abc5b9de17a5a7b68770c06ead5f154855c21fef Mon Sep 17 00:00:00 2001 From: Jordan Coin Jackson Date: Wed, 1 Jul 2026 21:40:35 -0400 Subject: [PATCH 2/3] Address Copilot review on #73 - Stop: use a platform-specific terminateProcess (SIGTERM on Unix, Kill on Windows) instead of a blanket Kill-on-any-SIGTERM-error fallback, so Unix no longer masks ESRCH/EPERM. - Test: re-exec the test binary with `-test.run=^$` for the exited-PID case instead of depending on `go` being on PATH. Co-Authored-By: Claude Opus 4.8 (1M context) --- watch/process_unix.go | 6 ++++++ watch/process_windows.go | 7 +++++++ watch/state.go | 10 +++------- watch/state_test.go | 4 +++- 4 files changed, 19 insertions(+), 8 deletions(-) diff --git a/watch/process_unix.go b/watch/process_unix.go index dec4ff1..fbddaad 100644 --- a/watch/process_unix.go +++ b/watch/process_unix.go @@ -31,3 +31,9 @@ func processAlive(pid int) bool { } return proc.Signal(syscall.Signal(0)) == nil } + +// terminateProcess asks the daemon to shut down. On Unix SIGTERM lets it clean +// up gracefully; its error (e.g. ESRCH/EPERM) is returned unchanged. +func terminateProcess(proc *os.Process) error { + return proc.Signal(syscall.SIGTERM) +} diff --git a/watch/process_windows.go b/watch/process_windows.go index d04c4dc..4cfbfac 100644 --- a/watch/process_windows.go +++ b/watch/process_windows.go @@ -4,6 +4,7 @@ package watch import ( "errors" + "os" "syscall" ) @@ -33,3 +34,9 @@ func processAlive(pid int) bool { } return code == stillActive } + +// terminateProcess stops the daemon. Windows has no SIGTERM, so Kill (which +// maps to TerminateProcess) is the correct mechanism. +func terminateProcess(proc *os.Process) error { + return proc.Kill() +} diff --git a/watch/state.go b/watch/state.go index 4c30955..1b6871e 100644 --- a/watch/state.go +++ b/watch/state.go @@ -6,7 +6,6 @@ import ( "os" "path/filepath" "strings" - "syscall" "time" ) @@ -108,12 +107,9 @@ func Stop(root string) error { if err != nil { return err } - if err := proc.Signal(syscall.SIGTERM); err != nil { - // Windows does not support SIGTERM; fall back to a hard kill so - // `watch stop` actually terminates the daemon there. - if killErr := proc.Kill(); killErr != nil { - return killErr - } + // terminateProcess is platform-specific: SIGTERM on Unix, Kill on Windows. + if err := terminateProcess(proc); err != nil { + return err } // Clean up PID file RemovePID(root) diff --git a/watch/state_test.go b/watch/state_test.go index d548170..5aaee8e 100644 --- a/watch/state_test.go +++ b/watch/state_test.go @@ -154,7 +154,9 @@ func TestProcessAliveDetectsLiveAndDeadPIDs(t *testing.T) { t.Fatal("pid 0 should not be reported alive") } // Spawn a short-lived process, wait for it to exit, then confirm it is dead. - cmd := exec.Command("go", "version") + // Re-exec this test binary with a run filter that matches nothing so it + // exits immediately — self-contained, no dependency on an external tool. + cmd := exec.Command(os.Args[0], "-test.run=^$") if err := cmd.Start(); err != nil { t.Skipf("cannot spawn helper process: %v", err) } From 45968bc1ac6f1433e3307beb98c7b75908bf7083 Mon Sep 17 00:00:00 2001 From: Jordan Coin Jackson Date: Wed, 1 Jul 2026 22:09:54 -0400 Subject: [PATCH 3/3] Address Codex review: don't introduce Windows kill on stale PID MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Revert the Windows Kill fallback in Stop. It let `watch stop` terminate any live process matching a stale .codemap/watch.pid — on Windows the PID could have been reused by an unrelated process, and processCommandLine cannot yet verify daemon ownership there (P2 flagged by Codex). Stop returns to its prior behavior (SIGTERM, error on Windows), so this PR is scoped to the reported issue: fixing `watch status` via the new platform-specific processAlive. Safe Windows `stop` (with PID-ownership verification) is left as tracked follow-up. This also resolves the earlier Copilot note that the blanket Kill fallback masked ESRCH/EPERM on Unix. Co-Authored-By: Claude Opus 4.8 (1M context) --- watch/process_unix.go | 6 ------ watch/process_windows.go | 7 ------- watch/state.go | 9 +++++++-- 3 files changed, 7 insertions(+), 15 deletions(-) diff --git a/watch/process_unix.go b/watch/process_unix.go index fbddaad..dec4ff1 100644 --- a/watch/process_unix.go +++ b/watch/process_unix.go @@ -31,9 +31,3 @@ func processAlive(pid int) bool { } return proc.Signal(syscall.Signal(0)) == nil } - -// terminateProcess asks the daemon to shut down. On Unix SIGTERM lets it clean -// up gracefully; its error (e.g. ESRCH/EPERM) is returned unchanged. -func terminateProcess(proc *os.Process) error { - return proc.Signal(syscall.SIGTERM) -} diff --git a/watch/process_windows.go b/watch/process_windows.go index 4cfbfac..d04c4dc 100644 --- a/watch/process_windows.go +++ b/watch/process_windows.go @@ -4,7 +4,6 @@ package watch import ( "errors" - "os" "syscall" ) @@ -34,9 +33,3 @@ func processAlive(pid int) bool { } return code == stillActive } - -// terminateProcess stops the daemon. Windows has no SIGTERM, so Kill (which -// maps to TerminateProcess) is the correct mechanism. -func terminateProcess(proc *os.Process) error { - return proc.Kill() -} diff --git a/watch/state.go b/watch/state.go index 1b6871e..1fcdec7 100644 --- a/watch/state.go +++ b/watch/state.go @@ -6,6 +6,7 @@ import ( "os" "path/filepath" "strings" + "syscall" "time" ) @@ -107,8 +108,12 @@ func Stop(root string) error { if err != nil { return err } - // terminateProcess is platform-specific: SIGTERM on Unix, Kill on Windows. - if err := terminateProcess(proc); err != nil { + // 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 { return err } // Clean up PID file