Fix ai.

Author: David Grieser

pkg/proc/basejob.go

@@ -39,8 +39,14 @@ func (job *baseJob) SignalAll(sig syscall.Signal) {
 	}
 
 	log.WithField("job.name", job.Config.Name).Infof("sending signal %d to process group", sig)
-	// job.Cmd.Process.Pid is passed; SignalAllFunc is expected to negate it for group signaling if needed.
-	errFunc(job.SignalAllFunc(job.Cmd.Process.Pid, sig))
+	errFunc(job.signalAll(sig))
+}
+
+func (job *baseJob) signalAll(sig syscall.Signal) error {
+	if job.Cmd.Process.Pid <= 0 { // Ensure PID is positive before negating
+		return syscall.Errno(syscall.ESRCH) // No such process or invalid PID
+	}
+	return syscall.Kill(-job.Cmd.Process.Pid, sig)
 }
 
 func (job *baseJob) Signal(sig os.Signal) {
@@ -59,10 +65,18 @@ func (job *baseJob) Signal(sig os.Signal) {
 
 	log.WithField("job.name", job.Config.Name).Infof("sending signal %d to process", sig)
 	errFunc(
-		job.SignalFunc(job.Cmd.Process.Pid, sig),
+		job.signal(sig),
 	)
 }
 
+func (job *baseJob) signal(sig os.Signal) error {
+	process, err := os.FindProcess(job.Cmd.Process.Pid)
+	if err != nil {
+		return err
+	}
+	return process.Signal(sig)
+}
+
 func (job *baseJob) Reset() {
 	job.phase = JobPhase{}
 }
@@ -170,12 +184,11 @@ func (job *baseJob) startOnce(ctx context.Context, process chan<- *os.Process) e
 	// job errChan or failed
 	case err := <-errChan:
 		if job.Cmd != nil && job.Cmd.Process != nil { // Check if process actually started
-			// Use SignalAllFunc for consistency. It expects PID and will negate for group.
-			if termErr := job.SignalAllFunc(job.Cmd.Process.Pid, syscall.SIGTERM); termErr != nil {
+			if termErr := job.signalAll(syscall.SIGTERM); termErr != nil {
 				if e, ok := termErr.(syscall.Errno); ok && e == syscall.ESRCH {
 					// ESRCH (Error No Such Process) is fine, means process group already gone
 				} else {
-					l.WithError(termErr).Error("failed to send SIGTERM to job's process group via SignalAllFunc")
+					l.WithError(termErr).Error("failed to send SIGTERM to job's process group")
 				}
 			}
 		}
@@ -198,15 +211,14 @@ func (job *baseJob) startOnce(ctx context.Context, process chan<- *os.Process) e
 	case <-ctx.Done():
 		if job.Cmd != nil && job.Cmd.Process != nil { // Check if process actually started
 			// ctx canceled, try to terminate job
-			// Use SignalAllFunc for consistency. It expects PID and will negate for group.
-			_ = job.SignalAllFunc(job.Cmd.Process.Pid, syscall.SIGTERM)
-			l.WithField("job.name", job.Config.Name).Info("sent SIGTERM to job's process group via SignalAllFunc on ctx.Done")
+			_ = job.signalAll(syscall.SIGTERM)
+			l.WithField("job.name", job.Config.Name).Info("sent SIGTERM to job's process group on ctx.Done")
 
 			select {
 			case <-time.After(time.Second * ShutdownWaitingTimeSeconds):
 				// process seems to hang, kill process
-				_ = job.SignalAllFunc(job.Cmd.Process.Pid, syscall.SIGKILL) // Use func for consistency
-				l.WithField("job.name", job.Config.Name).Error("forcefully killed job via SignalAllFunc")
+				_ = job.signalAll(syscall.SIGKILL) // Use func for consistency
+				l.WithField("job.name", job.Config.Name).Error("forcefully killed job")
 				return nil
 
 			case err := <-errChan:
@@ -219,11 +231,6 @@ func (job *baseJob) startOnce(ctx context.Context, process chan<- *os.Process) e
 		return ctx.Err() // Return context error
 	}
 }
-			// all good
-			return err
-		}
-	}
-}
 
 func (job *baseJob) closeStdFiles() {
 	hasStdout := len(job.Config.Stdout) > 0

pkg/proc/job_common.go

@@ -180,9 +180,7 @@ func (job *CommonJob) IsRunning() bool {
 	if job.Cmd == nil || job.Cmd.Process == nil || job.Cmd.Process.Pid <= 0 {
 		return false
 	}
-	// Use job.SignalFunc with syscall.Signal(0) for process existence check.
-	// This respects the mockable function. A nil signal (0) is conventional for checking existence.
-	err := job.SignalFunc(job.Cmd.Process.Pid, syscall.Signal(0))
+	err := job.signal(syscall.Signal(0))
 	return err == nil
 }
 

pkg/proc/job_lazy.go

@@ -10,46 +10,7 @@ import (
 	log "github.com/sirupsen/logrus"
 )
 
-// LazyJobReapGracePeriod defines the time to wait after SIGTERM before sending SIGKILL.
-// It's a variable to allow modification for testing.
-var LazyJobReapGracePeriod = 10 * time.Second
-
-// GetLazyJobReapGracePeriodForTest returns the current grace period. Used by tests.
-func GetLazyJobReapGracePeriodForTest() time.Duration {
-	return LazyJobReapGracePeriod
-}
-
-// SetCoolDownTimeout allows tests to set a custom coolDownTimeout.
-func (job *LazyJob) SetCoolDownTimeout(d time.Duration) {
-	job.coolDownTimeout = d
-}
-
-// SetActiveConnections allows tests to set activeConnections.
-func (job *LazyJob) SetActiveConnections(n uint32) {
-	job.activeConnections = n
-}
-
-// SetLastConnectionClosed allows tests to set lastConnectionClosed.
-func (job *LazyJob) SetLastConnectionClosed(t time.Time) {
-	job.lastConnectionClosed = t
-}
-
-// SetProcess allows tests to set the job's process.
-// This is primarily for simulating a running process.
-// It sets both job.process (used by LazyJob specific logic)
-// and job.Cmd.Process (used by BaseJob signal methods via job.Cmd).
-func (job *LazyJob) SetProcess(p *os.Process) {
-	job.process = p
-	if job.Cmd != nil { // Ensure Cmd is initialized
-		job.Cmd.Process = p
-	} else if p != nil { // If Cmd is nil, but we have a process, create a dummy Cmd
-		// This case is tricky; ideally Cmd should be fully set up by a Start-like method.
-		// For testing reaper logic which relies on job.Cmd.Process.Pid, we need it.
-		// This might need refinement depending on how tests set up job.Cmd.
-		// log.Warn("SetProcess called on LazyJob with nil Cmd; creating a minimal Cmd. May need attention.")
-		// job.Cmd = &exec.Cmd{} // This alone is not enough, Path and SysProcAttr are needed by reaper's callers
-	}
-}
+const lazyJobReapGracePeriod = 10 * time.Second
 
 func (job *LazyJob) AssertStarted(ctx context.Context) error {
 	l := log.WithField("job.name", job.Config.Name)
@@ -118,15 +79,13 @@ func (job *LazyJob) Run(ctx context.Context, errors chan<- error) error {
 		}()
 	}
 
-	job.StartProcessReaper(ctx) // Renamed to be exported
+	job.startProcessReaper(ctx)
 
 	log.Infof("holding off starting job %s until first request", job.Config.Name)
 	return nil
 }
 
