Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
37 changes: 37 additions & 0 deletions cmd/daemon.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
package root

import (
"fmt"
"os"
"os/exec"
)
Comment on lines +3 to +7

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

To use strings.HasPrefix as suggested in another comment for more robust flag parsing, you'll need to import the strings package.

Suggested change
import (
"fmt"
"os"
"os/exec"
)
import (
"fmt"
"os"
"os/exec"
"strings"
)


func prepareDaemonCommand() (*exec.Cmd, error) {
// Get the current executable path
executable, err := os.Executable()
if err != nil {
return nil, fmt.Errorf("failed to get executable path: %w", err)
}

// Get current working directory
workDir, err := os.Getwd()
if err != nil {
return nil, fmt.Errorf("failed to get working directory: %w", err)
}

// Build command arguments, excluding the daemon flag
args := os.Args[1:]
var newArgs []string
for _, arg := range args {
if arg != "--daemon" {
newArgs = append(newArgs, arg)
}
}
Comment on lines +25 to +29

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The current logic for removing the --daemon flag only handles the exact string "--daemon". It doesn't account for forms like --daemon=true, which is also valid for boolean flags in cobra. To make this more robust, it's better to also check for arguments that start with "--daemon=".

Suggested change
for _, arg := range args {
if arg != "--daemon" {
newArgs = append(newArgs, arg)
}
}
for _, arg := range args {
if arg != "--daemon" && !strings.HasPrefix(arg, "--daemon=") {
newArgs = append(newArgs, arg)
}
}


// Create the command
cmd := exec.Command(executable, newArgs...)
cmd.Dir = workDir
cmd.Env = os.Environ()

return cmd, nil
}
33 changes: 33 additions & 0 deletions cmd/daemon_unix.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
//go:build !windows

package root

import (
"fmt"
"os"
"syscall"
)

// daemonize forks the process to run in the background
func daemonize() error {
cmd, err := prepareDaemonCommand()
if err != nil {
return err
}

// Set up process attributes for daemon behavior
cmd.SysProcAttr = &syscall.SysProcAttr{
Setsid: true, // Create new session
}

// Start the process
if err := cmd.Start(); err != nil {
return fmt.Errorf("failed to start daemon process: %w", err)
}

// Exit the parent process
os.Exit(0)

// This line should never be reached, but Go requires it
return nil
}
32 changes: 32 additions & 0 deletions cmd/daemon_windows.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
//go:build windows

package root

import (
"fmt"
"os"
"syscall"
)

// daemonize forks the process to run in the background
func daemonize() error {
cmd, err := prepareDaemonCommand()
if err != nil {
return err
}

// On Windows, use DETACHED_PROCESS flag to run in the background
// without being attached to the parent's console.
cmd.SysProcAttr = &syscall.SysProcAttr{CreationFlags: syscall.DETACHED_PROCESS}

// Start the process
if err := cmd.Start(); err != nil {
return fmt.Errorf("failed to start daemon process: %w", err)
}

// Exit the parent process
os.Exit(0)

// This line should never be reached, but Go requires it
return nil
}
Comment thread
zx9597446 marked this conversation as resolved.
52 changes: 0 additions & 52 deletions cmd/mcp.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ package root
import (
"fmt"
"os"
"os/exec"
"os/signal"
"syscall"

Expand Down Expand Up @@ -111,57 +110,6 @@ and ignore SIGHUP signals.
},
}

// daemonize forks the process to run in the background
func daemonize() error {
// Get the current executable path
executable, err := os.Executable()
if err != nil {
return fmt.Errorf("failed to get executable path: %w", err)
}

// Get current working directory
workDir, err := os.Getwd()
if err != nil {
return fmt.Errorf("failed to get working directory: %w", err)
}

// Build command arguments, excluding the daemon flag
args := os.Args[1:]
var newArgs []string
for i, arg := range args {
if arg == "--daemon" {
// Skip the daemon flag
continue
}
if i > 0 && args[i-1] == "--daemon" {
// Skip the value if it was a separate argument
continue
}
newArgs = append(newArgs, arg)
}

// Create the command
cmd := exec.Command(executable, newArgs...)
cmd.Dir = workDir
cmd.Env = os.Environ()

// Set up process attributes for daemon behavior
cmd.SysProcAttr = &syscall.SysProcAttr{
Setsid: true, // Create new session
}

// Start the process
if err := cmd.Start(); err != nil {
return fmt.Errorf("failed to start daemon process: %w", err)
}

// Exit the parent process
os.Exit(0)

// This line should never be reached, but Go requires it
return nil
}

