fix: resolve nil pointer dereference panic in shell tool

internal/llm/tools/shell/shell.go

@@ -51,8 +51,21 @@ func GetPersistentShell(workingDir string) *PersistentShell {
 
 	if shellInstance == nil {
 		shellInstance = newPersistentShell(workingDir)
+		if shellInstance == nil {
+			// If we still can't create a shell, return a disabled shell instance
+			return &PersistentShell{
+				isAlive: false,
+				cwd:     workingDir,
+			}
+		}
 	} else if !shellInstance.isAlive {
-		shellInstance = newPersistentShell(shellInstance.cwd)
+		newShell := newPersistentShell(shellInstance.cwd)
+		if newShell != nil {
+			shellInstance = newShell
+		} else {
+			// If we can't recreate the shell, mark it as not alive
+			shellInstance.isAlive = false
+		}
 	}
 
 	return shellInstance
@@ -61,23 +74,23 @@ func GetPersistentShell(workingDir string) *PersistentShell {
 func newPersistentShell(cwd string) *PersistentShell {
 	// Get shell configuration from config
 	cfg := config.Get()
-	
+
 	// Default to environment variable if config is not set or nil
 	var shellPath string
 	var shellArgs []string
-	
+
 	if cfg != nil {
 		shellPath = cfg.Shell.Path
 		shellArgs = cfg.Shell.Args
 	}
-	
+
 	if shellPath == "" {
 		shellPath = os.Getenv("SHELL")
 		if shellPath == "" {
 			shellPath = "/bin/bash"
 		}
 	}
-	
+
 	// Default shell args
 	if len(shellArgs) == 0 {
 		shellArgs = []string{"-l"}
@@ -88,13 +101,15 @@ func newPersistentShell(cwd string) *PersistentShell {
 
 	stdinPipe, err := cmd.StdinPipe()
 	if err != nil {
+		fmt.Fprintf(os.Stderr, "Failed to create stdin pipe for shell: %v\n", err)
 		return nil
 	}
 
 	cmd.Env = append(os.Environ(), "GIT_EDITOR=true")
 
 	err = cmd.Start()
 	if err != nil {
+		fmt.Fprintf(os.Stderr, "Failed to start shell process: %v\n", err)
 		return nil
 	}
 
@@ -174,6 +189,14 @@ echo $EXEC_EXIT_CODE > %s
 		shellQuote(statusFile),
 	)
 
+	if s.stdin == nil {
+		return commandResult{
+			stderr:   "Shell stdin is not available",
+			exitCode: 1,
+			err:      errors.New("shell stdin is not available"),
+		}
+	}
+
 	_, err := s.stdin.Write([]byte(fullCommand + "\n"))
 	if err != nil {
 		return commandResult{
@@ -269,10 +292,20 @@ func (s *PersistentShell) killChildren() {
 }
 
 func (s *PersistentShell) Exec(ctx context.Context, command string, timeoutMs int) (string, string, int, bool, error) {
+	// Safety check for nil shell instance
+	if s == nil {
+		return "", "Shell instance is nil", 1, false, errors.New("shell instance is nil")
+	}
+
 	if !s.isAlive {
 		return "", "Shell is not alive", 1, false, errors.New("shell is not alive")
 	}
 
+	// Safety check for nil commandQueue
+	if s.commandQueue == nil {
+		return "", "Shell command queue is not initialized", 1, false, errors.New("shell command queue is not initialized")
+	}
+
 	timeout := time.Duration(timeoutMs) * time.Millisecond
 
 	resultChan := make(chan commandResult)
@@ -288,16 +321,25 @@ func (s *PersistentShell) Exec(ctx context.Context, command string, timeoutMs in
 }
 
 func (s *PersistentShell) Close() {
+	// Safety check for nil shell instance
+	if s == nil {
+		return
+	}
+
 	s.mu.Lock()
 	defer s.mu.Unlock()
 
 	if !s.isAlive {
 		return
 	}
 
-	s.stdin.Write([]byte("exit\n"))
+	if s.stdin != nil {
+		s.stdin.Write([]byte("exit\n"))
+	}
 
-	s.cmd.Process.Kill()
+	if s.cmd != nil && s.cmd.Process != nil {
+		s.cmd.Process.Kill()
+	}
 	s.isAlive = false
 }
 

internal/llm/tools/shell/shell_test.go

@@ -0,0 +1,126 @@
+package shell
+
+import (
+	"context"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
+
+func TestPersistentShell_NilSafety(t *testing.T) {
+	t.Run("nil shell instance should not panic", func(t *testing.T) {
+		var shell *PersistentShell = nil
+
+		// Test Exec with nil shell
+		stdout, stderr, exitCode, interrupted, err := shell.Exec(context.Background(), "echo test", 1000)
+
+		assert.Equal(t, "", stdout)
+		assert.Equal(t, "Shell instance is nil", stderr)
+		assert.Equal(t, 1, exitCode)
+		assert.False(t, interrupted)
+		assert.Error(t, err)
+		assert.Contains(t, err.Error(), "shell instance is nil")
+	})
+
+	t.Run("nil shell close should not panic", func(t *testing.T) {
+		var shell *PersistentShell = nil
+
+		// This should not panic
+		assert.NotPanics(t, func() {
+			shell.Close()
+		})
+	})
+
+	t.Run("shell with nil commandQueue should not panic", func(t *testing.T) {
+		shell := &PersistentShell{
+			isAlive:      true,
+			cwd:          "/tmp",
+			commandQueue: nil, // Explicitly nil
+		}
+
+		stdout, stderr, exitCode, interrupted, err := shell.Exec(context.Background(), "echo test", 1000)
+
+		assert.Equal(t, "", stdout)
+		assert.Equal(t, "Shell command queue is not initialized", stderr)
+		assert.Equal(t, 1, exitCode)
+		assert.False(t, interrupted)
+		assert.Error(t, err)
+		assert.Contains(t, err.Error(), "shell command queue is not initialized")
+	})
+
+	t.Run("shell with isAlive false should return error", func(t *testing.T) {
+		shell := &PersistentShell{
+			isAlive: false,
+			cwd:     "/tmp",
+		}
+
+		stdout, stderr, exitCode, interrupted, err := shell.Exec(context.Background(), "echo test", 1000)
+
+		assert.Equal(t, "", stdout)
+		assert.Equal(t, "Shell is not alive", stderr)
+		assert.Equal(t, 1, exitCode)
+		assert.False(t, interrupted)
+		assert.Error(t, err)
+		assert.Contains(t, err.Error(), "shell is not alive")
+	})
+}
+
+func TestGetPersistentShell_FailureHandling(t *testing.T) {
+	t.Run("should return disabled shell when creation fails", func(t *testing.T) {
+		// This test is tricky because we can't easily force newPersistentShell to fail
+		// But we can test that GetPersistentShell returns a non-nil shell
+		shell := GetPersistentShell("/tmp")
+
+		require.NotNil(t, shell)
+
+		// The shell should either be alive or disabled, but not nil
+		if !shell.isAlive {
+			// If shell is not alive, it should handle commands gracefully
+			stdout, stderr, exitCode, interrupted, err := shell.Exec(context.Background(), "echo test", 1000)
+
+			assert.Equal(t, "", stdout)
+			assert.Equal(t, "Shell is not alive", stderr)
+			assert.Equal(t, 1, exitCode)
+			assert.False(t, interrupted)
+			assert.Error(t, err)
+		}
+	})
+}
+
+func TestShellQuote(t *testing.T) {
+	tests := []struct {
+		input    string
+		expected string
+	}{
+		{"simple", "'simple'"},
+		{"with spaces", "'with spaces'"},
+		{"with'quote", "'with'\\''quote'"},
+		{"", "''"},
+		{"multiple'quotes'here", "'multiple'\\''quotes'\\''here'"},
+	}
+
+	for _, test := range tests {
+		t.Run(test.input, func(t *testing.T) {
+			result := shellQuote(test.input)
+			assert.Equal(t, test.expected, result)
+		})
+	}
+}
+
+func TestFileHelpers(t *testing.T) {
+	t.Run("fileExists should handle non-existent files", func(t *testing.T) {
+		exists := fileExists("/non/existent/file")
+		assert.False(t, exists)
+	})
+
+	t.Run("fileSize should handle non-existent files", func(t *testing.T) {
+		size := fileSize("/non/existent/file")
+		assert.Equal(t, int64(0), size)
+	})
+
+	t.Run("readFileOrEmpty should handle non-existent files", func(t *testing.T) {
+		content := readFileOrEmpty("/non/existent/file")
+		assert.Equal(t, "", content)
+	})
+}