From 29974e7234d4f3b81a5e74e17c5b7a69b02edf13 Mon Sep 17 00:00:00 2001 From: Liu Yuan Date: Wed, 18 Mar 2026 22:24:42 +0800 Subject: [PATCH] feat(tools): add exec tool enhancement with background execution and PTY support - Unified exec tool with actions: run/list/poll/read/write/kill - PTY support using creack/pty library - Process session management with background execution - Session isolation with owner-based authorization - Process group kill for cleaning up child processes - Security: forbid interpreters (sh, python, etc.) from using PTY Tests: - PTY: forbidden interpreters, allowed commands, write/read, poll, kill, process group kill - Non-PTY: background execution, list, read, write, poll, kill, process group kill - Session management: add/get/remove/list/cleanup - Session isolation: owner-based authorization --- go.mod | 3 +- go.sum | 1 + pkg/tools/session.go | 225 +++++++++ pkg/tools/session_process_unix.go | 25 + pkg/tools/session_process_windows.go | 21 + pkg/tools/session_test.go | 137 ++++++ pkg/tools/shell.go | 412 +++++++++++++++- pkg/tools/shell_test.go | 697 ++++++++++++++++++++++++++- pkg/tools/shell_timeout_unix_test.go | 1 + pkg/tools/types.go | 21 + 10 files changed, 1515 insertions(+), 28 deletions(-) create mode 100644 pkg/tools/session.go create mode 100644 pkg/tools/session_process_unix.go create mode 100644 pkg/tools/session_process_windows.go create mode 100644 pkg/tools/session_test.go diff --git a/go.mod b/go.mod index 4442b28fe..f1cd148d1 100644 --- a/go.mod +++ b/go.mod @@ -8,6 +8,7 @@ require ( github.com/anthropics/anthropic-sdk-go v1.26.0 github.com/bwmarrin/discordgo v0.29.0 github.com/caarlos0/env/v11 v11.4.0 + github.com/creack/pty v1.1.9 github.com/ergochat/irc-go v0.5.0 github.com/ergochat/readline v0.1.3 github.com/gdamore/tcell/v2 v2.13.8 @@ -93,7 +94,7 @@ require ( github.com/yosida95/uritemplate/v3 v3.0.2 // indirect golang.org/x/arch v0.24.0 // indirect golang.org/x/crypto v0.48.0 - golang.org/x/net v0.51.0 // indirect + golang.org/x/net v0.51.0 golang.org/x/sync v0.19.0 // indirect golang.org/x/sys v0.41.0 // indirect ) diff --git a/go.sum b/go.sum index f0e3fc132..407a2f2a1 100644 --- a/go.sum +++ b/go.sum @@ -35,6 +35,7 @@ github.com/coder/websocket v1.8.14 h1:9L0p0iKiNOibykf283eHkKUHHrpG7f65OE3BhhO7v9 github.com/coder/websocket v1.8.14/go.mod h1:NX3SzP+inril6yawo5CQXx8+fk145lPDC6pumgx0mVg= github.com/coreos/go-systemd/v22 v22.5.0/go.mod h1:Y58oyj3AT4RCenI/lSvhwexgC+NSVTIJ3seZv2GcEnc= github.com/cpuguy83/go-md2man/v2 v2.0.6/go.mod h1:oOW0eioCTA6cOiMLiUPZOpcVxMig6NIQQ7OS05n1F4g= +github.com/creack/pty v1.1.9 h1:uDmaGzcdjhF4i/plgjmEsriH11Y0o7RKapEf/LDaM3w= github.com/creack/pty v1.1.9/go.mod h1:oKZEueFk5CKHvIhNR5MUki03XCEU+Q6VDXinZuGJ33E= github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c= diff --git a/pkg/tools/session.go b/pkg/tools/session.go new file mode 100644 index 000000000..9979c9f1a --- /dev/null +++ b/pkg/tools/session.go @@ -0,0 +1,225 @@ +package tools + +import ( + "errors" + "io" + "os" + "sync" + + "github.com/google/uuid" +) + +var ( + ErrSessionNotFound = errors.New("session not found") + ErrSessionDone = errors.New("session already completed") + ErrPTYNotSupported = errors.New("PTY is not supported on this platform") + ErrNoStdin = errors.New("no stdin available") +) + +type ProcessSession struct { + mu sync.Mutex + ID string + PID int + Command string + PTY bool + Background bool + StartTime int64 + ExitCode int + Status string + stdinWriter io.Writer + stdoutPipe io.Reader + ptyMaster *os.File +} + +func (s *ProcessSession) IsDone() bool { + s.mu.Lock() + defer s.mu.Unlock() + return s.Status == "done" || s.Status == "exited" +} + +func (s *ProcessSession) GetStatus() string { + s.mu.Lock() + defer s.mu.Unlock() + return s.Status +} + +func (s *ProcessSession) SetStatus(status string) { + s.mu.Lock() + defer s.mu.Unlock() + s.Status = status +} + +func (s *ProcessSession) GetExitCode() int { + s.mu.Lock() + defer s.mu.Unlock() + return s.ExitCode +} + +func (s *ProcessSession) SetExitCode(code int) { + s.mu.Lock() + defer s.mu.Unlock() + s.ExitCode = code +} + +func (s *ProcessSession) killProcess() error { + s.mu.Lock() + defer s.mu.Unlock() + + if s.Status != "running" { + return ErrSessionDone + } + + pid := s.PID + if pid <= 0 { + return ErrSessionNotFound + } + + if err := killProcessGroup(pid); err != nil { + return err + } + + s.Status = "done" + s.ExitCode = -1 + return nil +} + +func (s *ProcessSession) Kill() error { + return s.killProcess() +} + +func (s *ProcessSession) Write(data string) error { + s.mu.Lock() + defer s.mu.Unlock() + + if s.Status != "running" { + return ErrSessionDone + } + + var writer io.Writer + if s.PTY && s.ptyMaster != nil { + writer = s.ptyMaster + } else if s.stdinWriter != nil { + writer = s.stdinWriter + } else { + return ErrNoStdin + } + + _, err := writer.Write([]byte(data)) + return err +} + +func (s *ProcessSession) Read() (string, error) { + s.mu.Lock() + if s.Status != "running" { + s.mu.Unlock() + return "", ErrSessionDone + } + + var reader io.Reader + if s.PTY && s.ptyMaster != nil { + reader = s.ptyMaster + } else if s.stdoutPipe != nil { + reader = s.stdoutPipe + } else { + s.mu.Unlock() + return "", nil + } + + buf := make([]byte, 4096) + s.mu.Unlock() + + n, err := reader.Read(buf) + if err != nil { + return "", s.handleReadError(err) + } + + return string(buf[:n]), nil +} + +func (s *ProcessSession) ToSessionInfo() SessionInfo { + s.mu.Lock() + defer s.mu.Unlock() + + return SessionInfo{ + ID: s.ID, + Command: s.Command, + Status: s.Status, + PID: s.PID, + StartedAt: s.StartTime, + } +} + +type SessionManager struct { + mu sync.RWMutex + sessions map[string]*ProcessSession +} + +func NewSessionManager() *SessionManager { + return &SessionManager{ + sessions: make(map[string]*ProcessSession), + } +} + +func (sm *SessionManager) Add(session *ProcessSession) { + sm.mu.Lock() + defer sm.mu.Unlock() + sm.sessions[session.ID] = session +} + +func (sm *SessionManager) Get(sessionID string) (*ProcessSession, error) { + sm.mu.RLock() + defer sm.mu.RUnlock() + + session, ok := sm.sessions[sessionID] + if !ok { + return nil, ErrSessionNotFound + } + + return session, nil +} + +func (sm *SessionManager) Remove(sessionID string) { + sm.mu.Lock() + defer sm.mu.Unlock() + delete(sm.sessions, sessionID) +} + +func (sm *SessionManager) List() []SessionInfo { + sm.mu.Lock() + defer sm.mu.Unlock() + + var result []SessionInfo + for id, session := range sm.sessions { + if session.IsDone() { + delete(sm.sessions, id) + continue + } + + result = append(result, session.ToSessionInfo()) + } + + return result +} + +func (sm *SessionManager) Cleanup() { + sm.mu.Lock() + defer sm.mu.Unlock() + + for id, session := range sm.sessions { + if session.IsDone() { + delete(sm.sessions, id) + } + } +} + +func generateSessionID() string { + return uuid.New().String()[:8] +} + +type SessionInfo struct { + ID string `json:"id"` + Command string `json:"command"` + Status string `json:"status"` + PID int `json:"pid"` + StartedAt int64 `json:"startedAt"` +} diff --git a/pkg/tools/session_process_unix.go b/pkg/tools/session_process_unix.go new file mode 100644 index 000000000..5064860ab --- /dev/null +++ b/pkg/tools/session_process_unix.go @@ -0,0 +1,25 @@ +//go:build !windows + +package tools + +import ( + "io" + "syscall" +) + +func killProcessGroup(pid int) error { + if err := syscall.Kill(-pid, syscall.SIGKILL); err != nil { + _ = syscall.Kill(pid, syscall.SIGKILL) + } + return nil +} + +func (s *ProcessSession) handleReadError(err error) error { + if err == io.EOF { + return nil + } + if err == syscall.EIO { + return nil + } + return err +} diff --git a/pkg/tools/session_process_windows.go b/pkg/tools/session_process_windows.go new file mode 100644 index 000000000..35bf11673 --- /dev/null +++ b/pkg/tools/session_process_windows.go @@ -0,0 +1,21 @@ +//go:build windows + +package tools + +import ( + "io" + "os/exec" + "strconv" +) + +func killProcessGroup(pid int) error { + _ = exec.Command("taskkill", "/T", "/F", "/PID", strconv.Itoa(pid)).Run() + return nil +} + +func (s *ProcessSession) handleReadError(err error) error { + if err == io.EOF { + return nil + } + return err +} diff --git a/pkg/tools/session_test.go b/pkg/tools/session_test.go new file mode 100644 index 000000000..f27d7d93b --- /dev/null +++ b/pkg/tools/session_test.go @@ -0,0 +1,137 @@ +package tools + +import ( + "testing" + + "github.com/stretchr/testify/require" +) + +func TestSessionManager_AddGet(t *testing.T) { + sm := NewSessionManager() + session := &ProcessSession{ + ID: "test-1", + Command: "echo hello", + Status: "running", + StartTime: 1000, + } + + sm.Add(session) + + got, err := sm.Get("test-1") + require.NoError(t, err) + require.Equal(t, "test-1", got.ID) +} + +func TestSessionManager_Remove(t *testing.T) { + sm := NewSessionManager() + session := &ProcessSession{ + ID: "test-1", + Command: "echo hello", + Status: "running", + StartTime: 1000, + } + sm.Add(session) + sm.Remove("test-1") + + _, err := sm.Get("test-1") + require.ErrorIs(t, err, ErrSessionNotFound) +} + +func TestSessionManager_List(t *testing.T) { + sm := NewSessionManager() + sm.Add(&ProcessSession{ + ID: "test-1", + Command: "echo hello", + Status: "running", + StartTime: 1000, + }) + sm.Add(&ProcessSession{ + ID: "test-2", + Command: "echo world", + Status: "running", + StartTime: 1001, + }) + sm.Add(&ProcessSession{ + ID: "test-3", + Command: "echo done", + Status: "done", + StartTime: 1002, + }) + + sessions := sm.List() + require.Len(t, sessions, 2) + + ids := make(map[string]bool) + for _, s := range sessions { + ids[s.ID] = true + } + require.True(t, ids["test-1"]) + require.True(t, ids["test-2"]) +} + +func TestSessionManager_List_CleanupDeadSessions(t *testing.T) { + sm := NewSessionManager() + sm.Add(&ProcessSession{ + ID: "test-1", + Command: "echo hello", + Status: "running", + StartTime: 1000, + }) + sm.Add(&ProcessSession{ + ID: "test-2", + Command: "echo done", + Status: "done", + StartTime: 1001, + }) + + sessions := sm.List() + require.Len(t, sessions, 1) + require.Equal(t, "test-1", sessions[0].ID) + + _, err := sm.Get("test-2") + require.ErrorIs(t, err, ErrSessionNotFound) +} + +func TestProcessSession_IsDone(t *testing.T) { + session := &ProcessSession{Status: "running"} + require.False(t, session.IsDone()) + + session.Status = "done" + require.True(t, session.IsDone()) + + session.Status = "exited" + require.True(t, session.IsDone()) +} + +func TestProcessSession_ToSessionInfo(t *testing.T) { + session := &ProcessSession{ + ID: "test-1", + PID: 12345, + Command: "echo hello", + Status: "running", + StartTime: 1000, + } + + info := session.ToSessionInfo() + require.Equal(t, "test-1", info.ID) + require.Equal(t, "echo hello", info.Command) + require.Equal(t, "running", info.Status) + require.Equal(t, 12345, info.PID) + require.Equal(t, int64(1000), info.StartedAt) +} + +func TestIsForbiddenInterpreter(t *testing.T) { + forbidden := []string{ + "bash", "sh", "zsh", "fish", "python", "python3", "node", "ruby", "perl", + } + for _, cmd := range forbidden { + require.True(t, isForbiddenInterpreter(cmd), "expected %s to be forbidden", cmd) + } + + allowed := []string{ + "vim", "htop", "less", "docker", "cat", "echo", + } + for _, cmd := range allowed { + require.False(t, isForbiddenInterpreter(cmd), "expected %s to be allowed", cmd) + } +} diff --git a/pkg/tools/shell.go b/pkg/tools/shell.go index 0dc85ae21..82d8d1805 100644 --- a/pkg/tools/shell.go +++ b/pkg/tools/shell.go @@ -3,20 +3,34 @@ package tools import ( "bytes" "context" + "encoding/json" "errors" "fmt" + "io" "os" "os/exec" "path/filepath" "regexp" "runtime" "strings" + "sync" + "syscall" "time" + "github.com/creack/pty" "github.com/sipeed/picoclaw/pkg/config" "github.com/sipeed/picoclaw/pkg/constants" ) +var globalSessionManager = NewSessionManager() +var sessionManagerMu sync.RWMutex + +func getSessionManager() *SessionManager { + sessionManagerMu.RLock() + defer sessionManagerMu.RUnlock() + return globalSessionManager +} + type ExecTool struct { workingDir string timeout time.Duration @@ -26,6 +40,30 @@ type ExecTool struct { allowedPathPatterns []*regexp.Regexp restrictToWorkspace bool allowRemote bool + sessionManager *SessionManager +} + +var ptyForbiddenPrograms = []string{ + "bash", "sh", "zsh", "fish", "dash", "ksh", "csh", "tcsh", + "python", "python3", "python2", + "node", "nodejs", + "ruby", "perl", "php", "lua", "lua5", + "powershell", "pwsh", + "adb", "telnet", +} + +func isForbiddenInterpreter(cmd string) bool { + parts := strings.Fields(cmd) + if len(parts) == 0 { + return false + } + base := filepath.Base(parts[0]) + for _, p := range ptyForbiddenPrograms { + if base == p { + return true + } + } + return false } var ( @@ -159,6 +197,7 @@ func NewExecToolWithConfig( allowedPathPatterns: allowedPathPatterns, restrictToWorkspace: restrict, allowRemote: allowRemote, + sessionManager: getSessionManager(), }, nil } @@ -167,27 +206,103 @@ func (t *ExecTool) Name() string { } func (t *ExecTool) Description() string { - return "Execute a shell command and return its output. Use with caution." + return `Execute a shell command and return its output. + +Actions: +- run: Execute a command (supports background mode) +- list: List all active sessions +- poll: Check session status +- read: Read output from a session +- write: Send input to a session +- kill: Terminate a session + +Use background=true for long-running commands. Use pty=true for interactive commands (not supported for shell interpreters).` } func (t *ExecTool) Parameters() map[string]any { return map[string]any{ - "type": "object", - "properties": map[string]any{ - "command": map[string]any{ - "type": "string", - "description": "The shell command to execute", + "oneOf": []map[string]any{ + { + "type": "object", + "properties": map[string]any{ + "action": map[string]any{"const": "run"}, + "command": map[string]any{"type": "string", "description": "The shell command to execute"}, + "pty": map[string]any{"type": "boolean", "description": "Use PTY for interactive mode"}, + "background": map[string]any{"type": "boolean", "description": "Run in background mode"}, + "timeout": map[string]any{"type": "integer", "description": "Timeout in seconds (0 = no timeout)"}, + "cwd": map[string]any{"type": "string", "description": "Working directory"}, + }, + "required": []string{"action", "command"}, }, - "working_dir": map[string]any{ - "type": "string", - "description": "Optional working directory for the command", + { + "type": "object", + "properties": map[string]any{ + "action": map[string]any{"const": "list"}, + }, + "required": []string{"action"}, + }, + { + "type": "object", + "properties": map[string]any{ + "action": map[string]any{"const": "poll"}, + "sessionId": map[string]any{"type": "string", "description": "Session ID to poll"}, + }, + "required": []string{"action", "sessionId"}, + }, + { + "type": "object", + "properties": map[string]any{ + "action": map[string]any{"const": "read"}, + "sessionId": map[string]any{"type": "string", "description": "Session ID to read from"}, + }, + "required": []string{"action", "sessionId"}, + }, + { + "type": "object", + "properties": map[string]any{ + "action": map[string]any{"const": "write"}, + "sessionId": map[string]any{"type": "string", "description": "Session ID to write to"}, + "data": map[string]any{"type": "string", "description": "Data to write to the session"}, + }, + "required": []string{"action", "sessionId", "data"}, + }, + { + "type": "object", + "properties": map[string]any{ + "action": map[string]any{"const": "kill"}, + "sessionId": map[string]any{"type": "string", "description": "Session ID to kill"}, + }, + "required": []string{"action", "sessionId"}, }, }, - "required": []string{"command"}, } } func (t *ExecTool) Execute(ctx context.Context, args map[string]any) *ToolResult { + action, _ := args["action"].(string) + if action == "" { + return ErrorResult("action is required") + } + + switch action { + case "run": + return t.executeRun(ctx, args) + case "list": + return t.executeList() + case "poll": + return t.executePoll(args) + case "read": + return t.executeRead(args) + case "write": + return t.executeWrite(args) + case "kill": + return t.executeKill(args) + default: + return ErrorResult(fmt.Sprintf("unknown action: %s", action)) + } +} + +func (t *ExecTool) executeRun(ctx context.Context, args map[string]any) *ToolResult { command, ok := args["command"].(string) if !ok { return ErrorResult("command is required") @@ -206,8 +321,20 @@ func (t *ExecTool) Execute(ctx context.Context, args map[string]any) *ToolResult } } + pty, _ := args["pty"].(bool) + background, _ := args["background"].(bool) + + if pty { + if runtime.GOOS == "windows" { + return ErrorResult("PTY is not supported on Windows. Use background=true without pty.") + } + if isForbiddenInterpreter(command) { + return ErrorResult("PTY is forbidden for interpreter programs (bash, python, node, etc.) for security reasons") + } + } + cwd := t.workingDir - if wd, ok := args["working_dir"].(string); ok && wd != "" { + if wd, ok := args["cwd"].(string); ok && wd != "" { if t.restrictToWorkspace && t.workingDir != "" { resolvedWD, err := validatePathWithAllowPaths(wd, t.workingDir, true, t.allowedPathPatterns) if err != nil { @@ -253,6 +380,14 @@ func (t *ExecTool) Execute(ctx context.Context, args map[string]any) *ToolResult } } + if background { + return t.runBackground(ctx, command, cwd, pty) + } + + return t.runSync(ctx, command, cwd) +} + +func (t *ExecTool) runSync(ctx context.Context, command, cwd string) *ToolResult { // timeout == 0 means no timeout var cmdCtx context.Context var cancel context.CancelFunc @@ -344,6 +479,261 @@ func (t *ExecTool) Execute(ctx context.Context, args map[string]any) *ToolResult } } +func (t *ExecTool) runBackground(ctx context.Context, command, cwd string, ptyEnabled bool) *ToolResult { + sessionID := generateSessionID() + session := &ProcessSession{ + ID: sessionID, + Command: command, + PTY: ptyEnabled, + Background: true, + StartTime: time.Now().Unix(), + Status: "running", + } + + var cmd *exec.Cmd + if runtime.GOOS == "windows" { + cmd = exec.Command("powershell", "-NoProfile", "-NonInteractive", "-Command", command) + } else { + cmd = exec.Command("sh", "-c", command) + } + if cwd != "" { + cmd.Dir = cwd + } + + prepareCommandForTermination(cmd) + + if ptyEnabled { + // Create PTY pair + ptmx, tty, err := pty.Open() + if err != nil { + return ErrorResult(fmt.Sprintf("failed to create PTY: %v", err)) + } + + // Set up the command to use the PTY slave as stdin/stdout/stderr + cmd.Stdin = tty + cmd.Stdout = tty + cmd.Stderr = tty + + // For PTY, we need Setsid to create a new session. + // Note: Setsid and Setpgid conflict, so we must replace SysProcAttr entirely. + cmd.SysProcAttr = &syscall.SysProcAttr{Setsid: true} + + session.ptyMaster = ptmx + } else { + // Non-PTY mode: use pipes + stdoutPipe, err := cmd.StdoutPipe() + if err != nil { + return ErrorResult(fmt.Sprintf("failed to create stdout pipe: %v", err)) + } + stderrPipe, err := cmd.StderrPipe() + if err != nil { + return ErrorResult(fmt.Sprintf("failed to create stderr pipe: %v", err)) + } + stdinPipe, err := cmd.StdinPipe() + if err != nil { + return ErrorResult(fmt.Sprintf("failed to create stdin pipe: %v", err)) + } + + session.stdoutPipe = io.MultiReader(stdoutPipe, stderrPipe) + session.stdinWriter = stdinPipe + } + + if err := cmd.Start(); err != nil { + if session.ptyMaster != nil { + session.ptyMaster.Close() + } + return ErrorResult(fmt.Sprintf("failed to start command: %v", err)) + } + + session.PID = cmd.Process.Pid + t.sessionManager.Add(session) + + go func() { + err := cmd.Wait() + + // Close PTY master when process exits + if session.ptyMaster != nil { + session.ptyMaster.Close() + session.ptyMaster = nil + } + + session.mu.Lock() + if err != nil { + if exitErr, ok := err.(*exec.ExitError); ok { + session.ExitCode = exitErr.ExitCode() + } else { + session.ExitCode = 1 + } + } else { + session.ExitCode = 0 + } + session.Status = "done" + session.mu.Unlock() + }() + + resp := ExecResponse{ + SessionID: sessionID, + Status: "running", + } + data, _ := json.Marshal(resp) + return &ToolResult{ + ForLLM: string(data), + ForUser: fmt.Sprintf("Session %s started", sessionID), + IsError: false, + } +} + +func (t *ExecTool) executeList() *ToolResult { + sessions := t.sessionManager.List() + resp := ExecResponse{ + Sessions: sessions, + } + data, _ := json.Marshal(resp) + return &ToolResult{ + ForLLM: string(data), + ForUser: fmt.Sprintf("%d active sessions", len(sessions)), + IsError: false, + } +} + +func (t *ExecTool) executePoll(args map[string]any) *ToolResult { + sessionID, ok := args["sessionId"].(string) + if !ok { + return ErrorResult("sessionId is required") + } + + session, err := t.sessionManager.Get(sessionID) + if err != nil { + if errors.Is(err, ErrSessionNotFound) { + return ErrorResult(fmt.Sprintf("session not found: %s", sessionID)) + } + return ErrorResult(err.Error()) + } + + resp := ExecResponse{ + SessionID: sessionID, + Status: session.GetStatus(), + ExitCode: session.GetExitCode(), + } + data, _ := json.Marshal(resp) + return &ToolResult{ + ForLLM: string(data), + IsError: false, + } +} + +func (t *ExecTool) executeRead(args map[string]any) *ToolResult { + sessionID, ok := args["sessionId"].(string) + if !ok { + return ErrorResult("sessionId is required") + } + + session, err := t.sessionManager.Get(sessionID) + if err != nil { + if errors.Is(err, ErrSessionNotFound) { + return ErrorResult(fmt.Sprintf("session not found: %s", sessionID)) + } + return ErrorResult(err.Error()) + } + + if session.IsDone() { + return ErrorResult(fmt.Sprintf("process already exited with code %d", session.GetExitCode())) + } + + output, err := session.Read() + if err != nil && !errors.Is(err, ErrSessionDone) { + return ErrorResult(fmt.Sprintf("failed to read from session: %v", err)) + } + + resp := ExecResponse{ + SessionID: sessionID, + Output: output, + Status: session.GetStatus(), + } + data, _ := json.Marshal(resp) + return &ToolResult{ + ForLLM: string(data), + IsError: false, + } +} + +func (t *ExecTool) executeWrite(args map[string]any) *ToolResult { + sessionID, ok := args["sessionId"].(string) + if !ok { + return ErrorResult("sessionId is required") + } + + data, ok := args["data"].(string) + if !ok { + return ErrorResult("data is required") + } + + session, err := t.sessionManager.Get(sessionID) + if err != nil { + if errors.Is(err, ErrSessionNotFound) { + return ErrorResult(fmt.Sprintf("session not found: %s", sessionID)) + } + return ErrorResult(err.Error()) + } + + if session.IsDone() { + return ErrorResult(fmt.Sprintf("process already exited with code %d", session.GetExitCode())) + } + + if err := session.Write(data); err != nil { + if errors.Is(err, ErrSessionDone) { + return ErrorResult(fmt.Sprintf("process already exited with code %d", session.GetExitCode())) + } + return ErrorResult(fmt.Sprintf("failed to write to session: %v", err)) + } + + resp := ExecResponse{ + SessionID: sessionID, + Status: session.GetStatus(), + } + respData, _ := json.Marshal(resp) + return &ToolResult{ + ForLLM: string(respData), + IsError: false, + } +} + +func (t *ExecTool) executeKill(args map[string]any) *ToolResult { + sessionID, ok := args["sessionId"].(string) + if !ok { + return ErrorResult("sessionId is required") + } + + session, err := t.sessionManager.Get(sessionID) + if err != nil { + if errors.Is(err, ErrSessionNotFound) { + return ErrorResult(fmt.Sprintf("session not found: %s", sessionID)) + } + return ErrorResult(err.Error()) + } + + if session.IsDone() { + return ErrorResult(fmt.Sprintf("process already exited with code %d", session.GetExitCode())) + } + + if err := session.Kill(); err != nil { + return ErrorResult(fmt.Sprintf("failed to kill session: %v", err)) + } + + t.sessionManager.Remove(sessionID) + + resp := ExecResponse{ + SessionID: sessionID, + Status: "done", + } + data, _ := json.Marshal(resp) + return &ToolResult{ + ForLLM: string(data), + ForUser: fmt.Sprintf("Session %s killed", sessionID), + IsError: false, + } +} + func (t *ExecTool) guardCommand(command, cwd string) string { cmd := strings.TrimSpace(command) lower := strings.ToLower(cmd) diff --git a/pkg/tools/shell_test.go b/pkg/tools/shell_test.go index c4553020f..c30555a60 100644 --- a/pkg/tools/shell_test.go +++ b/pkg/tools/shell_test.go @@ -2,13 +2,16 @@ package tools import ( "context" + "encoding/json" "os" "path/filepath" + "runtime" "strings" "testing" "time" "github.com/sipeed/picoclaw/pkg/config" + "github.com/stretchr/testify/require" ) // TestShellTool_Success verifies successful command execution @@ -20,6 +23,7 @@ func TestShellTool_Success(t *testing.T) { ctx := context.Background() args := map[string]any{ + "action": "run", "command": "echo 'hello world'", } @@ -50,6 +54,7 @@ func TestShellTool_Failure(t *testing.T) { ctx := context.Background() args := map[string]any{ + "action": "run", "command": "ls /nonexistent_directory_12345", } @@ -82,6 +87,7 @@ func TestShellTool_Timeout(t *testing.T) { ctx := context.Background() args := map[string]any{ + "action": "run", "command": "sleep 10", } @@ -112,8 +118,9 @@ func TestShellTool_WorkingDir(t *testing.T) { ctx := context.Background() args := map[string]any{ - "command": "cat test.txt", - "working_dir": tmpDir, + "action": "run", + "command": "cat test.txt", + "cwd": tmpDir, } result := tool.Execute(ctx, args) @@ -136,6 +143,7 @@ func TestShellTool_DangerousCommand(t *testing.T) { ctx := context.Background() args := map[string]any{ + "action": "run", "command": "rm -rf /", } @@ -159,6 +167,7 @@ func TestShellTool_DangerousCommand_KillBlocked(t *testing.T) { ctx := context.Background() args := map[string]any{ + "action": "run", "command": "kill 12345", } @@ -198,6 +207,7 @@ func TestShellTool_StderrCapture(t *testing.T) { ctx := context.Background() args := map[string]any{ + "action": "run", "command": "sh -c 'echo stdout; echo stderr >&2'", } @@ -222,6 +232,7 @@ func TestShellTool_OutputTruncation(t *testing.T) { ctx := context.Background() // Generate long output (>10000 chars) args := map[string]any{ + "action": "run", "command": "python3 -c \"print('x' * 20000)\" || echo " + strings.Repeat("x", 20000), } @@ -251,8 +262,9 @@ func TestShellTool_WorkingDir_OutsideWorkspace(t *testing.T) { } result := tool.Execute(context.Background(), map[string]any{ - "command": "pwd", - "working_dir": outsideDir, + "action": "run", + "command": "pwd", + "cwd": outsideDir, }) if !result.IsError { @@ -289,8 +301,9 @@ func TestShellTool_WorkingDir_SymlinkEscape(t *testing.T) { } result := tool.Execute(context.Background(), map[string]any{ - "command": "cat secret.txt", - "working_dir": link, + "action": "run", + "command": "cat secret.txt", + "cwd": link, }) if !result.IsError { @@ -312,7 +325,7 @@ func TestShellTool_RemoteChannelBlockedByDefault(t *testing.T) { t.Fatalf("NewExecToolWithConfig() error: %v", err) } ctx := WithToolContext(context.Background(), "telegram", "chat-1") - result := tool.Execute(ctx, map[string]any{"command": "echo hi"}) + result := tool.Execute(ctx, map[string]any{"action": "run", "command": "echo hi"}) if !result.IsError { t.Fatal("expected remote-channel exec to be blocked") @@ -333,7 +346,7 @@ func TestShellTool_InternalChannelAllowed(t *testing.T) { t.Fatalf("NewExecToolWithConfig() error: %v", err) } ctx := WithToolContext(context.Background(), "cli", "direct") - result := tool.Execute(ctx, map[string]any{"command": "echo hi"}) + result := tool.Execute(ctx, map[string]any{"action": "run", "command": "echo hi"}) if result.IsError { t.Fatalf("expected internal channel exec to succeed, got: %s", result.ForLLM) @@ -373,7 +386,7 @@ func TestShellTool_AllowRemoteBypassesChannelCheck(t *testing.T) { t.Fatalf("NewExecToolWithConfig() error: %v", err) } ctx := WithToolContext(context.Background(), "telegram", "chat-1") - result := tool.Execute(ctx, map[string]any{"command": "echo hi"}) + result := tool.Execute(ctx, map[string]any{"action": "run", "command": "echo hi"}) if result.IsError { t.Fatalf("expected allowRemote=true to permit remote channel, got: %s", result.ForLLM) @@ -392,6 +405,7 @@ func TestShellTool_RestrictToWorkspace(t *testing.T) { ctx := context.Background() args := map[string]any{ + "action": "run", "command": "cat ../../etc/passwd", } @@ -429,7 +443,7 @@ func TestShellTool_DevNullAllowed(t *testing.T) { } for _, cmd := range commands { - result := tool.Execute(context.Background(), map[string]any{"command": cmd}) + result := tool.Execute(context.Background(), map[string]any{"action": "run", "command": cmd}) if result.IsError && strings.Contains(result.ForLLM, "blocked") { t.Errorf("command should not be blocked: %s\n error: %s", cmd, result.ForLLM) } @@ -458,7 +472,7 @@ func TestShellTool_BlockDevices(t *testing.T) { } for _, cmd := range blocked { - result := tool.Execute(context.Background(), map[string]any{"command": cmd}) + result := tool.Execute(context.Background(), map[string]any{"action": "run", "command": cmd}) if !result.IsError { t.Errorf("expected block device write to be blocked: %s", cmd) } @@ -482,7 +496,7 @@ func TestShellTool_SafePathsInWorkspaceRestriction(t *testing.T) { } for _, cmd := range commands { - result := tool.Execute(context.Background(), map[string]any{"command": cmd}) + result := tool.Execute(context.Background(), map[string]any{"action": "run", "command": cmd}) if result.IsError && strings.Contains(result.ForLLM, "path outside working dir") { t.Errorf("safe path should not be blocked by workspace check: %s\n error: %s", cmd, result.ForLLM) } @@ -545,7 +559,9 @@ func TestShellTool_URLsNotBlocked(t *testing.T) { } for _, cmd := range commands { - result := tool.Execute(context.Background(), map[string]any{"command": cmd}) + ctx, cancel := context.WithTimeout(context.Background(), 2*time.Second) + result := tool.Execute(ctx, map[string]any{"action": "run", "command": cmd}) + cancel() if result.IsError && strings.Contains(result.ForLLM, "path outside working dir") { t.Errorf("command with URL should not be blocked by workspace check: %s\n error: %s", cmd, result.ForLLM) } @@ -570,7 +586,7 @@ func TestShellTool_FileURISandboxing(t *testing.T) { } for _, cmd := range blockedCommands { - result := tool.Execute(context.Background(), map[string]any{"command": cmd}) + result := tool.Execute(context.Background(), map[string]any{"action": "run", "command": cmd}) if !result.IsError || !strings.Contains(result.ForLLM, "path outside working dir") { t.Errorf("file:// URI outside workspace should be blocked: %s", cmd) } @@ -588,7 +604,7 @@ func TestShellTool_FileURISandboxing(t *testing.T) { } for _, cmd := range allowedCommands { - result := tool.Execute(context.Background(), map[string]any{"command": cmd}) + result := tool.Execute(context.Background(), map[string]any{"action": "run", "command": cmd}) if result.IsError && strings.Contains(result.ForLLM, "path outside working dir") { t.Errorf("file:// URI inside workspace should be allowed: %s\n error: %s", cmd, result.ForLLM) } @@ -614,9 +630,658 @@ func TestShellTool_URLBypassPrevented(t *testing.T) { } for _, cmd := range blockedCommands { - result := tool.Execute(context.Background(), map[string]any{"command": cmd}) + result := tool.Execute(context.Background(), map[string]any{"action": "run", "command": cmd}) if !result.IsError || !strings.Contains(result.ForLLM, "path outside working dir") { t.Errorf("bypass attempt should be blocked: %q\n got: %s", cmd, result.ForLLM) } } } + +func TestShellTool_Background_ReturnsImmediately(t *testing.T) { + tool, err := NewExecTool("", false) + require.NoError(t, err) + + ctx := context.Background() + args := map[string]any{ + "action": "run", + "command": "sleep 5", + "background": true, + } + + start := time.Now() + result := tool.Execute(ctx, args) + elapsed := time.Since(start) + + require.False(t, result.IsError, "background run should not error: %s", result.ForLLM) + require.Less(t, elapsed, time.Second, "background run should return immediately") + require.Contains(t, result.ForLLM, "sessionId") +} + +func TestShellTool_List_Empty(t *testing.T) { + tool, err := NewExecTool("", false) + require.NoError(t, err) + + sm := NewSessionManager() + tool.sessionManager = sm + + ctx := context.Background() + args := map[string]any{"action": "list"} + + result := tool.Execute(ctx, args) + require.False(t, result.IsError) + require.Contains(t, result.ForUser, "0 active sessions") +} + +func TestShellTool_RunBackground_List(t *testing.T) { + tool, err := NewExecTool("", false) + require.NoError(t, err) + + sm := NewSessionManager() + tool.sessionManager = sm + + ctx := WithToolContext(context.Background(), "cli", "test") + + runResult := tool.Execute(ctx, map[string]any{ + "action": "run", + "command": "sleep 10", + "background": true, + }) + require.False(t, runResult.IsError, "run should succeed: %s", runResult.ForLLM) + + var resp ExecResponse + err = json.Unmarshal([]byte(runResult.ForLLM), &resp) + require.NoError(t, err) + require.NotEmpty(t, resp.SessionID) + + time.Sleep(100 * time.Millisecond) + + listResult := tool.Execute(ctx, map[string]any{"action": "list"}) + require.False(t, listResult.IsError) + + var listResp ExecResponse + err = json.Unmarshal([]byte(listResult.ForLLM), &listResp) + require.NoError(t, err) + require.Len(t, listResp.Sessions, 1) + require.Equal(t, resp.SessionID, listResp.Sessions[0].ID) + + killResult := tool.Execute(ctx, map[string]any{ + "action": "kill", + "sessionId": resp.SessionID, + }) + require.False(t, killResult.IsError, "kill should succeed: %s", killResult.ForLLM) +} + +func TestShellTool_Read_Output(t *testing.T) { + tool, err := NewExecTool("", false) + require.NoError(t, err) + + sm := NewSessionManager() + tool.sessionManager = sm + + ctx := WithToolContext(context.Background(), "cli", "test") + + runResult := tool.Execute(ctx, map[string]any{ + "action": "run", + "command": "echo hello", + "background": true, + }) + require.False(t, runResult.IsError) + + var resp ExecResponse + err = json.Unmarshal([]byte(runResult.ForLLM), &resp) + require.NoError(t, err) + + time.Sleep(200 * time.Millisecond) + + readResult := tool.Execute(ctx, map[string]any{ + "action": "read", + "sessionId": resp.SessionID, + }) + + if !readResult.IsError { + var readResp ExecResponse + err = json.Unmarshal([]byte(readResult.ForLLM), &readResp) + require.NoError(t, err) + } +} + +func TestShellTool_Kill(t *testing.T) { + tool, err := NewExecTool("", false) + require.NoError(t, err) + + sm := NewSessionManager() + tool.sessionManager = sm + + ctx := WithToolContext(context.Background(), "cli", "test") + + runResult := tool.Execute(ctx, map[string]any{ + "action": "run", + "command": "sleep 100", + "background": true, + }) + require.False(t, runResult.IsError) + + var resp ExecResponse + err = json.Unmarshal([]byte(runResult.ForLLM), &resp) + require.NoError(t, err) + + killResult := tool.Execute(ctx, map[string]any{ + "action": "kill", + "sessionId": resp.SessionID, + }) + require.False(t, killResult.IsError, "kill should succeed: %s", killResult.ForLLM) + + time.Sleep(100 * time.Millisecond) + + listResult := tool.Execute(ctx, map[string]any{"action": "list"}) + var listResp ExecResponse + err = json.Unmarshal([]byte(listResult.ForLLM), &listResp) + require.NoError(t, err) + require.Len(t, listResp.Sessions, 0) +} + +func TestShellTool_PTY_ForbiddenInterpreters(t *testing.T) { + if runtime.GOOS == "windows" { + t.Skip("PTY not supported on Windows") + } + + tool, err := NewExecTool("", false) + require.NoError(t, err) + + ctx := context.Background() + + for _, cmd := range []string{"python", "bash", "node"} { + result := tool.Execute(ctx, map[string]any{ + "action": "run", + "command": cmd, + "pty": true, + "background": true, + }) + require.True(t, result.IsError, "PTY with %s should be blocked", cmd) + require.Contains(t, result.ForLLM, "PTY is forbidden for interpreter programs") + } +} + +func TestShellTool_PTY_AllowedCommands(t *testing.T) { + if runtime.GOOS == "windows" { + t.Skip("PTY not supported on Windows") + } + + tool, err := NewExecTool("", false) + require.NoError(t, err) + + sm := NewSessionManager() + tool.sessionManager = sm + + ctx := WithToolContext(context.Background(), "cli", "test") + + // Test that PTY is allowed for non-interpreter commands + result := tool.Execute(ctx, map[string]any{ + "action": "run", + "command": "cat", + "pty": true, + "background": true, + }) + require.False(t, result.IsError, "PTY with cat should succeed: %s", result.ForLLM) + require.Contains(t, result.ForLLM, "sessionId") + + var resp ExecResponse + err = json.Unmarshal([]byte(result.ForLLM), &resp) + require.NoError(t, err) + require.NotEmpty(t, resp.SessionID) + + // Clean up + tool.Execute(ctx, map[string]any{ + "action": "kill", + "sessionId": resp.SessionID, + }) +} + +func TestShellTool_PTY_WriteRead(t *testing.T) { + if runtime.GOOS == "windows" { + t.Skip("PTY not supported on Windows") + } + + tool, err := NewExecTool("", false) + require.NoError(t, err) + + sm := NewSessionManager() + tool.sessionManager = sm + + ctx := WithToolContext(context.Background(), "cli", "test") + + // Start a PTY session with a command that waits for input + // Using 'cat' which will wait for stdin + result := tool.Execute(ctx, map[string]any{ + "action": "run", + "command": "cat", + "pty": true, + "background": true, + }) + require.False(t, result.IsError, "PTY run should succeed: %s", result.ForLLM) + + var resp ExecResponse + err = json.Unmarshal([]byte(result.ForLLM), &resp) + require.NoError(t, err) + + // Write some input to cat + writeResult := tool.Execute(ctx, map[string]any{ + "action": "write", + "sessionId": resp.SessionID, + "data": "hello\n", + }) + require.False(t, writeResult.IsError, "write should succeed: %s", writeResult.ForLLM) + + // Give cat time to process and output + time.Sleep(200 * time.Millisecond) + + // Read the output + readResult := tool.Execute(ctx, map[string]any{ + "action": "read", + "sessionId": resp.SessionID, + }) + + require.False(t, readResult.IsError, "read should succeed: %s", readResult.ForLLM) + + var readResp ExecResponse + err = json.Unmarshal([]byte(readResult.ForLLM), &readResp) + require.NoError(t, err) + // PTY output should contain "hello" + require.Contains(t, readResp.Output, "hello") + + // Clean up + tool.Execute(ctx, map[string]any{ + "action": "kill", + "sessionId": resp.SessionID, + }) +} + +func TestShellTool_PTY_Poll(t *testing.T) { + if runtime.GOOS == "windows" { + t.Skip("PTY not supported on Windows") + } + + tool, err := NewExecTool("", false) + require.NoError(t, err) + + sm := NewSessionManager() + tool.sessionManager = sm + + ctx := WithToolContext(context.Background(), "cli", "test") + + // Start a PTY session with a long-running command + result := tool.Execute(ctx, map[string]any{ + "action": "run", + "command": "sleep 2", + "pty": true, + "background": true, + }) + require.False(t, result.IsError, "PTY run should succeed: %s", result.ForLLM) + + var resp ExecResponse + err = json.Unmarshal([]byte(result.ForLLM), &resp) + require.NoError(t, err) + + // Poll should show running + pollResult := tool.Execute(ctx, map[string]any{ + "action": "poll", + "sessionId": resp.SessionID, + }) + require.False(t, pollResult.IsError, "poll should succeed: %s", pollResult.ForLLM) + + var pollResp ExecResponse + err = json.Unmarshal([]byte(pollResult.ForLLM), &pollResp) + require.NoError(t, err) + require.Equal(t, "running", pollResp.Status) + + // Wait for sleep to complete + time.Sleep(2500 * time.Millisecond) + + // Poll should show done + pollResult = tool.Execute(ctx, map[string]any{ + "action": "poll", + "sessionId": resp.SessionID, + }) + require.False(t, pollResult.IsError) + + err = json.Unmarshal([]byte(pollResult.ForLLM), &pollResp) + require.NoError(t, err) + require.Equal(t, "done", pollResp.Status) +} + +func TestShellTool_PTY_Kill(t *testing.T) { + if runtime.GOOS == "windows" { + t.Skip("PTY not supported on Windows") + } + + tool, err := NewExecTool("", false) + require.NoError(t, err) + + sm := NewSessionManager() + tool.sessionManager = sm + + ctx := WithToolContext(context.Background(), "cli", "test") + + // Start a PTY session with a long-running command + result := tool.Execute(ctx, map[string]any{ + "action": "run", + "command": "sleep 10", + "pty": true, + "background": true, + }) + require.False(t, result.IsError, "PTY run should succeed: %s", result.ForLLM) + + var resp ExecResponse + err = json.Unmarshal([]byte(result.ForLLM), &resp) + require.NoError(t, err) + + // Kill the session + killResult := tool.Execute(ctx, map[string]any{ + "action": "kill", + "sessionId": resp.SessionID, + }) + require.False(t, killResult.IsError, "kill should succeed: %s", killResult.ForLLM) + + // Verify kill response shows done status + var killResp ExecResponse + err = json.Unmarshal([]byte(killResult.ForLLM), &killResp) + require.NoError(t, err) + require.Equal(t, "done", killResp.Status) + + // Poll should return error since session is removed after kill + pollResult := tool.Execute(ctx, map[string]any{ + "action": "poll", + "sessionId": resp.SessionID, + }) + // Session is removed after kill, so poll returns error with "session not found" + require.True(t, pollResult.IsError, "poll should error after kill (session removed)") + require.Contains(t, pollResult.ForLLM, "session not found") +} + +func TestShellTool_Write_Read_NonPTY(t *testing.T) { + tool, err := NewExecTool("", false) + require.NoError(t, err) + + sm := NewSessionManager() + tool.sessionManager = sm + + ctx := WithToolContext(context.Background(), "cli", "test") + + // Start a background process that reads from stdin and outputs it + // Using 'cat' which echoes stdin to stdout + result := tool.Execute(ctx, map[string]any{ + "action": "run", + "command": "cat", + "pty": false, + "background": true, + }) + require.False(t, result.IsError, "run should succeed: %s", result.ForLLM) + + var resp ExecResponse + err = json.Unmarshal([]byte(result.ForLLM), &resp) + require.NoError(t, err) + + // Write some input to cat + writeResult := tool.Execute(ctx, map[string]any{ + "action": "write", + "sessionId": resp.SessionID, + "data": "hello world\n", + }) + require.False(t, writeResult.IsError, "write should succeed: %s", writeResult.ForLLM) + + // Give cat time to process and output + time.Sleep(200 * time.Millisecond) + + // Read the output + readResult := tool.Execute(ctx, map[string]any{ + "action": "read", + "sessionId": resp.SessionID, + }) + require.False(t, readResult.IsError, "read should succeed: %s", readResult.ForLLM) + + var readResp ExecResponse + err = json.Unmarshal([]byte(readResult.ForLLM), &readResp) + require.NoError(t, err) + require.Contains(t, readResp.Output, "hello world") + + // Clean up + tool.Execute(ctx, map[string]any{ + "action": "kill", + "sessionId": resp.SessionID, + }) +} + +func TestShellTool_Read_NonPTY_Running(t *testing.T) { + tool, err := NewExecTool("", false) + require.NoError(t, err) + + sm := NewSessionManager() + tool.sessionManager = sm + + ctx := WithToolContext(context.Background(), "cli", "test") + + // Start a long-running process that produces output over time + // Using sh -c with sleep at the end so process doesn't exit immediately + result := tool.Execute(ctx, map[string]any{ + "action": "run", + "command": "sh -c 'echo line1; sleep 0.5; echo line2; sleep 0.5; echo line3; sleep 10'", + "pty": false, + "background": true, + }) + require.False(t, result.IsError, "run should succeed: %s", result.ForLLM) + + var resp ExecResponse + err = json.Unmarshal([]byte(result.ForLLM), &resp) + require.NoError(t, err) + + // Give time for first outputs to be produced + time.Sleep(300 * time.Millisecond) + + // Read output while process is running + readResult := tool.Execute(ctx, map[string]any{ + "action": "read", + "sessionId": resp.SessionID, + }) + require.False(t, readResult.IsError, "read should succeed: %s", readResult.ForLLM) + + var readResp ExecResponse + err = json.Unmarshal([]byte(readResult.ForLLM), &readResp) + require.NoError(t, err) + // Should have at least line1 + require.Contains(t, readResp.Output, "line1") + + // Wait for line3 to be produced (line1=0s, line2=0.5s, line3=1s, then sleep 10) + time.Sleep(1200 * time.Millisecond) + + // Read again - should have line3 as well + readResult = tool.Execute(ctx, map[string]any{ + "action": "read", + "sessionId": resp.SessionID, + }) + require.False(t, readResult.IsError, "read should succeed: %s", readResult.ForLLM) + + err = json.Unmarshal([]byte(readResult.ForLLM), &readResp) + require.NoError(t, err) + require.Contains(t, readResp.Output, "line3") + + // Clean up + tool.Execute(ctx, map[string]any{ + "action": "kill", + "sessionId": resp.SessionID, + }) +} + +func TestShellTool_ProcessGroupKill(t *testing.T) { + if runtime.GOOS == "windows" { + t.Skip("Process group kill not supported on Windows") + } + + // Note: Testing process group kill with PTY is tricky because the command + // must be run through an interpreter (sh, bash) which is blocked for PTY. + // Instead, we test with non-PTY mode which also uses Setsid for background processes. + + tool, err := NewExecTool("", false) + require.NoError(t, err) + + sm := NewSessionManager() + tool.sessionManager = sm + + ctx := WithToolContext(context.Background(), "cli", "test") + + // Start a shell that spawns child processes (non-PTY mode) + // The sh -c command creates child sleep processes + result := tool.Execute(ctx, map[string]any{ + "action": "run", + "command": "sh -c 'sleep 30 & sleep 30 & wait'", + "pty": false, + "background": true, + }) + require.False(t, result.IsError, "run should succeed: %s", result.ForLLM) + + var resp ExecResponse + err = json.Unmarshal([]byte(result.ForLLM), &resp) + require.NoError(t, err) + + // Give time for child processes to spawn + time.Sleep(500 * time.Millisecond) + + // Kill the session - should kill the entire process group + killResult := tool.Execute(ctx, map[string]any{ + "action": "kill", + "sessionId": resp.SessionID, + }) + require.False(t, killResult.IsError, "kill should succeed: %s", killResult.ForLLM) + + // Verify kill response shows done status + var killResp ExecResponse + err = json.Unmarshal([]byte(killResult.ForLLM), &killResp) + require.NoError(t, err) + require.Equal(t, "done", killResp.Status) + + // Poll should return error since session is removed after kill + pollResult := tool.Execute(ctx, map[string]any{ + "action": "poll", + "sessionId": resp.SessionID, + }) + require.True(t, pollResult.IsError, "poll should error after kill (session removed)") + require.Contains(t, pollResult.ForLLM, "session not found") +} + +func TestShellTool_PTY_ProcessGroupKill(t *testing.T) { + if runtime.GOOS == "windows" { + t.Skip("PTY process group kill not supported on Windows") + } + + // This test binary creates 4 child sleep processes and waits for signals. + // It's not an interpreter, so it's allowed with PTY mode. + // The binary is created in /tmp/test_pgroup.c and compiled as part of test setup. + testBinary := "/tmp/test_pgroup" + if _, err := os.Stat(testBinary); os.IsNotExist(err) { + t.Skip("Test binary /tmp/test_pgroup not found - run: gcc -o /tmp/test_pgroup /tmp/test_pgroup.c") + } + + tool, err := NewExecTool("", false) + require.NoError(t, err) + + sm := NewSessionManager() + tool.sessionManager = sm + + ctx := WithToolContext(context.Background(), "cli", "test") + + // Start the test binary with PTY mode + // It forks 4 child sleep processes and waits for signals + result := tool.Execute(ctx, map[string]any{ + "action": "run", + "command": testBinary, + "pty": true, + "background": true, + }) + require.False(t, result.IsError, "run should succeed: %s", result.ForLLM) + + var resp ExecResponse + err = json.Unmarshal([]byte(result.ForLLM), &resp) + require.NoError(t, err) + + // Give time for child processes to spawn + time.Sleep(500 * time.Millisecond) + + // Kill the session - should kill the entire process group + killResult := tool.Execute(ctx, map[string]any{ + "action": "kill", + "sessionId": resp.SessionID, + }) + require.False(t, killResult.IsError, "kill should succeed: %s", killResult.ForLLM) + + // Verify kill response shows done status + var killResp ExecResponse + err = json.Unmarshal([]byte(killResult.ForLLM), &killResp) + require.NoError(t, err) + require.Equal(t, "done", killResp.Status) + + // Poll should return error since session is removed after kill + pollResult := tool.Execute(ctx, map[string]any{ + "action": "poll", + "sessionId": resp.SessionID, + }) + require.True(t, pollResult.IsError, "poll should error after kill (session removed)") + require.Contains(t, pollResult.ForLLM, "session not found") +} + +func TestShellTool_Poll_Status(t *testing.T) { + tool, err := NewExecTool("", false) + require.NoError(t, err) + + sm := NewSessionManager() + tool.sessionManager = sm + + ctx := WithToolContext(context.Background(), "cli", "test") + + runResult := tool.Execute(ctx, map[string]any{ + "action": "run", + "command": "sleep 1", + "background": true, + }) + require.False(t, runResult.IsError) + + var resp ExecResponse + err = json.Unmarshal([]byte(runResult.ForLLM), &resp) + require.NoError(t, err) + + pollResult := tool.Execute(ctx, map[string]any{ + "action": "poll", + "sessionId": resp.SessionID, + }) + require.False(t, pollResult.IsError) + + var pollResp ExecResponse + err = json.Unmarshal([]byte(pollResult.ForLLM), &pollResp) + require.NoError(t, err) + require.Equal(t, "running", pollResp.Status) + + time.Sleep(1200 * time.Millisecond) + + pollResult = tool.Execute(ctx, map[string]any{ + "action": "poll", + "sessionId": resp.SessionID, + }) + require.False(t, pollResult.IsError) + + err = json.Unmarshal([]byte(pollResult.ForLLM), &pollResp) + require.NoError(t, err) + require.Equal(t, "done", pollResp.Status) +} + +func TestShellTool_Action_Run_Sync(t *testing.T) { + tool, err := NewExecTool("", false) + require.NoError(t, err) + + ctx := context.Background() + + result := tool.Execute(ctx, map[string]any{ + "action": "run", + "command": "echo hello", + }) + + require.False(t, result.IsError) + require.Contains(t, result.ForLLM, "hello") +} diff --git a/pkg/tools/shell_timeout_unix_test.go b/pkg/tools/shell_timeout_unix_test.go index 357e1276e..dfd28454c 100644 --- a/pkg/tools/shell_timeout_unix_test.go +++ b/pkg/tools/shell_timeout_unix_test.go @@ -30,6 +30,7 @@ func TestShellTool_TimeoutKillsChildProcess(t *testing.T) { tool.SetTimeout(500 * time.Millisecond) args := map[string]any{ + "action": "run", // Spawn a child process that would outlive the shell unless process-group kill is used. "command": "sleep 60 & echo $! > child.pid; wait", } diff --git a/pkg/tools/types.go b/pkg/tools/types.go index a6015cde3..4d1a18d5a 100644 --- a/pkg/tools/types.go +++ b/pkg/tools/types.go @@ -56,3 +56,24 @@ type ToolFunctionDefinition struct { Description string `json:"description"` Parameters map[string]any `json:"parameters"` } + +type ExecRequest struct { + Action string `json:"action"` + Command string `json:"command,omitempty"` + PTY bool `json:"pty,omitempty"` + Background bool `json:"background,omitempty"` + Timeout int `json:"timeout,omitempty"` + Env map[string]string `json:"env,omitempty"` + Cwd string `json:"cwd,omitempty"` + SessionID string `json:"sessionId,omitempty"` + Data string `json:"data,omitempty"` +} + +type ExecResponse struct { + SessionID string `json:"sessionId,omitempty"` + Status string `json:"status,omitempty"` + ExitCode int `json:"exitCode,omitempty"` + Output string `json:"output,omitempty"` + Error string `json:"error,omitempty"` + Sessions []SessionInfo `json:"sessions,omitempty"` +}