feat: kill entire process group on shell command timeout (#2253)

Author: nkanf-dev

tool/shell.go

@@ -501,6 +501,7 @@ func shellStopBackground(arguments map[string]interface{}) (*protocol.CallToolRe
 func shellBuildCommand(ctx context.Context, command string) (*exec.Cmd, error) {
 	cmd := exec.CommandContext(ctx, shellPlatformShell(), shellPlatformShellArg(), command)
 	cmd.Env = shellCommandEnv(false)
+	shellSetProcessGroup(cmd)
 	return cmd, nil
 }
 
@@ -511,28 +512,42 @@ func shellBuildPtyCommand(ctx context.Context, command string) (gopty.Pty, *gopt
 	}
 	cmd := ptmx.CommandContext(ctx, shellPlatformShell(), shellPlatformShellArg(), command)
 	cmd.Env = shellCommandEnv(true)
+	shellSetProcessGroupPty(cmd)
 	return ptmx, cmd, nil
 }
 
 func shellRunForeground(ctx context.Context, command string, workdir string, usePTY bool) (string, error) {
-	cmd, err := shellBuildCommand(ctx, command)
-	if err != nil {
-		return fmt.Sprintf("Error: %s", err.Error()), err
+	if !usePTY {
+		return shellRunForegroundCmd(ctx, command, workdir)
 	}
+	return shellRunForegroundPTY(ctx, command, workdir)
+}
 
-	if !usePTY {
-		if workdir != "" {
-			cmd.Dir = workdir
-		}
+func shellRunForegroundCmd(ctx context.Context, command string, workdir string) (string, error) {
+	cmd := exec.Command(shellPlatformShell(), shellPlatformShellArg(), command)
+	cmd.Env = shellCommandEnv(false)
+	shellSetProcessGroup(cmd)
+	if workdir != "" {
+		cmd.Dir = workdir
+	}
+
+	var stdout, stderr bytes.Buffer
+	cmd.Stdout = &stdout
+	cmd.Stderr = &stderr
 
-		var stdout, stderr bytes.Buffer
-		cmd.Stdout = &stdout
-		cmd.Stderr = &stderr
+	if startErr := cmd.Start(); startErr != nil {
+		return fmt.Sprintf("Error: %s", startErr.Error()), startErr
+	}
 
-		runErr := cmd.Run()
+	waitDone := make(chan error, 1)
+	go func() {
+		waitDone <- cmd.Wait()
+	}()
+
+	select {
+	case runErr := <-waitDone:
 		stdoutStr := stdout.String()
 		stderrStr := stderr.String()
-
 		if runErr != nil {
 			var parts []string
 			parts = append(parts, fmt.Sprintf("Error: %s", runErr.Error()))
@@ -544,7 +559,6 @@ func shellRunForeground(ctx context.Context, command string, workdir string, use
 			}
 			return strings.Join(parts, "\n"), runErr
 		}
-
 		result := stdoutStr
 		if stderrStr != "" {
 			if result != "" {
@@ -557,8 +571,15 @@ func shellRunForeground(ctx context.Context, command string, workdir string, use
 			result = "(no output)"
 		}
 		return result, nil
+
+	case <-ctx.Done():
+		shellKillProcessGroup(cmd.Process)
+		<-waitDone
+		return "", ctx.Err()
 	}
+}
 
+func shellRunForegroundPTY(ctx context.Context, command string, workdir string) (string, error) {
 	ptmx, ptyCmd, err := shellBuildPtyCommand(ctx, command)
 	if err != nil {
 		return fmt.Sprintf("Error: %s", err.Error()), err
@@ -586,25 +607,34 @@ func shellRunForeground(ctx context.Context, command string, workdir string, use
 		copyDone <- nil
 	}()
 
-	runErr := ptyCmd.Wait()
-	_ = ptmx.Close()
-	copyErr := <-copyDone
+	waitDone := make(chan error, 1)
+	go func() {
+		waitDone <- ptyCmd.Wait()
+	}()
 
-	if ctx.Err() == context.DeadlineExceeded {
-		return "", ctx.Err()
-	}
-	if copyErr != nil && runErr == nil {
-		runErr = copyErr
-	}
+	select {
+	case runErr := <-waitDone:
+		_ = ptmx.Close()
+		copyErr := <-copyDone
+		if copyErr != nil && runErr == nil {
+			runErr = copyErr
+		}
+		text := output.String()
+		if text == "" {
+			text = "(no output)"
+		}
+		if runErr != nil {
+			return fmt.Sprintf("Error: %s\nOutput:\n%s", runErr.Error(), text), runErr
+		}
+		return text, nil
 
-	text := output.String()
-	if text == "" {
-		text = "(no output)"
-	}
-	if runErr != nil {
-		return fmt.Sprintf("Error: %s\nOutput:\n%s", runErr.Error(), text), runErr
+	case <-ctx.Done():
+		shellKillProcessGroup(ptyCmd.Process)
+		_ = ptmx.Close()
+		<-waitDone
+		<-copyDone
+		return "", ctx.Err()
 	}
-	return text, nil
 }
 
 func shellStringArg(arguments map[string]interface{}, key string, defaultValue string) string {
@@ -971,13 +1001,13 @@ func (s *shellSession) stop() {
 		return
 	}
 	if err := process.Signal(os.Interrupt); err != nil {
-		_ = process.Kill()
+		shellKillProcessGroup(process)
 		return
 	}
 	select {
 	case <-done:
 	case <-time.After(2 * time.Second):
-		_ = process.Kill()
+		shellKillProcessGroup(process)
 	}
 }
 

tool/shell_kill_unix.go

@@ -0,0 +1,43 @@
+// Copyright 2026 The OpenAgent Authors. All Rights Reserved.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//      http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+//go:build unix
+
+package tool
+
+import (
+	"os"
+	"os/exec"
+	"syscall"
+
+	gopty "github.com/aymanbagabas/go-pty"
+)
+
+func shellSetProcessGroup(cmd *exec.Cmd) {
+	if cmd.SysProcAttr == nil {
+		cmd.SysProcAttr = &syscall.SysProcAttr{}
+	}
+	cmd.SysProcAttr.Setpgid = true
+}
+
+func shellSetProcessGroupPty(cmd *gopty.Cmd) {
+	if cmd.SysProcAttr == nil {
+		cmd.SysProcAttr = &syscall.SysProcAttr{}
+	}
+	cmd.SysProcAttr.Setpgid = true
+}
+
+func shellKillProcessGroup(process *os.Process) {
+	_ = syscall.Kill(-process.Pid, syscall.SIGKILL)
+}

tool/shell_kill_windows.go

@@ -0,0 +1,33 @@
+// Copyright 2026 The OpenAgent Authors. All Rights Reserved.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//      http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+//go:build windows
+
+package tool
+
+import (
+	"fmt"
+	"os"
+	"os/exec"
+
+	gopty "github.com/aymanbagabas/go-pty"
+)
+
+func shellSetProcessGroup(cmd *exec.Cmd) {}
+
+func shellSetProcessGroupPty(cmd *gopty.Cmd) {}
+
+func shellKillProcessGroup(process *os.Process) {
+	_ = exec.Command("taskkill", "/T", "/F", "/PID", fmt.Sprintf("%d", process.Pid)).Run()
+}