From a1d4f99725d2d15422f1534e8ec329f8f218e234 Mon Sep 17 00:00:00 2001 From: PierrunoYT Date: Tue, 14 Jul 2026 22:48:33 +0200 Subject: [PATCH 01/11] fix(config): kill provider-command process tree via job object on Windows taskkill /T walks the process tree by parent PID in user space and can miss descendants, leaving the sleeping child holding the inherited stdout/stderr pipes so cmd.Wait() blocked until natural completion (~10s instead of the 5s timeout). Assign the started command to a job object with JOB_OBJECT_LIMIT_KILL_ON_JOB_CLOSE and terminate the job on timeout so the kernel kills the whole tree atomically; taskkill remains as a fallback. Also set cmd.WaitDelay so Wait can never hang on pipes held by an escaped orphan, and add a regression assertion that the sleeping child is actually gone after the timeout returns. Fixes #683 Co-Authored-By: Claude Fable 5 --- internal/config/command.go | 8 ++- internal/config/command_test.go | 37 ++++++++++++- internal/config/process_posix.go | 18 ++++-- internal/config/process_posix_test.go | 10 ++++ internal/config/process_windows.go | 73 ++++++++++++++++++++++++- internal/config/process_windows_test.go | 20 +++++++ 6 files changed, 156 insertions(+), 10 deletions(-) create mode 100644 internal/config/process_posix_test.go create mode 100644 internal/config/process_windows_test.go diff --git a/internal/config/command.go b/internal/config/command.go index ba4094ee1..4422d01b5 100644 --- a/internal/config/command.go +++ b/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() @@ -65,7 +71,7 @@ func runProviderCommand(command string, timeout time.Duration) ([]byte, []byte, case err := <-done: return stdout.Bytes(), stderr.Bytes(), err case <-timer.C: - terminateCommandProcess(cmd) + proc.Terminate() <-done return stdout.Bytes(), stderr.Bytes(), errProviderCommandTimeout } diff --git a/internal/config/command_test.go b/internal/config/command_test.go index 5127424c3..d201c811b 100644 --- a/internal/config/command_test.go +++ b/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,29 @@ func TestLoadProviderCommandTimeout(t *testing.T) { if elapsed > maxElapsed { t.Fatalf("timeout returned after %s, want roughly 5s", 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 +143,7 @@ type commandScript struct { Stderr string ExitCode int SleepSeconds int + PidFile string } func writeCommand(t *testing.T, script commandScript) string { @@ -128,7 +154,11 @@ 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 '" + script.PidFile + "' -Value $PID -Encoding Ascii; " + sleep + } + lines = append(lines, "powershell -NoProfile -Command \""+sleep+"\"") } if script.Stdout != "" { lines = append(lines, "echo "+script.Stdout) @@ -146,6 +176,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 $$ > '"+script.PidFile+"'") + } lines = append(lines, "sleep "+itoa(script.SleepSeconds)) } if script.Stdout != "" { diff --git a/internal/config/process_posix.go b/internal/config/process_posix.go index 501b07536..7138ef03f 100644 --- a/internal/config/process_posix.go +++ b/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() {} diff --git a/internal/config/process_posix_test.go b/internal/config/process_posix_test.go new file mode 100644 index 000000000..dd5ccb65d --- /dev/null +++ b/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 +} diff --git a/internal/config/process_windows.go b/internal/config/process_windows.go index 2e358887d..49d193b43 100644 --- a/internal/config/process_windows.go +++ b/internal/config/process_windows.go @@ -7,17 +7,84 @@ import ( "os/exec" "path/filepath" "strconv" + "unsafe" + + "golang.org/x/sys/windows" ) func configureCommandProcess(cmd *exec.Cmd) {} -func terminateCommandProcess(cmd *exec.Cmd) { +// 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 attachCommandProcess(cmd *exec.Cmd) *commandProcess { + proc := &commandProcess{cmd: cmd} if cmd.Process == nil { + return proc + } + job, err := createKillOnCloseJob() + 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 +} + +func createKillOnCloseJob() (windows.Handle, error) { + job, err := windows.CreateJobObject(nil, nil) + if err != nil { + return 0, err + } + info := windows.JOBOBJECT_EXTENDED_LIMIT_INFORMATION{ + BasicLimitInformation: windows.JOBOBJECT_BASIC_LIMIT_INFORMATION{ + LimitFlags: windows.JOB_OBJECT_LIMIT_KILL_ON_JOB_CLOSE, + }, + } + if _, err := windows.SetInformationJobObject(job, windows.JobObjectExtendedLimitInformation, uintptr(unsafe.Pointer(&info)), uint32(unsafe.Sizeof(info))); err != nil { + _ = windows.CloseHandle(job) + return 0, err + } + return job, nil +} + +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; KILL_ON_JOB_CLOSE reaps any process still +// assigned to the job. +func (p *commandProcess) Close() { + if p.job == 0 { + return + } + _ = windows.CloseHandle(p.job) + p.job = 0 } func taskkillPath() string { diff --git a/internal/config/process_windows_test.go b/internal/config/process_windows_test.go new file mode 100644 index 000000000..658f3f2a5 --- /dev/null +++ b/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 // STATUS_PENDING + return code == stillActive +} From c847946aa618a2d1446c48b14f1822aec293ac9b Mon Sep 17 00:00:00 2001 From: PierrunoYT Date: Wed, 15 Jul 2026 04:07:46 +0200 Subject: [PATCH 02/11] fix(config): harden provider-command tree termination Address code review on PR #690: terminate the process tree when cmd.Wait() returns exec.ErrWaitDelay (a background descendant kept inherited pipes open, previously left running); start the Windows process suspended and assign it to the job object before resuming its main thread, closing the window where a fast child could escape the job; drop JOB_OBJECT_LIMIT_KILL_ON_JOB_CLOSE so a successful command's detached helpers survive Close() instead of being reaped; escape single quotes in generated PID-file paths; and correct a misnamed Windows exit-code constant in a comment. Co-Authored-By: Claude Sonnet 5 --- internal/config/command.go | 8 ++++ internal/config/command_test.go | 60 ++++++++++++++++++++++++- internal/config/process_windows.go | 54 +++++++++++++++------- internal/config/process_windows_test.go | 2 +- 4 files changed, 105 insertions(+), 19 deletions(-) diff --git a/internal/config/command.go b/internal/config/command.go index 4422d01b5..2f01c2d83 100644 --- a/internal/config/command.go +++ b/internal/config/command.go @@ -69,6 +69,14 @@ 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: proc.Terminate() diff --git a/internal/config/command_test.go b/internal/config/command_test.go index d201c811b..b8113c39f 100644 --- a/internal/config/command_test.go +++ b/internal/config/command_test.go @@ -90,6 +90,34 @@ func TestLoadProviderCommandTimeout(t *testing.T) { 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() @@ -144,6 +172,13 @@ type commandScript struct { 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 { @@ -156,7 +191,7 @@ func writeCommand(t *testing.T, script commandScript) string { if script.SleepSeconds > 0 { sleep := "Start-Sleep -Seconds " + itoa(script.SleepSeconds) if script.PidFile != "" { - sleep = "Set-Content -Path '" + script.PidFile + "' -Value $PID -Encoding Ascii; " + sleep + sleep = "Set-Content -Path '" + psSingleQuote(script.PidFile) + "' -Value $PID -Encoding Ascii; " + sleep } lines = append(lines, "powershell -NoProfile -Command \""+sleep+"\"") } @@ -166,6 +201,10 @@ func writeCommand(t *testing.T, script commandScript) string { 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) @@ -177,7 +216,7 @@ func writeCommand(t *testing.T, script commandScript) string { lines := []string{"#!/bin/sh"} if script.SleepSeconds > 0 { if script.PidFile != "" { - lines = append(lines, "echo $$ > '"+script.PidFile+"'") + lines = append(lines, "echo $$ > '"+shSingleQuote(script.PidFile)+"'") } lines = append(lines, "sleep "+itoa(script.SleepSeconds)) } @@ -187,6 +226,10 @@ 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) @@ -194,6 +237,19 @@ func writeCommand(t *testing.T, script commandScript) string { return path } +// psSingleQuote escapes a value for interpolation inside a PowerShell +// single-quoted string literal, where a literal quote is written as ''. +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" diff --git a/internal/config/process_windows.go b/internal/config/process_windows.go index 49d193b43..f9a13547b 100644 --- a/internal/config/process_windows.go +++ b/internal/config/process_windows.go @@ -7,12 +7,20 @@ 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 @@ -28,7 +36,11 @@ func attachCommandProcess(cmd *exec.Cmd) *commandProcess { if cmd.Process == nil { return proc } - job, err := createKillOnCloseJob() + // 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 } @@ -46,21 +58,28 @@ func attachCommandProcess(cmd *exec.Cmd) *commandProcess { return proc } -func createKillOnCloseJob() (windows.Handle, error) { - job, err := windows.CreateJobObject(nil, nil) +// 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 0, err - } - info := windows.JOBOBJECT_EXTENDED_LIMIT_INFORMATION{ - BasicLimitInformation: windows.JOBOBJECT_BASIC_LIMIT_INFORMATION{ - LimitFlags: windows.JOB_OBJECT_LIMIT_KILL_ON_JOB_CLOSE, - }, + return } - if _, err := windows.SetInformationJobObject(job, windows.JobObjectExtendedLimitInformation, uintptr(unsafe.Pointer(&info)), uint32(unsafe.Sizeof(info))); err != nil { - _ = windows.CloseHandle(job) - return 0, err + 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) } - return job, nil } func (p *commandProcess) Terminate() { @@ -77,8 +96,11 @@ func (p *commandProcess) Terminate() { _ = p.cmd.Process.Kill() } -// Close releases the job handle; KILL_ON_JOB_CLOSE reaps any process still -// assigned to the job. +// 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 diff --git a/internal/config/process_windows_test.go b/internal/config/process_windows_test.go index 658f3f2a5..2b14a1f7c 100644 --- a/internal/config/process_windows_test.go +++ b/internal/config/process_windows_test.go @@ -15,6 +15,6 @@ func processAlive(pid int) bool { if err := windows.GetExitCodeProcess(handle, &code); err != nil { return false } - const stillActive = 259 // STATUS_PENDING + const stillActive = 259 // STILL_ACTIVE return code == stillActive } From f9da403be8ebb363540626254e51c2ba1cb2f727 Mon Sep 17 00:00:00 2001 From: PierrunoYT Date: Wed, 15 Jul 2026 04:16:04 +0200 Subject: [PATCH 03/11] fix(config): avoid gofmt smart-quote rewrite in doc comment gofmt's doc-comment formatter rewrites a trailing '' into a curly closing quote, which CI's gofmt -l check flags as unformatted. Reword to avoid the pattern instead of embedding a raw quote pair. --- internal/config/command_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/config/command_test.go b/internal/config/command_test.go index b8113c39f..3ddbab695 100644 --- a/internal/config/command_test.go +++ b/internal/config/command_test.go @@ -238,7 +238,7 @@ func writeCommand(t *testing.T, script commandScript) string { } // psSingleQuote escapes a value for interpolation inside a PowerShell -// single-quoted string literal, where a literal quote is written as ''. +// single-quoted string literal, where a literal quote is doubled. func psSingleQuote(value string) string { return strings.ReplaceAll(value, "'", "''") } From 2020680129133e6d6a08a7b27b834d4498eb13ea Mon Sep 17 00:00:00 2001 From: PierrunoYT Date: Fri, 17 Jul 2026 14:21:13 +0200 Subject: [PATCH 04/11] fix(config): terminate leaked descendants on nonzero exit, avoid PID reuse Terminate the process tree on any error returned from cmd.Wait, not only exec.ErrWaitDelay. Go only returns ErrWaitDelay when the shell itself exited successfully; a nonzero exit (e.g. sleep 600 & exit 7) yields the bare *ExitError instead, so a detached background child that held the inherited pipes open was never terminated on that path. Also stop falling through to the taskkill/PID fallback once a job handle successfully contained the tree: by the time Terminate runs on the ErrWaitDelay path, cmd.Wait has already reaped the root process, so its PID may have been reused by an unrelated process, and taskkill /T /PID against a reused PID would force-kill the wrong tree. Addresses PR #690 review feedback. --- internal/config/command.go | 14 +++++++++----- internal/config/command_test.go | 30 ++++++++++++++++++++++++++++++ internal/config/process_windows.go | 13 +++++++++++-- 3 files changed, 50 insertions(+), 7 deletions(-) diff --git a/internal/config/command.go b/internal/config/command.go index 2f01c2d83..022966d92 100644 --- a/internal/config/command.go +++ b/internal/config/command.go @@ -69,12 +69,16 @@ 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. + if err != nil { + // The shell exited (whether via ErrWaitDelay because a + // background descendant kept the inherited stdout/stderr pipes + // open, or via a nonzero exit status) but a leftover descendant + // may still be running. Terminate is a no-op against an + // already-dead tree, so always call it rather than gating on + // the specific error to avoid leaking that descendant. proc.Terminate() + } + if errors.Is(err, exec.ErrWaitDelay) { return stdout.Bytes(), stderr.Bytes(), errProviderCommandTimeout } return stdout.Bytes(), stderr.Bytes(), err diff --git a/internal/config/command_test.go b/internal/config/command_test.go index 3ddbab695..7af13208b 100644 --- a/internal/config/command_test.go +++ b/internal/config/command_test.go @@ -118,6 +118,36 @@ func TestLoadProviderCommandTerminatesBackgroundChild(t *testing.T) { assertProcessTerminated(t, pidFile) } +// TestLoadProviderCommandTerminatesBackgroundChildOnFailure covers the same +// leaked-descendant scenario as TestLoadProviderCommandTerminatesBackgroundChild, +// but with a shell that exits nonzero. Go's exec.Cmd.Wait only returns +// exec.ErrWaitDelay when the command itself exited successfully; a nonzero +// exit yields the bare *ExitError instead, so the leftover child must still +// be terminated on that path. +func TestLoadProviderCommandTerminatesBackgroundChildOnFailure(t *testing.T) { + pidFile := filepath.Join(t.TempDir(), "bg-fail.pid") + command := writeCommand(t, commandScript{ + Stderr: "boom", + ExitCode: 7, + BackgroundSleepSeconds: 10, + BackgroundPidFile: pidFile, + }) + + start := time.Now() + _, err := LoadProviderCommand(command) + elapsed := time.Since(start) + if err == nil { + t.Fatal("LoadProviderCommand() error = nil, want failure from nonzero exit") + } + if !strings.Contains(err.Error(), "exit status") { + t.Fatalf("error = %q, want exit status failure", 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() diff --git a/internal/config/process_windows.go b/internal/config/process_windows.go index f9a13547b..954b08daa 100644 --- a/internal/config/process_windows.go +++ b/internal/config/process_windows.go @@ -84,13 +84,22 @@ func resumeMainThread(pid int) { func (p *commandProcess) Terminate() { if p.job != 0 { + // A job handle means the process tree was contained from launch, so + // TerminateJobObject alone kills every member atomically. Don't also + // fall through to the taskkill/PID path below: by the time + // Terminate runs on the ErrWaitDelay path, cmd.Wait has already + // reaped the root process, so its PID may have been reused by an + // unrelated process, and forcing /T /PID against a reused PID would + // kill the wrong tree. _ = windows.TerminateJobObject(p.job, 1) + return } if p.cmd.Process == nil { return } - // Fallback for descendants spawned before the job assignment or when - // job creation failed. + // Fallback for the rare case where job creation or assignment failed: + // there's no containment, so the PID is the best signal available even + // though it carries the same reuse risk noted above. taskkill := taskkillPath() _ = exec.Command(taskkill, "/T", "/F", "/PID", strconv.Itoa(p.cmd.Process.Pid)).Run() _ = p.cmd.Process.Kill() From a03a2e63e6a2a87e947a14ed4f824bf3b7915b8f Mon Sep 17 00:00:00 2001 From: PierrunoYT Date: Fri, 17 Jul 2026 14:26:18 +0200 Subject: [PATCH 05/11] fix(config): surface suspended-thread resume failures instead of hanging resumeMainThread now reports whether it actually resumed at least one thread. If Toolhelp/OpenThread/ResumeThread all fail, the launched process would otherwise sit suspended forever, never exit on its own, and get misreported as a plain 5s provider-command timeout instead of the real resume failure. attachCommandProcess now terminates the process immediately in that case. Also documents the known, non-regressive limitation that the taskkill fallback (used only when job creation/assignment itself fails) can still miss a descendant that has reparented away from the tree it walks by PID. Addresses remaining PR #690 review feedback; PR description corrected separately to drop the outdated JOB_OBJECT_LIMIT_KILL_ON_JOB_CLOSE claim. --- internal/config/process_windows.go | 36 +++++++++++++++++++------ internal/config/process_windows_test.go | 18 ++++++++++++- 2 files changed, 45 insertions(+), 9 deletions(-) diff --git a/internal/config/process_windows.go b/internal/config/process_windows.go index 954b08daa..b0a8edc54 100644 --- a/internal/config/process_windows.go +++ b/internal/config/process_windows.go @@ -37,8 +37,17 @@ func attachCommandProcess(cmd *exec.Cmd) *commandProcess { 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) + // once we're done attaching, however that turns out. If resuming fails + // outright (Toolhelp/OpenThread/ResumeThread errors are otherwise + // swallowed), the process would sit suspended forever and never exit on + // its own, making the failure look like an ordinary 5s provider-command + // timeout. Terminate it immediately instead so the real cause surfaces + // quickly and no suspended process is left behind. + defer func() { + if !resumeMainThread(cmd.Process.Pid) { + proc.Terminate() + } + }() job, err := windows.CreateJobObject(nil, nil) if err != nil { @@ -58,17 +67,20 @@ func attachCommandProcess(cmd *exec.Cmd) *commandProcess { 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) { +// resumeMainThread resumes every (assumed suspended) thread of pid and +// reports whether at least one was actually resumed. Callers must treat a +// false result as a failed resume, not a benign no-op: a suspended process +// that's never woken up will neither exit nor produce output on its own. +func resumeMainThread(pid int) bool { snapshot, err := windows.CreateToolhelp32Snapshot(windows.TH32CS_SNAPTHREAD, 0) if err != nil { - return + return false } defer func() { _ = windows.CloseHandle(snapshot) }() var entry windows.ThreadEntry32 entry.Size = uint32(unsafe.Sizeof(entry)) + resumed := false for err := windows.Thread32First(snapshot, &entry); err == nil; err = windows.Thread32Next(snapshot, &entry) { if entry.OwnerProcessID != uint32(pid) { continue @@ -77,9 +89,12 @@ func resumeMainThread(pid int) { if err != nil { continue } - _, _ = windows.ResumeThread(thread) + if _, err := windows.ResumeThread(thread); err == nil { + resumed = true + } _ = windows.CloseHandle(thread) } + return resumed } func (p *commandProcess) Terminate() { @@ -99,7 +114,12 @@ func (p *commandProcess) Terminate() { } // Fallback for the rare case where job creation or assignment failed: // there's no containment, so the PID is the best signal available even - // though it carries the same reuse risk noted above. + // though it carries the same reuse risk noted above. Known limitation: + // unlike the job-object path, this can't reach a descendant that has + // already reparented away from the tree taskkill /T walks. This is not + // a regression versus pre-job-object behavior (taskkill /T was the only + // mechanism then too); closing it fully would require containment that + // doesn't depend on job objects and is tracked as a follow-up. taskkill := taskkillPath() _ = exec.Command(taskkill, "/T", "/F", "/PID", strconv.Itoa(p.cmd.Process.Pid)).Run() _ = p.cmd.Process.Kill() diff --git a/internal/config/process_windows_test.go b/internal/config/process_windows_test.go index 2b14a1f7c..2252024b8 100644 --- a/internal/config/process_windows_test.go +++ b/internal/config/process_windows_test.go @@ -2,7 +2,23 @@ package config -import "golang.org/x/sys/windows" +import ( + "testing" + + "golang.org/x/sys/windows" +) + +// TestResumeMainThreadReportsFailureForUnknownPID covers the contract +// attachCommandProcess relies on: when no thread belonging to pid can be +// found (here, because the pid doesn't correspond to a live process), +// resumeMainThread must report false rather than silently doing nothing, +// so a caller can react (e.g. terminate the process) instead of leaving it +// suspended indefinitely. +func TestResumeMainThreadReportsFailureForUnknownPID(t *testing.T) { + if resumeMainThread(0) { + t.Fatal("resumeMainThread(0) = true, want false for a pid with no threads") + } +} func processAlive(pid int) bool { handle, err := windows.OpenProcess(windows.PROCESS_QUERY_LIMITED_INFORMATION, false, uint32(pid)) From 4559643ad60e1839308c02f9bf1900276344877d Mon Sep 17 00:00:00 2001 From: PierrunoYT Date: Sat, 18 Jul 2026 13:36:01 +0200 Subject: [PATCH 06/11] fix(config): guard taskkill with retained process identity --- internal/config/command_test.go | 7 ++- internal/config/process_windows.go | 62 +++++++++++++++---------- internal/config/process_windows_test.go | 31 ++++++++++++- 3 files changed, 73 insertions(+), 27 deletions(-) diff --git a/internal/config/command_test.go b/internal/config/command_test.go index 7af13208b..14aec9159 100644 --- a/internal/config/command_test.go +++ b/internal/config/command_test.go @@ -232,8 +232,13 @@ func writeCommand(t *testing.T, script commandScript) string { 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) + readyFile := script.BackgroundPidFile + ".ready" + bgSleep := "Set-Content -LiteralPath '" + psSingleQuote(script.BackgroundPidFile) + "' -Value $PID -Encoding Ascii; " + + "Set-Content -LiteralPath '" + psSingleQuote(readyFile) + "' -Value ready -Encoding Ascii; " + + "Start-Sleep -Seconds " + itoa(script.BackgroundSleepSeconds) lines = append(lines, "start /B powershell -NoProfile -Command \""+bgSleep+"\"") + waitForReady := "while (-not (Test-Path -LiteralPath '" + psSingleQuote(readyFile) + "')) { Start-Sleep -Milliseconds 10 }" + lines = append(lines, "powershell -NoProfile -Command \""+waitForReady+"\"") } lines = append(lines, "exit /b "+itoa(script.ExitCode)) if err := os.WriteFile(path, []byte(strings.Join(lines, "\r\n")), 0o700); err != nil { diff --git a/internal/config/process_windows.go b/internal/config/process_windows.go index b0a8edc54..8f8864bf1 100644 --- a/internal/config/process_windows.go +++ b/internal/config/process_windows.go @@ -27,8 +27,9 @@ func configureCommandProcess(cmd *exec.Cmd) { // 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 + cmd *exec.Cmd + job windows.Handle + processHandle windows.Handle } func attachCommandProcess(cmd *exec.Cmd) *commandProcess { @@ -49,16 +50,16 @@ func attachCommandProcess(cmd *exec.Cmd) *commandProcess { } }() - job, err := windows.CreateJobObject(nil, nil) + handle, err := windows.OpenProcess(windows.PROCESS_SET_QUOTA|windows.PROCESS_TERMINATE|windows.PROCESS_QUERY_LIMITED_INFORMATION, false, uint32(cmd.Process.Pid)) if err != nil { return proc } - handle, err := windows.OpenProcess(windows.PROCESS_SET_QUOTA|windows.PROCESS_TERMINATE, false, uint32(cmd.Process.Pid)) + proc.processHandle = handle + + job, err := windows.CreateJobObject(nil, nil) 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 @@ -112,32 +113,43 @@ func (p *commandProcess) Terminate() { if p.cmd.Process == nil { return } + if p.processHandle == 0 { + // Without a retained identity, never target a numeric PID that may + // have been reaped and reused. os.Process.Kill uses Go's original + // process state instead. + _ = p.cmd.Process.Kill() + return + } // Fallback for the rare case where job creation or assignment failed: - // there's no containment, so the PID is the best signal available even - // though it carries the same reuse risk noted above. Known limitation: - // unlike the job-object path, this can't reach a descendant that has - // already reparented away from the tree taskkill /T walks. This is not - // a regression versus pre-job-object behavior (taskkill /T was the only - // mechanism then too); closing it fully would require containment that - // doesn't depend on job objects and is tracked as a follow-up. - taskkill := taskkillPath() - _ = exec.Command(taskkill, "/T", "/F", "/PID", strconv.Itoa(p.cmd.Process.Pid)).Run() - _ = p.cmd.Process.Kill() + // only invoke taskkill while the identity-preserving handle confirms the + // original root is still active. Keeping that handle open through Run + // prevents Windows from reusing its PID for an unrelated process. + var exitCode uint32 + if err := windows.GetExitCodeProcess(p.processHandle, &exitCode); err == nil && exitCode == stillActive { + taskkill := taskkillPath() + _ = exec.Command(taskkill, "/T", "/F", "/PID", strconv.Itoa(p.cmd.Process.Pid)).Run() + } + _ = windows.TerminateProcess(p.processHandle, 1) } -// 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. +// Close releases the retained handles 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 + if p.job != 0 { + _ = windows.CloseHandle(p.job) + p.job = 0 + } + if p.processHandle != 0 { + _ = windows.CloseHandle(p.processHandle) + p.processHandle = 0 } - _ = windows.CloseHandle(p.job) - p.job = 0 } +const stillActive = uint32(259) // STILL_ACTIVE + func taskkillPath() string { systemRoot := os.Getenv("SystemRoot") if systemRoot == "" { diff --git a/internal/config/process_windows_test.go b/internal/config/process_windows_test.go index 2252024b8..2e1f919ba 100644 --- a/internal/config/process_windows_test.go +++ b/internal/config/process_windows_test.go @@ -3,6 +3,7 @@ package config import ( + "os/exec" "testing" "golang.org/x/sys/windows" @@ -20,6 +21,35 @@ func TestResumeMainThreadReportsFailureForUnknownPID(t *testing.T) { } } +func TestCommandProcessRetainsIdentityAfterWait(t *testing.T) { + cmd := exec.Command("cmd", "/C", "exit /b 0") + configureCommandProcess(cmd) + if err := cmd.Start(); err != nil { + t.Fatalf("Start: %v", err) + } + proc := attachCommandProcess(cmd) + t.Cleanup(proc.Close) + if proc.processHandle == 0 { + t.Fatal("attachCommandProcess did not retain a process handle") + } + if err := cmd.Wait(); err != nil { + t.Fatalf("Wait: %v", err) + } + + var code uint32 + if err := windows.GetExitCodeProcess(proc.processHandle, &code); err != nil { + t.Fatalf("GetExitCodeProcess after Wait: %v", err) + } + if code == stillActive { + t.Fatalf("exit code = STILL_ACTIVE after Wait") + } + + proc.Close() + if proc.job != 0 || proc.processHandle != 0 { + t.Fatalf("Close left handles open: job=%v process=%v", proc.job, proc.processHandle) + } +} + func processAlive(pid int) bool { handle, err := windows.OpenProcess(windows.PROCESS_QUERY_LIMITED_INFORMATION, false, uint32(pid)) if err != nil { @@ -31,6 +61,5 @@ func processAlive(pid int) bool { if err := windows.GetExitCodeProcess(handle, &code); err != nil { return false } - const stillActive = 259 // STILL_ACTIVE return code == stillActive } From d6700a1ee60352653a36a9ad7a2ee5a47dd918b1 Mon Sep 17 00:00:00 2001 From: PierrunoYT Date: Sat, 18 Jul 2026 23:02:19 +0200 Subject: [PATCH 07/11] fix(config): retain POSIX process group identity --- internal/config/command.go | 6 ++-- internal/config/process_posix.go | 51 ++++++++++++++++++++------- internal/config/process_posix_test.go | 36 ++++++++++++++++++- internal/config/process_windows.go | 14 +++++++- 4 files changed, 89 insertions(+), 18 deletions(-) diff --git a/internal/config/command.go b/internal/config/command.go index 022966d92..01d11cdbb 100644 --- a/internal/config/command.go +++ b/internal/config/command.go @@ -42,7 +42,6 @@ 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 @@ -52,11 +51,10 @@ func runProviderCommand(command string, timeout time.Duration) ([]byte, []byte, cmd.Stdout = &stdout cmd.Stderr = &stderr - if err := cmd.Start(); err != nil { + proc, err := startCommandProcess(cmd) + if err != nil { return nil, nil, err } - - proc := attachCommandProcess(cmd) defer proc.Close() done := make(chan error, 1) diff --git a/internal/config/process_posix.go b/internal/config/process_posix.go index 7138ef03f..9ca09d491 100644 --- a/internal/config/process_posix.go +++ b/internal/config/process_posix.go @@ -3,28 +3,55 @@ package config import ( + "io" "os/exec" "syscall" ) -func configureCommandProcess(cmd *exec.Cmd) { - cmd.SysProcAttr = &syscall.SysProcAttr{Setpgid: true} -} - type commandProcess struct { - cmd *exec.Cmd + groupID int + anchor *exec.Cmd + anchorInput io.WriteCloser } -func attachCommandProcess(cmd *exec.Cmd) *commandProcess { - return &commandProcess{cmd: cmd} +// startCommandProcess retains a live process-group leader until cleanup. The +// provider shell may be reaped before Wait finishes draining descendant-held +// pipes, so its PID cannot safely identify the group after Wait returns. +func startCommandProcess(cmd *exec.Cmd) (*commandProcess, error) { + anchor := exec.Command("sh", "-c", "read _") + anchor.SysProcAttr = &syscall.SysProcAttr{Setpgid: true} + anchorInput, err := anchor.StdinPipe() + if err != nil { + return nil, err + } + if err := anchor.Start(); err != nil { + _ = anchorInput.Close() + return nil, err + } + + proc := &commandProcess{groupID: anchor.Process.Pid, anchor: anchor, anchorInput: anchorInput} + cmd.SysProcAttr = &syscall.SysProcAttr{Setpgid: true, Pgid: proc.groupID} + if err := cmd.Start(); err != nil { + proc.Close() + return nil, err + } + return proc, nil } func (p *commandProcess) Terminate() { - if p.cmd.Process == nil { - return + if p.groupID != 0 { + _ = syscall.Kill(-p.groupID, syscall.SIGKILL) } - _ = syscall.Kill(-p.cmd.Process.Pid, syscall.SIGKILL) - _ = p.cmd.Process.Kill() } -func (p *commandProcess) Close() {} +func (p *commandProcess) Close() { + if p.anchorInput != nil { + _ = p.anchorInput.Close() + p.anchorInput = nil + } + if p.anchor != nil { + _ = p.anchor.Wait() + p.anchor = nil + } + p.groupID = 0 +} diff --git a/internal/config/process_posix_test.go b/internal/config/process_posix_test.go index dd5ccb65d..0f8d298d5 100644 --- a/internal/config/process_posix_test.go +++ b/internal/config/process_posix_test.go @@ -2,7 +2,41 @@ package config -import "syscall" +import ( + "os/exec" + "syscall" + "testing" +) + +func TestCommandProcessRetainsGroupIdentityAfterProviderWait(t *testing.T) { + cmd := exec.Command("sh", "-c", "exit 7") + proc, err := startCommandProcess(cmd) + if err != nil { + t.Fatalf("startCommandProcess: %v", err) + } + defer func() { + proc.Terminate() + proc.Close() + }() + + providerPID := cmd.Process.Pid + anchorPID := proc.groupID + if err := cmd.Wait(); err == nil { + t.Fatal("Wait error = nil, want nonzero exit") + } + if anchorPID == providerPID { + t.Fatalf("group identity reused provider PID %d", providerPID) + } + if !processAlive(anchorPID) { + t.Fatalf("group leader %d exited before cleanup", anchorPID) + } + + proc.Terminate() + proc.Close() + if processAlive(anchorPID) { + t.Fatalf("group leader %d still alive after cleanup", anchorPID) + } +} func processAlive(pid int) bool { err := syscall.Kill(pid, 0) diff --git a/internal/config/process_windows.go b/internal/config/process_windows.go index 8f8864bf1..56209e67c 100644 --- a/internal/config/process_windows.go +++ b/internal/config/process_windows.go @@ -3,11 +3,13 @@ package config import ( + "context" "os" "os/exec" "path/filepath" "strconv" "syscall" + "time" "unsafe" "golang.org/x/sys/windows" @@ -22,6 +24,14 @@ func configureCommandProcess(cmd *exec.Cmd) { cmd.SysProcAttr = &syscall.SysProcAttr{CreationFlags: windows.CREATE_SUSPENDED} } +func startCommandProcess(cmd *exec.Cmd) (*commandProcess, error) { + configureCommandProcess(cmd) + if err := cmd.Start(); err != nil { + return nil, err + } + return attachCommandProcess(cmd), nil +} + // 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, @@ -127,7 +137,9 @@ func (p *commandProcess) Terminate() { var exitCode uint32 if err := windows.GetExitCodeProcess(p.processHandle, &exitCode); err == nil && exitCode == stillActive { taskkill := taskkillPath() - _ = exec.Command(taskkill, "/T", "/F", "/PID", strconv.Itoa(p.cmd.Process.Pid)).Run() + ctx, cancel := context.WithTimeout(context.Background(), 2*time.Second) + _ = exec.CommandContext(ctx, taskkill, "/T", "/F", "/PID", strconv.Itoa(p.cmd.Process.Pid)).Run() + cancel() } _ = windows.TerminateProcess(p.processHandle, 1) } From 0089d0d7fe611f011a31408e060c86f4ab34b788 Mon Sep 17 00:00:00 2001 From: PierrunoYT Date: Sat, 18 Jul 2026 23:02:19 +0200 Subject: [PATCH 08/11] test(tui): match provider wizard shortcut --- internal/tui/provider_wizard_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/tui/provider_wizard_test.go b/internal/tui/provider_wizard_test.go index 7efd36596..d550bf264 100644 --- a/internal/tui/provider_wizard_test.go +++ b/internal/tui/provider_wizard_test.go @@ -191,7 +191,7 @@ func TestProviderWizardAdvancesProviderAPIKeyAndModelSteps(t *testing.T) { for _, want := range []string{ "Paste API key", "ANTHROPIC_API_KEY", - "Enter continue", + "Enter/→ continue", } { assertContains(t, view, want) } From e10961e5cde3ba1cf8d82373afb647ef9fc4cbf1 Mon Sep 17 00:00:00 2001 From: PierrunoYT Date: Sun, 19 Jul 2026 13:50:13 +0200 Subject: [PATCH 09/11] test(tui): pin credential-step env for deterministic footer assertion MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The credential step's footer only shows the /→ shortcut once a key is present, whether typed or inherited from the provider's AuthEnvVars. The previous commit changed the assertion to "Enter/→ continue" without accounting for that, so the test only passed by coincidence when ANTHROPIC_API_KEY happened to be set in the environment — failing deterministically on a clean CI runner (as smoke reported on all three OSes). Pin the env var empty and assert the no-key footer text so the test is correct regardless of ambient environment. Co-Authored-By: Claude Sonnet 5 --- internal/tui/provider_wizard_test.go | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/internal/tui/provider_wizard_test.go b/internal/tui/provider_wizard_test.go index d550bf264..c5607fe60 100644 --- a/internal/tui/provider_wizard_test.go +++ b/internal/tui/provider_wizard_test.go @@ -169,6 +169,12 @@ func TestProviderWizardModelsAreProviderScoped(t *testing.T) { } func TestProviderWizardAdvancesProviderAPIKeyAndModelSteps(t *testing.T) { + // The credential-step footer's "Enter/→ continue" shortcut only appears + // once a key is present, whether typed or inherited from the + // provider's AuthEnvVars — pin ANTHROPIC_API_KEY empty so the assertion + // below doesn't depend on the ambient environment. + t.Setenv("ANTHROPIC_API_KEY", "") + m := newModel(context.Background(), Options{}) m = openProviderWizardForTest(t, m) @@ -191,7 +197,7 @@ func TestProviderWizardAdvancesProviderAPIKeyAndModelSteps(t *testing.T) { for _, want := range []string{ "Paste API key", "ANTHROPIC_API_KEY", - "Enter/→ continue", + "Enter continue", } { assertContains(t, view, want) } From cff78d6b3ea18806ef924b317a687620c3e7dfbf Mon Sep 17 00:00:00 2001 From: PierrunoYT Date: Sun, 19 Jul 2026 16:10:30 +0200 Subject: [PATCH 10/11] fix(config): preserve provider process attributes --- internal/config/process_posix.go | 6 +++++- internal/config/process_posix_test.go | 22 ++++++++++++++++++++++ internal/tui/provider_wizard_test.go | 6 ++---- 3 files changed, 29 insertions(+), 5 deletions(-) diff --git a/internal/config/process_posix.go b/internal/config/process_posix.go index 9ca09d491..258672337 100644 --- a/internal/config/process_posix.go +++ b/internal/config/process_posix.go @@ -30,7 +30,11 @@ func startCommandProcess(cmd *exec.Cmd) (*commandProcess, error) { } proc := &commandProcess{groupID: anchor.Process.Pid, anchor: anchor, anchorInput: anchorInput} - cmd.SysProcAttr = &syscall.SysProcAttr{Setpgid: true, Pgid: proc.groupID} + if cmd.SysProcAttr == nil { + cmd.SysProcAttr = &syscall.SysProcAttr{} + } + cmd.SysProcAttr.Setpgid = true + cmd.SysProcAttr.Pgid = proc.groupID if err := cmd.Start(); err != nil { proc.Close() return nil, err diff --git a/internal/config/process_posix_test.go b/internal/config/process_posix_test.go index 0f8d298d5..e1694696e 100644 --- a/internal/config/process_posix_test.go +++ b/internal/config/process_posix_test.go @@ -8,6 +8,28 @@ import ( "testing" ) +func TestCommandProcessPreservesSysProcAttr(t *testing.T) { + cmd := exec.Command("sh", "-c", "exit 0") + attr := &syscall.SysProcAttr{Pdeathsig: 0} + cmd.SysProcAttr = attr + + proc, err := startCommandProcess(cmd) + if err != nil { + t.Fatalf("startCommandProcess: %v", err) + } + defer proc.Close() + + if cmd.SysProcAttr != attr { + t.Fatal("startCommandProcess replaced SysProcAttr") + } + if !cmd.SysProcAttr.Setpgid || cmd.SysProcAttr.Pgid != proc.groupID { + t.Fatalf("process group = Setpgid %v, Pgid %d; want true, %d", cmd.SysProcAttr.Setpgid, cmd.SysProcAttr.Pgid, proc.groupID) + } + if err := cmd.Wait(); err != nil { + t.Fatalf("Wait: %v", err) + } +} + func TestCommandProcessRetainsGroupIdentityAfterProviderWait(t *testing.T) { cmd := exec.Command("sh", "-c", "exit 7") proc, err := startCommandProcess(cmd) diff --git a/internal/tui/provider_wizard_test.go b/internal/tui/provider_wizard_test.go index c5607fe60..4a5786e92 100644 --- a/internal/tui/provider_wizard_test.go +++ b/internal/tui/provider_wizard_test.go @@ -169,10 +169,8 @@ func TestProviderWizardModelsAreProviderScoped(t *testing.T) { } func TestProviderWizardAdvancesProviderAPIKeyAndModelSteps(t *testing.T) { - // The credential-step footer's "Enter/→ continue" shortcut only appears - // once a key is present, whether typed or inherited from the - // provider's AuthEnvVars — pin ANTHROPIC_API_KEY empty so the assertion - // below doesn't depend on the ambient environment. + // Pin the credential state so the footer assertion does not depend on the + // ambient environment. t.Setenv("ANTHROPIC_API_KEY", "") m := newModel(context.Background(), Options{}) From 467ff555d152a481ad0f5d24245378b3e4537195 Mon Sep 17 00:00:00 2001 From: PierrunoYT Date: Sun, 19 Jul 2026 16:32:56 +0200 Subject: [PATCH 11/11] test(config): keep SysProcAttr coverage portable --- internal/config/process_posix_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/config/process_posix_test.go b/internal/config/process_posix_test.go index e1694696e..309ed9e74 100644 --- a/internal/config/process_posix_test.go +++ b/internal/config/process_posix_test.go @@ -10,7 +10,7 @@ import ( func TestCommandProcessPreservesSysProcAttr(t *testing.T) { cmd := exec.Command("sh", "-c", "exit 0") - attr := &syscall.SysProcAttr{Pdeathsig: 0} + attr := &syscall.SysProcAttr{} cmd.SysProcAttr = attr proc, err := startCommandProcess(cmd)