diff --git a/cmd/serve_jobs_v2.go b/cmd/serve_jobs_v2.go index 8e4d1b83..69e95dc6 100644 --- a/cmd/serve_jobs_v2.go +++ b/cmd/serve_jobs_v2.go @@ -265,7 +265,6 @@ func (r *jobsV2ProgramRunner) Run(ctx context.Context, job jobsV2Job, pw progres } else { cmd = exec.CommandContext(ctx, cfg.Command, cfg.Args...) } - cmd.WaitDelay = jobsV2ProgramWaitDelay if strings.TrimSpace(cfg.Cwd) != "" { cmd.Dir = cfg.Cwd } @@ -273,10 +272,13 @@ func (r *jobsV2ProgramRunner) Run(ctx context.Context, job jobsV2Job, pw progres cmd.Env = append(os.Environ(), cfg.Env...) } - cleanup, prepErr := procutil.PrepareCommand(cmd) + cleanup, prepErr := tools.PrepareCommand(cmd) if prepErr != nil { return jobsV2RunResult{}, fmt.Errorf("program setup failed: %w", prepErr) } + // tools.PrepareCommand installs shell-tool defaults while adding robust + // descendant cleanup; keep the program runner's existing wait-delay behavior. + cmd.WaitDelay = jobsV2ProgramWaitDelay defer cleanup() stdout := procutil.NewLimitedBuffer(jobsV2ProgramOutputLimit) diff --git a/cmd/serve_jobs_v2_program_test.go b/cmd/serve_jobs_v2_program_test.go index 5b098b60..a7bfc210 100644 --- a/cmd/serve_jobs_v2_program_test.go +++ b/cmd/serve_jobs_v2_program_test.go @@ -3,6 +3,14 @@ package cmd import ( "context" "encoding/json" + "errors" + "fmt" + "os" + "path/filepath" + "runtime" + "strconv" + "strings" + "syscall" "testing" "time" ) @@ -55,6 +63,33 @@ func TestJobsV2ProgramRunner_TimeoutKillsBackgroundChildrenPromptly(t *testing.T } } +func TestJobsV2ProgramRunner_CleansUpBackgroundChildOnSuccess(t *testing.T) { + if runtime.GOOS == "windows" { + t.Skip("process-group cleanup test is unix-specific") + } + + pidPath := filepath.Join(t.TempDir(), "child.pid") + runner := &jobsV2ProgramRunner{} + job := testJobsV2ProgramJob(t, jobsV2ProgramConfig{ + Command: fmt.Sprintf(`sleep 30 >/dev/null 2>&1 & echo $! > %s`, jobsV2ProgramTestShellQuote(pidPath)), + Shell: true, + }) + + result, err := runner.Run(context.Background(), job, nil) + if err != nil { + t.Fatalf("Run failed: %v (stdout=%q stderr=%q)", err, result.Stdout, result.Stderr) + } + + pid := readJobsV2ProgramPID(t, pidPath) + defer func() { + if !jobsV2ProcessHasExited(pid) { + _ = syscall.Kill(pid, syscall.SIGKILL) + } + }() + + waitForJobsV2ProcessExit(t, pid) +} + func TestJobsV2ProgramRunner_TruncatesCapturedOutput(t *testing.T) { origLimit := jobsV2ProgramOutputLimit jobsV2ProgramOutputLimit = 32 @@ -88,3 +123,65 @@ func TestJobsV2ProgramRunner_TruncatesCapturedOutput(t *testing.T) { t.Fatalf("expected classifyRunError to preserve truncation") } } + +func readJobsV2ProgramPID(t *testing.T, path string) int { + t.Helper() + + data, err := os.ReadFile(path) + if err != nil { + t.Fatalf("read child pid file: %v", err) + } + pidText := strings.TrimSpace(string(data)) + pid, err := strconv.Atoi(pidText) + if err != nil { + t.Fatalf("parse child pid %q: %v", pidText, err) + } + return pid +} + +func waitForJobsV2ProcessExit(t *testing.T, pid int) { + t.Helper() + + deadline := time.Now().Add(2 * time.Second) + for time.Now().Before(deadline) { + if jobsV2ProcessHasExited(pid) { + return + } + time.Sleep(20 * time.Millisecond) + } + + t.Fatalf("timed out waiting for background child process %d to exit", pid) +} + +func jobsV2ProcessHasExited(pid int) bool { + err := syscall.Kill(pid, 0) + if err != nil { + return errors.Is(err, syscall.ESRCH) + } + if runtime.GOOS == "linux" { + state, ok := jobsV2LinuxProcState(pid) + return ok && state == 'Z' + } + return false +} + +func jobsV2LinuxProcState(pid int) (byte, bool) { + data, err := os.ReadFile(filepath.Join("/proc", strconv.Itoa(pid), "stat")) + if err != nil { + return 0, false + } + stat := string(data) + end := strings.LastIndex(stat, ")") + if end == -1 { + return 0, false + } + rest := strings.TrimSpace(stat[end+1:]) + if rest == "" { + return 0, false + } + return rest[0], true +} + +func jobsV2ProgramTestShellQuote(s string) string { + return "'" + strings.ReplaceAll(s, "'", "'\\''") + "'" +}