mirror of
https://github.com/Twigpine/zero.git
synced 2026-11-09 18:08:31 -10:00
* 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 <[email protected]> * 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 <[email protected]> * 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. * 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. * 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. * fix(config): guard taskkill with retained process identity * fix(config): retain POSIX process group identity * test(tui): match provider wizard shortcut * test(tui): pin credential-step env for deterministic footer assertion 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 <[email protected]> * fix(config): preserve provider process attributes * test(config): keep SysProcAttr coverage portable --------- Co-authored-by: Claude Fable 5 <[email protected]>
66 lines
1.8 KiB
Go
66 lines
1.8 KiB
Go
//go:build windows
|
|
|
|
package config
|
|
|
|
import (
|
|
"os/exec"
|
|
"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 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 {
|
|
return false
|
|
}
|
|
defer func() { _ = windows.CloseHandle(handle) }()
|
|
|
|
var code uint32
|
|
if err := windows.GetExitCodeProcess(handle, &code); err != nil {
|
|
return false
|
|
}
|
|
return code == stillActive
|
|
}
|