From c691a0165d4f460ff82f7da356af5df8c59c7997 Mon Sep 17 00:00:00 2001 From: kujtimiihoxha Date: Wed, 29 Oct 2025 13:44:20 +0100 Subject: [PATCH] chore: small fixes & killall on shutdown --- internal/agent/tools/bash_kill.go | 26 +++--------- internal/agent/tools/bash_kill.md | 18 +++++++++ internal/agent/tools/bash_output.go | 27 +++---------- internal/agent/tools/bash_output.md | 19 +++++++++ internal/app/app.go | 4 ++ internal/shell/background.go | 16 ++++++++ internal/shell/background_test.go | 62 +++++++++++++++++++++++++++++ 7 files changed, 129 insertions(+), 43 deletions(-) create mode 100644 internal/agent/tools/bash_kill.md create mode 100644 internal/agent/tools/bash_output.md diff --git a/internal/agent/tools/bash_kill.go b/internal/agent/tools/bash_kill.go index e89d75655abdb8bb29d3c2a77520cf349e0c0b5e..f8a6a2cce154eca6df87c3ee9f5ef71caae5809a 100644 --- a/internal/agent/tools/bash_kill.go +++ b/internal/agent/tools/bash_kill.go @@ -2,6 +2,7 @@ package tools import ( "context" + _ "embed" "fmt" "charm.land/fantasy" @@ -12,6 +13,9 @@ const ( BashKillToolName = "bash_kill" ) +//go:embed bash_kill.md +var bashKillDescription []byte + type BashKillParams struct { ShellID string `json:"shell_id" description:"The ID of the background shell to terminate"` } @@ -20,30 +24,10 @@ type BashKillResponseMetadata struct { ShellID string `json:"shell_id"` } -const bashKillDescription = `Terminates a background shell process. - - -- Provide the shell ID returned from a background bash execution -- Cancels the running process and cleans up resources - - - -- Stop long-running background processes -- Clean up completed background shells -- Immediately terminates the process - - - -- Use this when you need to stop a background process -- The process is terminated immediately (similar to SIGTERM) -- After killing, the shell ID becomes invalid - -` - func NewBashKillTool() fantasy.AgentTool { return fantasy.NewAgentTool( BashKillToolName, - bashKillDescription, + string(bashKillDescription), func(ctx context.Context, params BashKillParams, call fantasy.ToolCall) (fantasy.ToolResponse, error) { if params.ShellID == "" { return fantasy.NewTextErrorResponse("missing shell_id"), nil diff --git a/internal/agent/tools/bash_kill.md b/internal/agent/tools/bash_kill.md new file mode 100644 index 0000000000000000000000000000000000000000..5a929ab36fc6be3bf4dec5d8fe61f88677d2469a --- /dev/null +++ b/internal/agent/tools/bash_kill.md @@ -0,0 +1,18 @@ +Terminates a background shell process. + + +- Provide the shell ID returned from a background bash execution +- Cancels the running process and cleans up resources + + + +- Stop long-running background processes +- Clean up completed background shells +- Immediately terminates the process + + + +- Use this when you need to stop a background process +- The process is terminated immediately (similar to SIGTERM) +- After killing, the shell ID becomes invalid + diff --git a/internal/agent/tools/bash_output.go b/internal/agent/tools/bash_output.go index 6fd6551ff9ed0f51dd98c243a89dc2aab5cd626b..448b5516e7532a9c6b58691cedff05633d2cac1a 100644 --- a/internal/agent/tools/bash_output.go +++ b/internal/agent/tools/bash_output.go @@ -2,6 +2,7 @@ package tools import ( "context" + _ "embed" "fmt" "strings" @@ -13,6 +14,9 @@ const ( BashOutputToolName = "bash_output" ) +//go:embed bash_output.md +var bashOutputDescription []byte + type BashOutputParams struct { ShellID string `json:"shell_id" description:"The ID of the background shell to retrieve output from"` } @@ -23,31 +27,10 @@ type BashOutputResponseMetadata struct { WorkingDirectory string `json:"working_directory"` } -const bashOutputDescription = `Retrieves the current output from a background shell. - - -- Provide the shell ID returned from a background bash execution -- Returns the current stdout and stderr output -- Indicates whether the shell has completed execution - - - -- View output from running background processes -- Check if background process has completed -- Get cumulative output from process start - - - -- Use this to monitor long-running processes -- Check the 'done' status to see if process completed -- Can be called multiple times to view incremental output - -` - func NewBashOutputTool() fantasy.AgentTool { return fantasy.NewAgentTool( BashOutputToolName, - bashOutputDescription, + string(bashOutputDescription), func(ctx context.Context, params BashOutputParams, call fantasy.ToolCall) (fantasy.ToolResponse, error) { if params.ShellID == "" { return fantasy.NewTextErrorResponse("missing shell_id"), nil diff --git a/internal/agent/tools/bash_output.md b/internal/agent/tools/bash_output.md new file mode 100644 index 0000000000000000000000000000000000000000..460496ccb4a04a36606b5a25252187feeb2c8aae --- /dev/null +++ b/internal/agent/tools/bash_output.md @@ -0,0 +1,19 @@ +Retrieves the current output from a background shell. + + +- Provide the shell ID returned from a background bash execution +- Returns the current stdout and stderr output +- Indicates whether the shell has completed execution + + + +- View output from running background processes +- Check if background process has completed +- Get cumulative output from process start + + + +- Use this to monitor long-running processes +- Check the 'done' status to see if process completed +- Can be called multiple times to view incremental output + diff --git a/internal/app/app.go b/internal/app/app.go index a801f70a5feccac655209ac5c36deaae7e38592b..4c190ed32edb4416b4c142316815b8c28a8efe38 100644 --- a/internal/app/app.go +++ b/internal/app/app.go @@ -24,6 +24,7 @@ import ( "github.com/charmbracelet/crush/internal/permission" "github.com/charmbracelet/crush/internal/pubsub" "github.com/charmbracelet/crush/internal/session" + "github.com/charmbracelet/crush/internal/shell" "github.com/charmbracelet/x/ansi" ) @@ -325,6 +326,9 @@ func (app *App) Shutdown() { app.AgentCoordinator.CancelAll() } + // Kill all background shells. + shell.GetBackgroundShellManager().KillAll() + // Shutdown all LSP clients. for name, client := range app.LSPClients.Seq2() { shutdownCtx, cancel := context.WithTimeout(app.globalCtx, 5*time.Second) diff --git a/internal/shell/background.go b/internal/shell/background.go index d68567aba74e11c8418374acbd19efeb49d65fe1..d6b51447096dccd53a597b817f7b160b3bd6cf33 100644 --- a/internal/shell/background.go +++ b/internal/shell/background.go @@ -121,6 +121,22 @@ func (m *BackgroundShellManager) List() []string { return ids } +// KillAll terminates all background shells. +func (m *BackgroundShellManager) KillAll() { + m.mu.Lock() + shells := make([]*BackgroundShell, 0, len(m.shells)) + for _, shell := range m.shells { + shells = append(shells, shell) + } + m.shells = make(map[string]*BackgroundShell) + m.mu.Unlock() + + for _, shell := range shells { + shell.cancel() + <-shell.done + } +} + // GetOutput returns the current output of a background shell. func (bs *BackgroundShell) GetOutput() (stdout string, stderr string, done bool, err error) { bs.mu.RLock() diff --git a/internal/shell/background_test.go b/internal/shell/background_test.go index 715164d6a436c613a7f3e520d133f8022c81c3fc..ca8f9cf3765d3c907b26d8b92e83b7056f86f923 100644 --- a/internal/shell/background_test.go +++ b/internal/shell/background_test.go @@ -212,3 +212,65 @@ func TestBackgroundShellManager_List(t *testing.T) { manager.Kill(bgShell1.ID) manager.Kill(bgShell2.ID) } + +func TestBackgroundShellManager_KillAll(t *testing.T) { + t.Parallel() + + ctx := context.Background() + workingDir := t.TempDir() + manager := GetBackgroundShellManager() + + // Start multiple long-running shells + shell1, err := manager.Start(ctx, workingDir, nil, "sleep 10") + if err != nil { + t.Fatalf("failed to start shell 1: %v", err) + } + + shell2, err := manager.Start(ctx, workingDir, nil, "sleep 10") + if err != nil { + t.Fatalf("failed to start shell 2: %v", err) + } + + shell3, err := manager.Start(ctx, workingDir, nil, "sleep 10") + if err != nil { + t.Fatalf("failed to start shell 3: %v", err) + } + + // Verify shells are running + if shell1.IsDone() || shell2.IsDone() || shell3.IsDone() { + t.Error("shells should not be done yet") + } + + // Kill all shells + manager.KillAll() + + // Verify all shells are done + if !shell1.IsDone() { + t.Error("shell1 should be done after KillAll") + } + if !shell2.IsDone() { + t.Error("shell2 should be done after KillAll") + } + if !shell3.IsDone() { + t.Error("shell3 should be done after KillAll") + } + + // Verify they're removed from the manager + if _, ok := manager.Get(shell1.ID); ok { + t.Error("shell1 should be removed from manager") + } + if _, ok := manager.Get(shell2.ID); ok { + t.Error("shell2 should be removed from manager") + } + if _, ok := manager.Get(shell3.ID); ok { + t.Error("shell3 should be removed from manager") + } + + // Verify list is empty (or doesn't contain our shells) + ids := manager.List() + for _, id := range ids { + if id == shell1.ID || id == shell2.ID || id == shell3.ID { + t.Errorf("shell %s should not be in list after KillAll", id) + } + } +}