shelley: use login shell to source user's PATH and environment

Philip Zeyliger and Shelley created 2 months ago

Prompt: In a new worktree, fix https://github.com/boldsoftware/shelley/issues/72 ; we should try to respect the user's shell settings at least if it's bash.

Run bash commands via 'bash --login -c' instead of 'bash -c' so that
the user's shell configuration (~/.profile, ~/.bash_profile) is sourced.

This allows Shelley to access CLI tools that users have set up with
custom PATH entries (~/.local/bin), mise, fnox, and other shell
configuration.

Fixes https://github.com/boldsoftware/shelley/issues/72

Co-authored-by: Shelley <[e-mail]>

Change summary

claudetool/bash.go      | 10 ++++------
claudetool/bash_test.go | 40 ++++++++++++++++++++--------------------
2 files changed, 24 insertions(+), 26 deletions(-)

Detailed changes

claudetool/bash.go

@@ -107,7 +107,7 @@ func (b *BashTool) isNoTrailerSet() bool {
 
 const (
 	bashName        = "bash"
-	bashDescription = `Executes shell commands via bash -c, returning combined stdout/stderr.
+	bashDescription = `Executes shell commands via bash --login -c, returning combined stdout/stderr.
 Bash state changes (working dir, variables, aliases) don't persist between calls.
 
 With background=true, returns immediately, with output redirected to a file.
@@ -250,7 +250,7 @@ const (
 )
 
 func (b *BashTool) makeBashCommand(ctx context.Context, command string, out io.Writer) *exec.Cmd {
-	cmd := exec.CommandContext(ctx, "bash", "-c", command)
+	cmd := exec.CommandContext(ctx, "bash", "--login", "-c", command)
 	// Use shared WorkingDir if available, then context, then Pwd fallback
 	cmd.Dir = b.getWorkingDir()
 	cmd.Stdin = nil
@@ -267,11 +267,9 @@ func (b *BashTool) makeBashCommand(ctx context.Context, command string, out io.W
 		return syscall.Kill(-cmd.Process.Pid, syscall.SIGKILL) // kill entire process group
 	}
 	cmd.WaitDelay = 15 * time.Second // prevent indefinite hangs when child processes keep pipes open
-	// Remove SKETCH_MODEL_URL, SKETCH_PUB_KEY, SKETCH_MODEL_API_KEY,
-	// and any other future SKETCH_ goodies from the environment.
-	// ...except for SKETCH_PROXY_ID, which is intentionally available.
+	// Remove SHELLEY_CONVERSATION_ID so we control it explicitly below.
 	env := slices.DeleteFunc(os.Environ(), func(s string) bool {
-		return strings.HasPrefix(s, "SKETCH_") && s != "SKETCH_PROXY_ID"
+		return strings.HasPrefix(s, "SHELLEY_CONVERSATION_ID=")
 	})
 	env = append(env, "SKETCH=1")          // signal that this has been run by Sketch, sometimes useful for scripts
 	env = append(env, "EDITOR=/bin/false") // interactive editors won't work

claudetool/bash_test.go

@@ -207,23 +207,6 @@ func TestExecuteBash(t *testing.T) {
 		}
 	})
 
-	// Test SKETCH=1 environment variable is set
-	t.Run("SKETCH Environment Variable", func(t *testing.T) {
-		req := bashInput{
-			Command: "echo $SKETCH",
-		}
-
-		output, err := bashTool.executeBash(ctx, req, 5*time.Second)
-		if err != nil {
-			t.Fatalf("Unexpected error: %v", err)
-		}
-
-		want := "1\n"
-		if output != want {
-			t.Errorf("Expected SKETCH=1, got %q", output)
-		}
-	})
-
 	// Test SHELLEY_CONVERSATION_ID environment variable is set when configured
 	t.Run("SHELLEY_CONVERSATION_ID Environment Variable", func(t *testing.T) {
 		bashWithConvID := &BashTool{
@@ -263,6 +246,23 @@ func TestExecuteBash(t *testing.T) {
 		}
 	})
 
+	// Test that bash runs as a login shell (sources user profile)
+	t.Run("Login Shell", func(t *testing.T) {
+		req := bashInput{
+			Command: "shopt login_shell | grep -q on && echo login",
+		}
+
+		output, err := bashTool.executeBash(ctx, req, 5*time.Second)
+		if err != nil {
+			t.Fatalf("Unexpected error: %v", err)
+		}
+
+		want := "login\n"
+		if output != want {
+			t.Errorf("Expected bash to run as login shell, got %q", output)
+		}
+	})
+
 	// Test command with output to stderr
 	t.Run("Command with stderr", func(t *testing.T) {
 		req := bashInput{
@@ -327,7 +327,7 @@ func TestBackgroundBash(t *testing.T) {
 			Command    string `json:"command"`
 			Background bool   `json:"background"`
 		}{
-			Command:    "echo 'Hello from background' $SKETCH",
+			Command:    "echo 'Hello from background'",
 			Background: true,
 		}
 		inputJSON, err := json.Marshal(inputObj)
@@ -387,8 +387,8 @@ func TestBackgroundBash(t *testing.T) {
 		}
 		// The implementation appends a completion message to the output
 		outputStr := string(outputContent)
-		if !strings.Contains(outputStr, "Hello from background 1") {
-			t.Errorf("Expected output to contain 'Hello from background 1', got %q", outputStr)
+		if !strings.Contains(outputStr, "Hello from background") {
+			t.Errorf("Expected output to contain 'Hello from background', got %q", outputStr)
 		}
 		if !strings.Contains(outputStr, "[background process completed]") {
 			t.Errorf("Expected output to contain completion message, got %q", outputStr)