fix(config): kill provider-command process tree via job object on Windows

internal/config/command.go

@@ -43,6 +43,9 @@ var errProviderCommandTimeout = errors.New("provider command timeout")
 func runProviderCommand(command string, timeout time.Duration) ([]byte, []byte, error) {
 	cmd := shellCommand(command)
 	configureCommandProcess(cmd)
+	// Bound the time Wait spends draining I/O pipes after the process exits,
+	// in case an orphaned descendant still holds the write ends open.
+	cmd.WaitDelay = time.Second
 
 	var stdout bytes.Buffer
 	var stderr bytes.Buffer
@@ -53,6 +56,9 @@ func runProviderCommand(command string, timeout time.Duration) ([]byte, []byte,
 		return nil, nil, err
 	}
 
+	proc := attachCommandProcess(cmd)
+	defer proc.Close()
+
 	done := make(chan error, 1)
 	go func() {
 		done <- cmd.Wait()
@@ -63,9 +69,17 @@ func runProviderCommand(command string, timeout time.Duration) ([]byte, []byte,
 
 	select {
 	case err := <-done:
+		if errors.Is(err, exec.ErrWaitDelay) {
+			// The process exited but a background descendant kept the
+			// inherited stdout/stderr pipes open past WaitDelay. Treat this
+			// like a timeout so the tree is actually terminated instead of
+			// leaking that descendant.
+			proc.Terminate()
+			return stdout.Bytes(), stderr.Bytes(), errProviderCommandTimeout
+		}
 		return stdout.Bytes(), stderr.Bytes(), err
 	case <-timer.C:
-		terminateCommandProcess(cmd)
+		proc.Terminate()
 		<-done
 		return stdout.Bytes(), stderr.Bytes(), errProviderCommandTimeout
 	}

internal/config/command_test.go

@@ -4,6 +4,7 @@ import (
 	"os"
 	"path/filepath"
 	"runtime"
+	"strconv"
 	"strings"
 	"testing"
 	"time"
@@ -67,7 +68,8 @@ func TestLoadProviderCommandFailureIncludesExitAndRedactsOutput(t *testing.T) {
 }
 
 func TestLoadProviderCommandTimeout(t *testing.T) {
-	command := writeCommand(t, commandScript{SleepSeconds: 10})
+	pidFile := filepath.Join(t.TempDir(), "sleep.pid")
+	command := writeCommand(t, commandScript{SleepSeconds: 10, PidFile: pidFile})
 
 	start := time.Now()
 	_, err := LoadProviderCommand(command)
@@ -85,6 +87,57 @@ func TestLoadProviderCommandTimeout(t *testing.T) {
 	if elapsed > maxElapsed {
 		t.Fatalf("timeout returned after %s, want roughly 5s", elapsed)
 	}
+	assertProcessTerminated(t, pidFile)
+}
+
+// TestLoadProviderCommandTerminatesBackgroundChild covers a command that
+// exits immediately but leaves a detached child holding the inherited
+// stdout/stderr pipes open (e.g. `sleep 600 & exit`). cmd.Wait() only
+// unblocks once WaitDelay elapses, and that path must still terminate the
+// leftover child instead of just returning and leaking it.
+func TestLoadProviderCommandTerminatesBackgroundChild(t *testing.T) {
+	pidFile := filepath.Join(t.TempDir(), "bg.pid")
+	command := writeCommand(t, commandScript{
+		Stdout:                 `{"name":"cmd","provider":"openai","apiKey":"sk-command","model":"gpt-command"}`,
+		BackgroundSleepSeconds: 10,
+		BackgroundPidFile:      pidFile,
+	})
+
+	start := time.Now()
+	_, err := LoadProviderCommand(command)
+	elapsed := time.Since(start)
+	if err == nil {
+		t.Fatal("LoadProviderCommand() error = nil, want error from WaitDelay-bounded wait")
+	}
+	if !strings.Contains(err.Error(), "timed out after 5s") {
+		t.Fatalf("error = %q, want timeout", err.Error())
+	}
+	if elapsed > 4*time.Second {
+		t.Fatalf("returned after %s, want well under the 5s provider-command timeout since WaitDelay (1s) should trigger termination", elapsed)
+	}
+	assertProcessTerminated(t, pidFile)
+}
+
+func assertProcessTerminated(t *testing.T, pidFile string) {
+	t.Helper()
+
+	data, err := os.ReadFile(pidFile)
+	if err != nil {
+		t.Fatalf("read sleeper pid file: %v", err)
+	}
+	pid, err := strconv.Atoi(strings.TrimSpace(string(data)))
+	if err != nil {
+		t.Fatalf("parse sleeper pid %q: %v", data, err)
+	}
+
+	deadline := time.Now().Add(3 * time.Second)
+	for time.Now().Before(deadline) {
+		if !processAlive(pid) {
+			return
+		}
+		time.Sleep(100 * time.Millisecond)
+	}
+	t.Fatalf("sleeper process %d still alive after timeout", pid)
 }
 
 func TestLoadProviderCommandInvalidJSON(t *testing.T) {
@@ -118,6 +171,14 @@ type commandScript struct {
 	Stderr       string
 	ExitCode     int
 	SleepSeconds int
+	PidFile      string
+
+	// BackgroundSleepSeconds, if set, spawns a detached child that keeps the
+	// inherited stdout/stderr handles open well after the script itself
+	// exits, simulating a `sleep 600 & exit` style command. BackgroundPidFile
+	// records the detached child's PID.
+	BackgroundSleepSeconds int
+	BackgroundPidFile      string
 }
 
 func writeCommand(t *testing.T, script commandScript) string {
@@ -128,14 +189,22 @@ func writeCommand(t *testing.T, script commandScript) string {
 		path := filepath.Join(dir, "provider.cmd")
 		lines := []string{"@echo off"}
 		if script.SleepSeconds > 0 {
-			lines = append(lines, "powershell -NoProfile -Command \"Start-Sleep -Seconds "+itoa(script.SleepSeconds)+"\"")
+			sleep := "Start-Sleep -Seconds " + itoa(script.SleepSeconds)
+			if script.PidFile != "" {
+				sleep = "Set-Content -Path '" + psSingleQuote(script.PidFile) + "' -Value $PID -Encoding Ascii; " + sleep
+			}
+			lines = append(lines, "powershell -NoProfile -Command \""+sleep+"\"")
 		}
 		if script.Stdout != "" {
 			lines = append(lines, "echo "+script.Stdout)
 		}
 		if script.Stderr != "" {
 			lines = append(lines, "echo "+script.Stderr+" 1>&2")
 		}
+		if script.BackgroundSleepSeconds > 0 {
+			bgSleep := "Set-Content -Path '" + psSingleQuote(script.BackgroundPidFile) + "' -Value $PID -Encoding Ascii; Start-Sleep -Seconds " + itoa(script.BackgroundSleepSeconds)
+			lines = append(lines, "start /B powershell -NoProfile -Command \""+bgSleep+"\"")
+		}
 		lines = append(lines, "exit /b "+itoa(script.ExitCode))
 		if err := os.WriteFile(path, []byte(strings.Join(lines, "\r\n")), 0o700); err != nil {
 			t.Fatalf("write command: %v", err)
@@ -146,6 +215,9 @@ func writeCommand(t *testing.T, script commandScript) string {
 	path := filepath.Join(dir, "provider.sh")
 	lines := []string{"#!/bin/sh"}
 	if script.SleepSeconds > 0 {
+		if script.PidFile != "" {
+			lines = append(lines, "echo $$ > '"+shSingleQuote(script.PidFile)+"'")
+		}
 		lines = append(lines, "sleep "+itoa(script.SleepSeconds))
 	}
 	if script.Stdout != "" {
@@ -154,13 +226,30 @@ func writeCommand(t *testing.T, script commandScript) string {
 	if script.Stderr != "" {
 		lines = append(lines, "printf '%s\\n' '"+script.Stderr+"' >&2")
 	}
+	if script.BackgroundSleepSeconds > 0 {
+		lines = append(lines, "sleep "+itoa(script.BackgroundSleepSeconds)+" &")
+		lines = append(lines, "echo $! > '"+shSingleQuote(script.BackgroundPidFile)+"'")
+	}
 	lines = append(lines, "exit "+itoa(script.ExitCode))
 	if err := os.WriteFile(path, []byte(strings.Join(lines, "\n")), 0o700); err != nil {
 		t.Fatalf("write command: %v", err)
 	}
 	return path
 }
 
+// psSingleQuote escapes a value for interpolation inside a PowerShell
+// single-quoted string literal, where a literal quote is doubled.
+func psSingleQuote(value string) string {
+	return strings.ReplaceAll(value, "'", "''")
+}
+
+// shSingleQuote escapes a value for interpolation inside a POSIX shell
+// single-quoted string literal, where a literal quote must close the
+// quoted section, emit an escaped quote, then reopen it.
+func shSingleQuote(value string) string {
+	return strings.ReplaceAll(value, "'", `'\''`)
+}
+
 func itoa(value int) string {
 	if value == 0 {
 		return "0"

internal/config/process_posix.go

@@ -11,10 +11,20 @@ func configureCommandProcess(cmd *exec.Cmd) {
 	cmd.SysProcAttr = &syscall.SysProcAttr{Setpgid: true}
 }
 
-func terminateCommandProcess(cmd *exec.Cmd) {
-	if cmd.Process == nil {
+type commandProcess struct {
+	cmd *exec.Cmd
+}
+
+func attachCommandProcess(cmd *exec.Cmd) *commandProcess {
+	return &commandProcess{cmd: cmd}
+}
+
+func (p *commandProcess) Terminate() {
+	if p.cmd.Process == nil {
 		return
 	}
-	_ = syscall.Kill(-cmd.Process.Pid, syscall.SIGKILL)
-	_ = cmd.Process.Kill()
+	_ = syscall.Kill(-p.cmd.Process.Pid, syscall.SIGKILL)
+	_ = p.cmd.Process.Kill()
 }
+
+func (p *commandProcess) Close() {}

internal/config/process_posix_test.go

@@ -0,0 +1,10 @@
+//go:build !windows
+
+package config
+
+import "syscall"
+
+func processAlive(pid int) bool {
+	err := syscall.Kill(pid, 0)
+	return err == nil || err == syscall.EPERM
+}

internal/config/process_windows.go

@@ -7,17 +7,106 @@ import (
 	"os/exec"
 	"path/filepath"
 	"strconv"
+	"syscall"
+	"unsafe"
+
+	"golang.org/x/sys/windows"
 )
 
-func configureCommandProcess(cmd *exec.Cmd) {}
+// configureCommandProcess starts the process suspended so it can be
+// assigned to the job object before its main thread (and therefore any
+// code it runs) executes. Without this, a fast command can spawn and
+// detach a descendant before AssignProcessToJobObject runs, letting that
+// descendant escape the job and survive termination.
+func configureCommandProcess(cmd *exec.Cmd) {
+	cmd.SysProcAttr = &syscall.SysProcAttr{CreationFlags: windows.CREATE_SUSPENDED}
+}
+
+// commandProcess tracks a started provider command so its entire process
+// tree can be terminated on timeout. It prefers a job object: taskkill /T
+// walks the tree by parent PID in user space and can miss descendants,
+// letting them run to completion while Wait blocks on inherited pipes.
+type commandProcess struct {
+	cmd *exec.Cmd
+	job windows.Handle
+}
 
-func terminateCommandProcess(cmd *exec.Cmd) {
+func attachCommandProcess(cmd *exec.Cmd) *commandProcess {
+	proc := &commandProcess{cmd: cmd}
 	if cmd.Process == nil {
+		return proc
+	}
+	// The main thread is suspended (see configureCommandProcess); resume it
+	// once we're done attaching, however that turns out.
+	defer resumeMainThread(cmd.Process.Pid)
+
+	job, err := windows.CreateJobObject(nil, nil)
+	if err != nil {
+		return proc
+	}
+	handle, err := windows.OpenProcess(windows.PROCESS_SET_QUOTA|windows.PROCESS_TERMINATE, false, uint32(cmd.Process.Pid))
+	if err != nil {
+		_ = windows.CloseHandle(job)
+		return proc
+	}
+	defer func() { _ = windows.CloseHandle(handle) }()
+	if err := windows.AssignProcessToJobObject(job, handle); err != nil {
+		_ = windows.CloseHandle(job)
+		return proc
+	}
+	proc.job = job
+	return proc
+}
+
+// resumeMainThread resumes the (assumed suspended) primary thread of pid.
+// It's a no-op if the thread can't be found or is already running.
+func resumeMainThread(pid int) {
+	snapshot, err := windows.CreateToolhelp32Snapshot(windows.TH32CS_SNAPTHREAD, 0)
+	if err != nil {
 		return
 	}
+	defer func() { _ = windows.CloseHandle(snapshot) }()
+
+	var entry windows.ThreadEntry32
+	entry.Size = uint32(unsafe.Sizeof(entry))
+	for err := windows.Thread32First(snapshot, &entry); err == nil; err = windows.Thread32Next(snapshot, &entry) {
+		if entry.OwnerProcessID != uint32(pid) {
+			continue
+		}
+		thread, err := windows.OpenThread(windows.THREAD_SUSPEND_RESUME, false, entry.ThreadID)
+		if err != nil {
+			continue
+		}
+		_, _ = windows.ResumeThread(thread)
+		_ = windows.CloseHandle(thread)
+	}
+}
+
+func (p *commandProcess) Terminate() {
+	if p.job != 0 {
+		_ = windows.TerminateJobObject(p.job, 1)
+	}
+	if p.cmd.Process == nil {
+		return
+	}
+	// Fallback for descendants spawned before the job assignment or when
+	// job creation failed.
 	taskkill := taskkillPath()
-	_ = exec.Command(taskkill, "/T", "/F", "/PID", strconv.Itoa(cmd.Process.Pid)).Run()
-	_ = cmd.Process.Kill()
+	_ = exec.Command(taskkill, "/T", "/F", "/PID", strconv.Itoa(p.cmd.Process.Pid)).Run()
+	_ = p.cmd.Process.Kill()
+}
+
+// Close releases the job handle without touching any still-running
+// descendants: the job carries no KILL_ON_JOB_CLOSE limit, so on the
+// success path a provider command's detached helpers keep running exactly
+// as they did before job objects were introduced. Descendant termination
+// happens explicitly via Terminate, called only on timeout/error.
+func (p *commandProcess) Close() {
+	if p.job == 0 {
+		return
+	}
+	_ = windows.CloseHandle(p.job)
+	p.job = 0
 }
 
 func taskkillPath() string {

internal/config/process_windows_test.go

@@ -0,0 +1,20 @@
+//go:build windows
+
+package config
+
+import "golang.org/x/sys/windows"
+
+func processAlive(pid int) bool {
+	handle, err := windows.OpenProcess(windows.PROCESS_QUERY_LIMITED_INFORMATION, false, uint32(pid))
+	if err != nil {
+		return false
+	}
+	defer func() { _ = windows.CloseHandle(handle) }()
+
+	var code uint32
+	if err := windows.GetExitCodeProcess(handle, &code); err != nil {
+		return false
+	}
+	const stillActive = 259 // STILL_ACTIVE
+	return code == stillActive
+}