// setupSIGHUPHandler sets up signal handling to ignore SIGHUP in daemon mode
func setupSIGHUPHandler(logger *common.Logger) {
sigChan := make(chan os.Signal, 1)
Expand Down
22 changes: 12 additions & 10 deletions pkg/command/command_exec.go
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ func (h *CommandHandler) executeToolCommand(ctx context.Context, params map[stri
}

// Wrap command with timeout if configured and timeout command is available
if h.timeout != "" && common.CheckExecutableExists("timeout") {
if h.timeout != "" {
timeoutDuration, err := time.ParseDuration(h.timeout)
if err != nil {
h.logger.Error("Invalid timeout format '%s': %v", h.timeout, err)
Expand All @@ -101,15 +101,17 @@ func (h *CommandHandler) executeToolCommand(ctx context.Context, params map[stri
// Escape single quotes in the command for shell
escapedCmd := strings.ReplaceAll(cmd, "'", "'\"'\"'")

// Wrap the command with timeout
// Using timeout command which will kill the entire process group
// This works reliably on Unix/Linux/macOS systems
cmd = fmt.Sprintf("timeout --kill-after=5s %ds sh -c '%s'", timeoutSeconds, escapedCmd)
h.logger.Debug("Wrapped command with Unix timeout: %ds", timeoutSeconds)
} else if h.timeout != "" {
// timeout command not available (probably Windows)
// Fall back to context-based timeout (less reliable for child processes)
h.logger.Debug("Unix timeout command not available, using context-based timeout: %s", h.timeout)
// On Unix systems, try to use the 'timeout' command if available, otherwise use context-based timeout
// On Windows, always use context-based timeout as 'timeout' command doesn't limit execution time
if shouldUseUnixTimeoutCommand() {
// On Unix/Linux/macOS systems, use timeout command with Unix syntax
cmd = fmt.Sprintf("timeout --kill-after=5s %ds sh -c '%s'", timeoutSeconds, escapedCmd)
h.logger.Debug("Wrapped command with Unix timeout: %ds", timeoutSeconds)
} else {
// timeout command not available on this platform or this is Windows
// Fall back to context-based timeout (less reliable for child processes)
h.logger.Debug("Timeout command not available, using context-based timeout: %s", h.timeout)
}
Comment thread
inercia marked this conversation as resolved.
}

// h.logger.Debug("Processed command: %s", cmd)
Expand Down
29 changes: 29 additions & 0 deletions pkg/command/platform_unix.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
//go:build !windows

package command

import (
"strings"

"github.com/inercia/MCPShell/pkg/common"
)

// getShellCommandArgs returns the correct arguments for different shell types on Unix systems
func getShellCommandArgs(shell string, command string) (string, []string) {
shellLower := strings.ToLower(shell)

// Check if this is a PowerShell (might be available on Unix via PowerShell Core)
if strings.Contains(shellLower, "powershell") ||
strings.HasSuffix(shellLower, "powershell.exe") ||
strings.HasSuffix(shellLower, "pwsh.exe") {
return shell, []string{"-Command", command}
}

// For Unix-like systems and default fallback
return shell, []string{"-c", command}
}

// shouldUseUnixTimeoutCommand returns whether to use the Unix-style timeout command
func shouldUseUnixTimeoutCommand() bool {
return common.CheckExecutableExists("timeout")
}
38 changes: 38 additions & 0 deletions pkg/command/platform_windows.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
//go:build windows

package command

import (
"runtime"
"strings"
)

// getShellCommandArgs returns the correct arguments for different shell types on Windows
func getShellCommandArgs(shell string, command string) (string, []string) {
shellLower := strings.ToLower(shell)

// Check if this is a cmd shell (Windows)
if strings.Contains(shellLower, "cmd") ||
strings.HasSuffix(shellLower, "cmd.exe") ||
(shell == "" && runtime.GOOS == "windows") { // Default to cmd on Windows if no shell specified
return shell, []string{"/c", command}
}

// Check if this is a PowerShell
if strings.Contains(shellLower, "powershell") ||
strings.HasSuffix(shellLower, "powershell.exe") ||
strings.HasSuffix(shellLower, "pwsh.exe") {
return shell, []string{"-Command", command}
}

// For WSL, we might have bash or other Unix shells
// For Unix-like systems and default fallback
return shell, []string{"-c", command}
}

// shouldUseUnixTimeoutCommand returns whether to use the Unix-style timeout command
func shouldUseUnixTimeoutCommand() bool {
// On Windows, we don't use Unix-style timeout command even if a 'timeout' command exists
// because Windows 'timeout' is for pausing, not for limiting execution time
return false
}
Loading