Make sure to stop the whole process group

Terminating the command executed and all of its children too

Signed-off-by: Djordje Lukic <djordje.lukic@docker.com>

Author: rumpl

.gitattributes

@@ -0,0 +1,28 @@
+# Auto detect text files and normalize to LF
+* text=auto eol=lf
+
+# Go source files
+*.go text eol=lf
+
+# Shell scripts
+*.sh text eol=lf
+
+# Windows specific files
+*.bat text eol=crlf
+*.cmd text eol=crlf
+*.ps1 text eol=crlf
+
+# Binary files
+*.exe binary
+*.dll binary
+*.so binary
+*.dylib binary
+*.png binary
+*.jpg binary
+*.jpeg binary
+*.gif binary
+*.ico binary
+*.pdf binary
+*.zip binary
+*.tar binary
+*.gz binary

go.mod

@@ -35,6 +35,7 @@ require (
 	go.opentelemetry.io/otel/sdk v1.38.0
 	go.opentelemetry.io/otel/trace v1.38.0
 	golang.org/x/oauth2 v0.32.0
+	golang.org/x/sys v0.37.0
 	golang.org/x/term v0.36.0
 	google.golang.org/genai v1.31.0
 	modernc.org/sqlite v1.39.1
@@ -116,7 +117,6 @@ require (
 	golang.org/x/exp v0.0.0-20250819193227-8b4c13bb791b // indirect
 	golang.org/x/net v0.46.0 // indirect
 	golang.org/x/sync v0.17.0 // indirect
-	golang.org/x/sys v0.37.0 // indirect
 	golang.org/x/text v0.30.0 // indirect
 	golang.org/x/time v0.14.0 // indirect
 	google.golang.org/genproto/googleapis/api v0.0.0-20250826171959-ef028d996bc1 // indirect

pkg/tools/builtin/cmd_unix.go

@@ -7,12 +7,20 @@ import (
 	"syscall"
 )
 
+type processGroup struct {
+	// Unix doesn't need to store handles, process group is managed by kernel
+}
+
 func platformSpecificSysProcAttr() *syscall.SysProcAttr {
 	return &syscall.SysProcAttr{
 		Setpgid: true,
 	}
 }
 
-func kill(proc *os.Process) error {
+func createProcessGroup(proc *os.Process) (*processGroup, error) {
+	return &processGroup{}, nil
+}
+
+func kill(proc *os.Process, pg *processGroup) error {
 	return syscall.Kill(-proc.Pid, syscall.SIGTERM)
 }

pkg/tools/builtin/cmd_windows.go

@@ -3,12 +3,70 @@ package builtin
 import (
 	"os"
 	"syscall"
+	"unsafe"
+
+	"golang.org/x/sys/windows"
 )
 
+type processGroup struct {
+	jobHandle     windows.Handle
+	processHandle windows.Handle
+}
+
 func platformSpecificSysProcAttr() *syscall.SysProcAttr {
 	return nil
 }
 
-func kill(proc *os.Process) error {
+func createProcessGroup(proc *os.Process) (*processGroup, error) {
+	job, err := windows.CreateJobObject(nil, nil)
+	if err != nil {
+		return nil, 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 nil, err
+	}
+
+	handle, err := windows.OpenProcess(windows.PROCESS_SET_QUOTA|windows.PROCESS_TERMINATE, false, uint32(proc.Pid))
+	if err != nil {
+		_ = windows.CloseHandle(job)
+		return nil, err
+	}
+
+	if err := windows.AssignProcessToJobObject(job, handle); err != nil {
+		_ = windows.CloseHandle(handle)
+		_ = windows.CloseHandle(job)
+		return nil, err
+	}
+
+	return &processGroup{
+		jobHandle:     job,
+		processHandle: handle,
+	}, nil
+}
+
+func kill(proc *os.Process, pg *processGroup) error {
+	if pg != nil {
+		// Close handles to trigger JOB_OBJECT_LIMIT_KILL_ON_JOB_CLOSE
+		// which will terminate all processes in the job
+		if pg.processHandle != 0 {
+			_ = windows.CloseHandle(pg.processHandle)
+		}
+		if pg.jobHandle != 0 {
+			_ = windows.CloseHandle(pg.jobHandle)
+		}
+	}
+
+	// Also call Kill on the process as a fallback
 	return proc.Kill()
 }

pkg/tools/builtin/shell.go

@@ -1,12 +1,14 @@
 package builtin
 
 import (
+	"bytes"
 	"context"
 	"encoding/json"
 	"fmt"
 	"os"
 	"os/exec"
 	"runtime"
+	"strings"
 
 	"github.com/docker/cagent/pkg/tools"
 )
@@ -36,37 +38,67 @@ func (h *shellHandler) RunShell(ctx context.Context, toolCall tools.ToolCall) (*
 		return nil, fmt.Errorf("invalid arguments: %w", err)
 	}
 
-	cmd := exec.CommandContext(ctx, h.shell, append(h.shellArgsPrefix, params.Cmd)...)
+	cmd := exec.Command(h.shell, append(h.shellArgsPrefix, params.Cmd)...)
 	cmd.Env = h.env
 	if params.Cwd != "" {
 		cmd.Dir = params.Cwd
 	} else {
-		// Use the current working directory; avoid PWD on Windows (may be MSYS-style like /c/...)
 		if wd, err := os.Getwd(); err == nil {
 			cmd.Dir = wd
 		}
 	}
 
-	// Set up process group for proper cleanup
-	// On Unix: create new process group so we can kill the entire tree
 	cmd.SysProcAttr = platformSpecificSysProcAttr()
 
-	output, err := cmd.CombinedOutput()
-	if err != nil {
+	var outBuf bytes.Buffer
+	cmd.Stdout = &outBuf
+	cmd.Stderr = &outBuf
+
+	if err := cmd.Start(); err != nil {
 		return &tools.ToolCallResult{
-			Output: fmt.Sprintf("Error executing command: %s\nOutput: %s", err, string(output)),
+			Output: fmt.Sprintf("Error starting command: %s", err),
 		}, nil
 	}
 
-	if len(output) == 0 {
+	pg, err := createProcessGroup(cmd.Process)
+	if err != nil {
 		return &tools.ToolCallResult{
-			Output: "<no output>",
+			Output: fmt.Sprintf("Error creating process group: %s", err),
 		}, nil
 	}
 
-	return &tools.ToolCallResult{
-		Output: fmt.Sprintf("Output: %s", string(output)),
-	}, nil
+	done := make(chan error, 1)
+	go func() {
+		done <- cmd.Wait()
+	}()
+
+	select {
+	case <-ctx.Done():
+		if cmd.Process != nil {
+			_ = kill(cmd.Process, pg)
+		}
+		return &tools.ToolCallResult{
+			Output: "Command cancelled",
+		}, nil
+	case err := <-done:
+		output := outBuf.String()
+
+		if err != nil {
+			return &tools.ToolCallResult{
+				Output: fmt.Sprintf("Error executing command: %s\nOutput: %s", err, output),
+			}, nil
+		}
+
+		if strings.TrimSpace(output) == "" {
+			return &tools.ToolCallResult{
+				Output: "<no output>",
+			}, nil
+		}
+
+		return &tools.ToolCallResult{
+			Output: fmt.Sprintf("Output: %s", output),
+		}, nil
+	}
 }
 
 func NewShellTool(env []string) *ShellTool {