-// StartProcessReaper starts the goroutine that monitors the lazy job's process
-// and terminates it if it's idle for too long.
-func (job *LazyJob) StartProcessReaper(ctx context.Context) {
+func (job *LazyJob) startProcessReaper(ctx context.Context) {
 	reaperInterval := job.coolDownTimeout / 2
 	if reaperInterval < 1*time.Second {
 		reaperInterval = 1 * time.Second
@@ -174,15 +133,15 @@ func (job *LazyJob) StartProcessReaper(ctx context.Context) {
 				}
 				pidToReap := job.Cmd.Process.Pid
 				l.Infof("sending SIGTERM to idle process PID %d", pidToReap)
-				if err := job.Signal(syscall.SIGTERM); err != nil {
+				if err := job.signal(syscall.SIGTERM); err != nil {
 					l.WithError(err).Warnf("failed to send SIGTERM to PID %d", pidToReap)
 					job.lazyStartLock.Unlock()
 					continue
 				}
 
 				job.lazyStartLock.Unlock()
 
-				graceTimer := time.NewTimer(LazyJobReapGracePeriod) // Use the variable
+				graceTimer := time.NewTimer(lazyJobReapGracePeriod)
 				defer graceTimer.Stop()
 
 				select {
@@ -191,7 +150,7 @@ func (job *LazyJob) StartProcessReaper(ctx context.Context) {
 					// Check if the process we sent SIGTERM to is still running
 					if job.process != nil && job.Cmd != nil && job.Cmd.Process != nil && job.Cmd.Process.Pid == pidToReap {
 						l.Warnf("process PID %d did not exit after SIGTERM and grace period; sending SIGKILL", pidToReap)
-						if err := job.SignalAll(syscall.SIGKILL); err != nil {
+						if err := job.signalAll(syscall.SIGKILL); err != nil {
 							l.WithError(err).Errorf("failed to send SIGKILL to PID %d", pidToReap)
 						}
 					} else if job.process != nil {