Detailed changes
@@ -10,7 +10,6 @@ import (
"charm.land/fantasy"
"github.com/charmbracelet/crush/internal/agent/tools"
"github.com/charmbracelet/crush/internal/message"
- "github.com/charmbracelet/crush/internal/shell"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"gopkg.in/dnaeon/go-vcr.v4/pkg/recorder"
@@ -40,7 +39,6 @@ func setupAgent(t *testing.T, pair modelPair) (SessionAgent, env) {
createSimpleGoProject(t, env.workingDir)
agent, err := coderAgent(r, env, large, small)
- shell.Reset(env.workingDir)
require.NoError(t, err)
return agent, env
}
@@ -21,14 +21,14 @@ import (
type BashParams struct {
Command string `json:"command" description:"The command to execute"`
Description string `json:"description,omitempty" description:"A brief description of what the command does"`
- Timeout int `json:"timeout,omitempty" description:"Optional timeout in milliseconds (max 600000)"`
+ WorkingDir string `json:"working_dir,omitempty" description:"The working directory to execute the command in (defaults to current directory)"`
Background bool `json:"background,omitempty" description:"Run the command in a background shell. Returns a shell ID for managing the process."`
}
type BashPermissionsParams struct {
Command string `json:"command"`
Description string `json:"description"`
- Timeout int `json:"timeout"`
+ WorkingDir string `json:"working_dir"`
Background bool `json:"background"`
}
@@ -45,10 +45,9 @@ type BashResponseMetadata struct {
const (
BashToolName = "bash"
- DefaultTimeout = 1 * 60 * 1000 // 1 minutes in milliseconds
- MaxTimeout = 10 * 60 * 1000 // 10 minutes in milliseconds
- MaxOutputLength = 30000
- BashNoOutput = "no output"
+ AutoBackgroundThreshold = 1 * time.Minute // Commands taking longer automatically become background jobs
+ MaxOutputLength = 30000
+ BashNoOutput = "no output"
)
//go:embed bash.tpl
@@ -185,23 +184,20 @@ func blockFuncs() []shell.BlockFunc {
}
func NewBashTool(permissions permission.Service, workingDir string, attribution *config.Attribution) fantasy.AgentTool {
- // Set up command blocking on the persistent shell
- persistentShell := shell.GetPersistentShell(workingDir)
- persistentShell.SetBlockFuncs(blockFuncs())
return fantasy.NewAgentTool(
BashToolName,
string(bashDescription(attribution)),
func(ctx context.Context, params BashParams, call fantasy.ToolCall) (fantasy.ToolResponse, error) {
- if params.Timeout > MaxTimeout {
- params.Timeout = MaxTimeout
- } else if params.Timeout <= 0 {
- params.Timeout = DefaultTimeout
- }
-
if params.Command == "" {
return fantasy.NewTextErrorResponse("missing command"), nil
}
+ // Determine working directory
+ execWorkingDir := workingDir
+ if params.WorkingDir != "" {
+ execWorkingDir = params.WorkingDir
+ }
+
isSafeReadOnly := false
cmdLower := strings.ToLower(params.Command)
@@ -219,25 +215,15 @@ func NewBashTool(permissions permission.Service, workingDir string, attribution
return fantasy.ToolResponse{}, fmt.Errorf("session ID is required for executing shell command")
}
if !isSafeReadOnly {
- var shellDir string
- if params.Background {
- shellDir = workingDir
- } else {
- shellDir = shell.GetPersistentShell(workingDir).GetWorkingDir()
- }
p := permissions.Request(
permission.CreatePermissionRequest{
SessionID: sessionID,
- Path: shellDir,
+ Path: execWorkingDir,
ToolCallID: call.ID,
ToolName: BashToolName,
Action: "execute",
Description: fmt.Sprintf("Execute command: %s", params.Command),
- Params: BashPermissionsParams{
- Command: params.Command,
- Description: params.Description,
- Background: params.Background,
- },
+ Params: BashPermissionsParams(params),
},
)
if !p {
@@ -245,10 +231,11 @@ func NewBashTool(permissions permission.Service, workingDir string, attribution
}
}
+ // If explicitly requested as background, start immediately
if params.Background {
startTime := time.Now()
bgManager := shell.GetBackgroundShellManager()
- bgShell, err := bgManager.Start(ctx, workingDir, blockFuncs(), params.Command)
+ bgShell, err := bgManager.Start(ctx, execWorkingDir, blockFuncs(), params.Command)
if err != nil {
return fantasy.ToolResponse{}, fmt.Errorf("error starting background shell: %w", err)
}
@@ -265,65 +252,110 @@ func NewBashTool(permissions permission.Service, workingDir string, attribution
return fantasy.WithResponseMetadata(fantasy.NewTextResponse(response), metadata), nil
}
+ // Start synchronous execution with auto-background support
startTime := time.Now()
- if params.Timeout > 0 {
- var cancel context.CancelFunc
- ctx, cancel = context.WithTimeout(ctx, time.Duration(params.Timeout)*time.Millisecond)
- defer cancel()
- }
- persistentShell := shell.GetPersistentShell(workingDir)
- stdout, stderr, err := persistentShell.Exec(ctx, params.Command)
+ // Start background shell immediately but wait for threshold before deciding
+ bgManager := shell.GetBackgroundShellManager()
+ bgShell, err := bgManager.Start(ctx, execWorkingDir, blockFuncs(), params.Command)
+ if err != nil {
+ return fantasy.ToolResponse{}, fmt.Errorf("error starting shell: %w", err)
+ }
- currentWorkingDir := persistentShell.GetWorkingDir()
- interrupted := shell.IsInterrupt(err)
- exitCode := shell.ExitCode(err)
- if exitCode == 0 && !interrupted && err != nil {
- return fantasy.ToolResponse{}, fmt.Errorf("error executing command: %w", err)
+ // Wait for either completion, auto-background threshold, or context cancellation
+ ticker := time.NewTicker(100 * time.Millisecond)
+ defer ticker.Stop()
+ timeout := time.After(AutoBackgroundThreshold)
+
+ var stdout, stderr string
+ var done bool
+ var execErr error
+
+ waitLoop:
+ for {
+ select {
+ case <-ticker.C:
+ stdout, stderr, done, execErr = bgShell.GetOutput()
+ if done {
+ break waitLoop
+ }
+ case <-timeout:
+ stdout, stderr, done, execErr = bgShell.GetOutput()
+ break waitLoop
+ case <-ctx.Done():
+ // Context was cancelled, kill the shell and return error
+ bgManager.Kill(bgShell.ID)
+ return fantasy.ToolResponse{}, ctx.Err()
+ }
}
- stdout = truncateOutput(stdout)
- stderr = truncateOutput(stderr)
+ if done {
+ // Command completed within threshold - return synchronously
+ // Remove from background manager since we're returning directly
+ // Don't call Kill() as it cancels the context and corrupts the exit code
+ bgManager.Remove(bgShell.ID)
- errorMessage := stderr
- if errorMessage == "" && err != nil {
- errorMessage = err.Error()
- }
+ interrupted := shell.IsInterrupt(execErr)
+ exitCode := shell.ExitCode(execErr)
+ if exitCode == 0 && !interrupted && execErr != nil {
+ return fantasy.ToolResponse{}, fmt.Errorf("error executing command: %w", execErr)
+ }
- if interrupted {
- if errorMessage != "" {
- errorMessage += "\n"
+ stdout = truncateOutput(stdout)
+ stderr = truncateOutput(stderr)
+
+ errorMessage := stderr
+ if errorMessage == "" && execErr != nil {
+ errorMessage = execErr.Error()
}
- errorMessage += "Command was aborted before completion"
- } else if exitCode != 0 {
- if errorMessage != "" {
- errorMessage += "\n"
+
+ if interrupted {
+ if errorMessage != "" {
+ errorMessage += "\n"
+ }
+ errorMessage += "Command was aborted before completion"
+ } else if exitCode != 0 {
+ if errorMessage != "" {
+ errorMessage += "\n"
+ }
+ errorMessage += fmt.Sprintf("Exit code %d", exitCode)
}
- errorMessage += fmt.Sprintf("Exit code %d", exitCode)
- }
- hasBothOutputs := stdout != "" && stderr != ""
+ hasBothOutputs := stdout != "" && stderr != ""
- if hasBothOutputs {
- stdout += "\n"
- }
+ if hasBothOutputs {
+ stdout += "\n"
+ }
- if errorMessage != "" {
- stdout += "\n" + errorMessage
+ if errorMessage != "" {
+ stdout += "\n" + errorMessage
+ }
+
+ metadata := BashResponseMetadata{
+ StartTime: startTime.UnixMilli(),
+ EndTime: time.Now().UnixMilli(),
+ Output: stdout,
+ Description: params.Description,
+ WorkingDirectory: bgShell.GetWorkingDir(),
+ }
+ if stdout == "" {
+ return fantasy.WithResponseMetadata(fantasy.NewTextResponse(BashNoOutput), metadata), nil
+ }
+ stdout += fmt.Sprintf("\n\n<cwd>%s</cwd>", normalizeWorkingDir(bgShell.GetWorkingDir()))
+ return fantasy.WithResponseMetadata(fantasy.NewTextResponse(stdout), metadata), nil
}
+ // Still running - keep as background job
metadata := BashResponseMetadata{
StartTime: startTime.UnixMilli(),
EndTime: time.Now().UnixMilli(),
- Output: stdout,
Description: params.Description,
- WorkingDirectory: currentWorkingDir,
- }
- if stdout == "" {
- return fantasy.WithResponseMetadata(fantasy.NewTextResponse(BashNoOutput), metadata), nil
+ WorkingDirectory: bgShell.GetWorkingDir(),
+ Background: true,
+ ShellID: bgShell.ID,
}
- stdout += fmt.Sprintf("\n\n<cwd>%s</cwd>", normalizeWorkingDir(currentWorkingDir))
- return fantasy.WithResponseMetadata(fantasy.NewTextResponse(stdout), metadata), nil
+ response := fmt.Sprintf("Command is taking longer than expected and has been moved to background.\n\nBackground shell ID: %s\n\nUse bash_output tool to view output or bash_kill to terminate.", bgShell.ID)
+ return fantasy.WithResponseMetadata(fantasy.NewTextResponse(response), metadata), nil
})
}
@@ -1,4 +1,4 @@
-Executes bash commands in persistent shell session with timeout and security measures.
+Executes bash commands with automatic background conversion for long-running tasks.
<cross_platform>
Uses mvdan/sh interpreter (Bash-compatible on all platforms including Windows).
@@ -10,21 +10,22 @@ Common shell builtins and core utils available on Windows.
1. Directory Verification: If creating directories/files, use LS tool to verify parent exists
2. Security Check: Banned commands ({{ .BannedCommands }}) return error - explain to user. Safe read-only commands execute without prompts
3. Command Execution: Execute with proper quoting, capture output
-4. Output Processing: Truncate if exceeds {{ .MaxOutputLength }} characters
-5. Return Result: Include errors, metadata with <cwd></cwd> tags
+4. Auto-Background: Commands exceeding 1 minute automatically move to background and return shell ID
+5. Output Processing: Truncate if exceeds {{ .MaxOutputLength }} characters
+6. Return Result: Include errors, metadata with <cwd></cwd> tags
</execution_steps>
<usage_notes>
-- Command required, timeout optional (max 600000ms/10min, default 30min if unspecified)
+- Command required, working_dir optional (defaults to current directory)
- IMPORTANT: Use Grep/Glob/Agent tools instead of 'find'/'grep'. Use View/LS tools instead of 'cat'/'head'/'tail'/'ls'
- Chain with ';' or '&&', avoid newlines except in quoted strings
-- Shell state persists (env vars, virtual envs, cwd, etc.) unless running in background
+- Each command runs in independent shell (no state persistence between calls)
- Prefer absolute paths over 'cd' (use 'cd' only if user explicitly requests)
</usage_notes>
<background_execution>
- Set background=true to run commands in a separate background shell
-- Background shells don't share state with the persistent shell
+- Commands taking longer than 1 minute automatically convert to background
- Returns a shell ID for managing the background process
- Use bash_output tool to view current output from background shell
- Use bash_kill tool to terminate a background shell
@@ -355,3 +355,78 @@ func TestBackgroundShell_IDFormat(t *testing.T) {
require.Greater(t, len(bgShell.ID), 5, "ID should be long enough")
require.Less(t, len(bgShell.ID), 100, "ID should not be too long")
}
+
+func TestBackgroundShell_AutoBackground(t *testing.T) {
+ t.Parallel()
+
+ workingDir := t.TempDir()
+ ctx := context.Background()
+
+ // Test that a quick command completes synchronously
+ t.Run("quick command completes synchronously", func(t *testing.T) {
+ t.Parallel()
+ bgManager := shell.GetBackgroundShellManager()
+ bgShell, err := bgManager.Start(ctx, workingDir, nil, "echo 'quick'")
+ require.NoError(t, err)
+
+ // Wait threshold time
+ time.Sleep(5 * time.Second)
+
+ // Should be done by now
+ stdout, stderr, done, err := bgShell.GetOutput()
+ require.NoError(t, err)
+ require.True(t, done, "Quick command should be done")
+ require.Contains(t, stdout, "quick")
+ require.Empty(t, stderr)
+
+ // Clean up
+ bgManager.Kill(bgShell.ID)
+ })
+
+ // Test that a long command stays in background
+ t.Run("long command stays in background", func(t *testing.T) {
+ t.Parallel()
+ bgManager := shell.GetBackgroundShellManager()
+ bgShell, err := bgManager.Start(ctx, workingDir, nil, "sleep 20 && echo '20 seconds completed'")
+ require.NoError(t, err)
+ defer bgManager.Kill(bgShell.ID)
+
+ // Wait threshold time
+ time.Sleep(5 * time.Second)
+
+ // Should still be running
+ stdout, stderr, done, err := bgShell.GetOutput()
+ require.NoError(t, err)
+ require.False(t, done, "Long command should still be running")
+ require.Empty(t, stdout, "No output yet from sleep command")
+ require.Empty(t, stderr)
+
+ // Verify we can get the shell from manager
+ retrieved, ok := bgManager.Get(bgShell.ID)
+ require.True(t, ok, "Should be able to retrieve background shell")
+ require.Equal(t, bgShell.ID, retrieved.ID)
+ })
+
+ // Test that we can check output of long-running command later
+ t.Run("can check output after completion", func(t *testing.T) {
+ t.Parallel()
+ bgManager := shell.GetBackgroundShellManager()
+ bgShell, err := bgManager.Start(ctx, workingDir, nil, "sleep 3 && echo 'completed'")
+ require.NoError(t, err)
+ defer bgManager.Kill(bgShell.ID)
+
+ // Initially should be running
+ _, _, done, _ := bgShell.GetOutput()
+ require.False(t, done, "Should be running initially")
+
+ // Wait for completion
+ time.Sleep(4 * time.Second)
+
+ // Now should be done
+ stdout, stderr, done, err := bgShell.GetOutput()
+ require.NoError(t, err)
+ require.True(t, done, "Should be done after waiting")
+ require.Contains(t, stdout, "completed")
+ require.Empty(t, stderr)
+ })
+}
@@ -13,15 +13,14 @@ import (
// BackgroundShell represents a shell running in the background.
type BackgroundShell struct {
- ID string
- Shell *Shell
- ctx context.Context
- cancel context.CancelFunc
- stdout *bytes.Buffer
- stderr *bytes.Buffer
- done chan struct{}
- exitErr error
- workingDir string
+ ID string
+ Shell *Shell
+ ctx context.Context
+ cancel context.CancelFunc
+ stdout *bytes.Buffer
+ stderr *bytes.Buffer
+ done chan struct{}
+ exitErr error
}
// BackgroundShellManager manages background shell instances.
@@ -56,14 +55,13 @@ func (m *BackgroundShellManager) Start(ctx context.Context, workingDir string, b
shellCtx, cancel := context.WithCancel(ctx)
bgShell := &BackgroundShell{
- ID: id,
- Shell: shell,
- ctx: shellCtx,
- cancel: cancel,
- stdout: &bytes.Buffer{},
- stderr: &bytes.Buffer{},
- done: make(chan struct{}),
- workingDir: workingDir,
+ ID: id,
+ Shell: shell,
+ ctx: shellCtx,
+ cancel: cancel,
+ stdout: &bytes.Buffer{},
+ stderr: &bytes.Buffer{},
+ done: make(chan struct{}),
}
m.shells.Set(id, bgShell)
@@ -86,6 +84,16 @@ func (m *BackgroundShellManager) Get(id string) (*BackgroundShell, bool) {
return m.shells.Get(id)
}
+// Remove removes a background shell from the manager without terminating it.
+// This is useful when a shell has already completed and you just want to clean up tracking.
+func (m *BackgroundShellManager) Remove(id string) error {
+ _, ok := m.shells.Take(id)
+ if !ok {
+ return fmt.Errorf("background shell not found: %s", id)
+ }
+ return nil
+}
+
// Kill terminates a background shell by ID.
func (m *BackgroundShellManager) Kill(id string) error {
shell, ok := m.shells.Take(id)
@@ -148,5 +156,5 @@ func (bs *BackgroundShell) Wait() {
// GetWorkingDir returns the current working directory of the background shell.
func (bs *BackgroundShell) GetWorkingDir() string {
- return bs.workingDir
+ return bs.Shell.GetWorkingDir()
}
@@ -16,12 +16,7 @@ package shell
// shell.Exec(ctx, "export FOO=bar")
// shell.Exec(ctx, "echo $FOO") // Will print "bar"
//
-// 3. For the singleton persistent shell (used by tools):
-//
-// shell := shell.GetPersistentShell("/path/to/cwd")
-// stdout, stderr, err := shell.Exec(ctx, "ls -la")
-//
-// 4. Managing environment and working directory:
+// 3. Managing environment and working directory:
//
// shell := shell.NewShell(nil)
// shell.SetEnv("MY_VAR", "value")
@@ -1,43 +0,0 @@
-package shell
-
-import (
- "log/slog"
- "sync"
-)
-
-// PersistentShell is a singleton shell instance that maintains state across the application
-type PersistentShell struct {
- *Shell
-}
-
-var (
- once sync.Once
- shellInstance *PersistentShell
-)
-
-// GetPersistentShell returns the singleton persistent shell instance
-// This maintains backward compatibility with the existing API
-func GetPersistentShell(cwd string) *PersistentShell {
- once.Do(func() {
- shellInstance = &PersistentShell{
- Shell: NewShell(&Options{
- WorkingDir: cwd,
- Logger: &loggingAdapter{},
- }),
- }
- })
- return shellInstance
-}
-
-// INFO: only used for tests
-func Reset(cwd string) {
- once = sync.Once{}
- _ = GetPersistentShell(cwd)
-}
-
-// slog.dapter adapts the internal slog.package to the Logger interface
-type loggingAdapter struct{}
-
-func (l *loggingAdapter) InfoPersist(msg string, keysAndValues ...any) {
- slog.Info(msg, keysAndValues...)
-}
@@ -1,13 +1,12 @@
// Package shell provides cross-platform shell execution capabilities.
//
-// This package offers two main types:
-// - Shell: A general-purpose shell executor for one-off or managed commands
-// - PersistentShell: A singleton shell that maintains state across the application
+// This package provides Shell instances for executing commands with their own
+// working directory and environment. Each shell execution is independent.
//
// WINDOWS COMPATIBILITY:
-// This implementation provides both POSIX shell emulation (mvdan.cc/sh/v3),
-// even on Windows. Some caution has to be taken: commands should have forward
-// slashes (/) as path separators to work, even on Windows.
+// This implementation provides POSIX shell emulation (mvdan.cc/sh/v3) even on
+// Windows. Commands should use forward slashes (/) as path separators to work
+// correctly on all platforms.
package shell
import (