From 1ad34859239311c5b9d0cd7f193cfae9d230b099 Mon Sep 17 00:00:00 2001 From: Carlos Alexandro Becker Date: Fri, 20 Jun 2025 10:07:22 -0300 Subject: [PATCH 1/3] fix: small improvements around command execution --- internal/fsext/fileutil.go | 20 +++++++++++++++----- internal/llm/tools/glob.go | 2 +- internal/llm/tools/grep.go | 14 +++----------- internal/llm/tools/shell/shell.go | 18 ++++++------------ 4 files changed, 25 insertions(+), 29 deletions(-) diff --git a/internal/fsext/fileutil.go b/internal/fsext/fileutil.go index a401e014ad3b85b91a6cdf1e0c3fa7f1b9420f74..8230e3a3c2f965bef22131678b146173d81935a1 100644 --- a/internal/fsext/fileutil.go +++ b/internal/fsext/fileutil.go @@ -25,12 +25,10 @@ func init() { rgPath, err = exec.LookPath("rg") if err != nil { logging.Warn("Ripgrep (rg) not found in $PATH. Some features might be limited or slower.") - rgPath = "" } fzfPath, err = exec.LookPath("fzf") if err != nil { logging.Warn("FZF not found in $PATH. Some features might be limited or slower.") - fzfPath = "" } } @@ -49,9 +47,21 @@ func GetRgCmd(globPattern string) *exec.Cmd { } rgArgs = append(rgArgs, "--glob", globPattern) } - cmd := exec.Command(rgPath, rgArgs...) - cmd.Dir = "." - return cmd + return exec.Command(rgPath, rgArgs...) +} + +func GetRgSearchCmd(pattern, path, include string) *exec.Cmd { + if rgPath == "" { + return nil + } + // Use -n to show line numbers and include the matched line + args := []string{"-n", pattern} + if include != "" { + args = append(args, "--glob", include) + } + args = append(args, path) + + return exec.Command("rg", args...) } type FileInfo struct { diff --git a/internal/llm/tools/glob.go b/internal/llm/tools/glob.go index f01620b38304a9bf3bce52a6941f8f6bf639d827..3a0adf0581b3ba75f4bd86e337ab355323a4f3e5 100644 --- a/internal/llm/tools/glob.go +++ b/internal/llm/tools/glob.go @@ -150,7 +150,7 @@ func runRipgrep(cmd *exec.Cmd, searchRoot string, limit int) ([]string, error) { } var matches []string - for _, p := range bytes.Split(out, []byte{0}) { + for p := range bytes.SplitSeq(out, []byte{0}) { if len(p) == 0 { continue } diff --git a/internal/llm/tools/grep.go b/internal/llm/tools/grep.go index b02e77469f164f2b2706272bc60e6a7d6869e608..b004e47fe5f13caa7bc22be304fd074b19658471 100644 --- a/internal/llm/tools/grep.go +++ b/internal/llm/tools/grep.go @@ -257,19 +257,11 @@ func searchFiles(pattern, rootPath, include string, limit int) ([]grepMatch, boo } func searchWithRipgrep(pattern, path, include string) ([]grepMatch, error) { - _, err := exec.LookPath("rg") - if err != nil { - return nil, fmt.Errorf("ripgrep not found: %w", err) - } - - // Use -n to show line numbers and include the matched line - args := []string{"-n", pattern} - if include != "" { - args = append(args, "--glob", include) + cmd := fsext.GetRgSearchCmd(pattern, path, include) + if cmd == nil { + return nil, fmt.Errorf("ripgrep not found in $PATH") } - args = append(args, path) - cmd := exec.Command("rg", args...) output, err := cmd.Output() if err != nil { if exitErr, ok := err.(*exec.ExitError); ok && exitErr.ExitCode() == 1 { diff --git a/internal/llm/tools/shell/shell.go b/internal/llm/tools/shell/shell.go index 6c94904c9584c8d1500130196fef10cad202620d..38c3021a89a33579f97ea2610a824ba0f5b40dfb 100644 --- a/internal/llm/tools/shell/shell.go +++ b/internal/llm/tools/shell/shell.go @@ -4,6 +4,7 @@ import ( "context" "errors" "fmt" + "io" "os" "os/exec" "path/filepath" @@ -17,7 +18,7 @@ import ( type PersistentShell struct { cmd *exec.Cmd - stdin *os.File + stdin io.WriteCloser isAlive bool cwd string mu sync.Mutex @@ -39,22 +40,15 @@ type commandResult struct { err error } -var ( - shellInstance *PersistentShell - shellInstanceOnce sync.Once -) +var shellInstance *PersistentShell func GetPersistentShell(workingDir string) *PersistentShell { - shellInstanceOnce.Do(func() { - shellInstance = newPersistentShell(workingDir) - }) - if shellInstance == nil { shellInstance = newPersistentShell(workingDir) - } else if !shellInstance.isAlive { + } + if !shellInstance.isAlive { shellInstance = newPersistentShell(shellInstance.cwd) } - return shellInstance } @@ -100,7 +94,7 @@ func newPersistentShell(cwd string) *PersistentShell { shell := &PersistentShell{ cmd: cmd, - stdin: stdinPipe.(*os.File), + stdin: stdinPipe, isAlive: true, cwd: cwd, commandQueue: make(chan *commandExecution, 10), From 97a73a9dd418bc14a33750065157a0b6efebc16a Mon Sep 17 00:00:00 2001 From: Carlos Alexandro Becker Date: Fri, 20 Jun 2025 10:29:37 -0300 Subject: [PATCH 2/3] fix: improve killing shell children processes --- go.mod | 10 +++++++++- go.sum | 21 ++++++++++++++++++++ internal/fsext/fileutil.go | 4 ++-- internal/llm/tools/shell/shell.go | 33 +++++++++++++++---------------- 4 files changed, 48 insertions(+), 20 deletions(-) diff --git a/go.mod b/go.mod index f5b9b0ef945822cf386f62767ead9079787524ea..6e39841d125c18db69d77c9ab0bf88222c348206 100644 --- a/go.mod +++ b/go.mod @@ -32,7 +32,7 @@ require ( github.com/pressly/goose/v3 v3.24.2 github.com/sabhiram/go-gitignore v0.0.0-20210923224102-525f6e181f06 github.com/sahilm/fuzzy v0.1.1 - github.com/sergi/go-diff v1.3.2-0.20230802210424-5b0b94c5c0d3 + github.com/shirou/gopsutil/v4 v4.25.5 github.com/spf13/cobra v1.9.1 github.com/spf13/viper v1.20.0 github.com/srwiley/oksvg v0.0.0-20221011165216-be6e8873101c @@ -75,9 +75,11 @@ require ( github.com/disintegration/gift v1.1.2 // indirect github.com/dlclark/regexp2 v1.11.4 // indirect github.com/dustin/go-humanize v1.0.1 // indirect + github.com/ebitengine/purego v0.8.4 // indirect github.com/felixge/httpsnoop v1.0.4 // indirect github.com/go-logr/logr v1.4.2 // indirect github.com/go-logr/stdr v1.2.2 // indirect + github.com/go-ole/go-ole v1.2.6 // indirect github.com/go-viper/mapstructure/v2 v2.2.1 // indirect github.com/golang-jwt/jwt/v5 v5.2.2 // indirect github.com/google/go-cmp v0.7.0 // indirect @@ -89,6 +91,7 @@ require ( github.com/inconshreveable/mousetrap v1.1.0 // indirect github.com/kylelemons/godebug v1.1.0 // indirect github.com/lucasb-eyer/go-colorful v1.2.0 + github.com/lufia/plan9stats v0.0.0-20211012122336-39d0f177ccd0 // indirect github.com/mattn/go-isatty v0.0.20 // indirect github.com/mattn/go-runewidth v0.0.16 // indirect github.com/mfridman/interpolate v0.0.2 // indirect @@ -102,9 +105,11 @@ require ( github.com/pelletier/go-toml/v2 v2.2.3 // indirect github.com/pkg/browser v0.0.0-20240102092130-5ac0b6a4141c // indirect github.com/pmezard/go-difflib v1.0.0 // indirect + github.com/power-devops/perfstat v0.0.0-20210106213030-5aafc221ea8c // indirect github.com/rivo/uniseg v0.4.7 github.com/rogpeppe/go-internal v1.14.1 // indirect github.com/sagikazarmark/locafero v0.7.0 // indirect + github.com/sergi/go-diff v1.3.2-0.20230802210424-5b0b94c5c0d3 // indirect github.com/sethvargo/go-retry v0.3.0 // indirect github.com/sourcegraph/conc v0.3.0 // indirect github.com/spf13/afero v1.12.0 // indirect @@ -116,10 +121,13 @@ require ( github.com/tidwall/match v1.1.1 // indirect github.com/tidwall/pretty v1.2.1 // indirect github.com/tidwall/sjson v1.2.5 // indirect + github.com/tklauser/go-sysconf v0.3.12 // indirect + github.com/tklauser/numcpus v0.6.1 // indirect github.com/xo/terminfo v0.0.0-20220910002029-abceb7e1c41e // indirect github.com/yosida95/uritemplate/v3 v3.0.2 // indirect github.com/yuin/goldmark v1.7.8 // indirect github.com/yuin/goldmark-emoji v1.0.5 // indirect + github.com/yusufpapurcu/wmi v1.2.4 // indirect go.opentelemetry.io/auto/sdk v1.1.0 // indirect go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp v0.54.0 // indirect go.opentelemetry.io/otel v1.35.0 // indirect diff --git a/go.sum b/go.sum index 851200f8fb15478d7f2e7d83dd72ae841473e762..9148472301521bcb9a31d84bab3444655bef8f03 100644 --- a/go.sum +++ b/go.sum @@ -108,6 +108,8 @@ github.com/dlclark/regexp2 v1.11.4 h1:rPYF9/LECdNymJufQKmri9gV604RvvABwgOA8un7yA github.com/dlclark/regexp2 v1.11.4/go.mod h1:DHkYz0B9wPfa6wondMfaivmHpzrQ3v9q8cnmRbL6yW8= github.com/dustin/go-humanize v1.0.1 h1:GzkhY7T5VNhEkwH0PVJgjz+fX1rhBrR7pRT3mDkpeCY= github.com/dustin/go-humanize v1.0.1/go.mod h1:Mu1zIs6XwVuF/gI1OepvI0qD18qycQx+mFykh5fBlto= +github.com/ebitengine/purego v0.8.4 h1:CF7LEKg5FFOsASUj0+QwaXf8Ht6TlFxg09+S9wz0omw= +github.com/ebitengine/purego v0.8.4/go.mod h1:iIjxzd6CiRiOG0UyXP+V1+jWqUXVjPKLAI0mRfJZTmQ= github.com/felixge/httpsnoop v1.0.4 h1:NFTV2Zj1bL4mc9sqWACXbQFVBBg2W3GPvqp8/ESS2Wg= github.com/felixge/httpsnoop v1.0.4/go.mod h1:m8KPJKqk1gH5J9DgRY2ASl2lWCfGKXixSwevea8zH2U= github.com/frankban/quicktest v1.14.6 h1:7Xjx+VpznH+oBnejlPUj8oUpdxnVs4f8XU8WnHkI4W8= @@ -121,12 +123,15 @@ github.com/go-logr/logr v1.4.2 h1:6pFjapn8bFcIbiKo3XT4j/BhANplGihG6tvd+8rYgrY= github.com/go-logr/logr v1.4.2/go.mod h1:9T104GzyrTigFIr8wt5mBrctHMim0Nb2HLGrmQ40KvY= github.com/go-logr/stdr v1.2.2 h1:hSWxHoqTgW2S2qGc0LTAI563KZ5YKYRhT3MFKZMbjag= github.com/go-logr/stdr v1.2.2/go.mod h1:mMo/vtBO5dYbehREoey6XUKy/eSumjCCveDpRre4VKE= +github.com/go-ole/go-ole v1.2.6 h1:/Fpf6oFPoeFik9ty7siob0G6Ke8QvQEuVcuChpwXzpY= +github.com/go-ole/go-ole v1.2.6/go.mod h1:pprOEPIfldk/42T2oK7lQ4v4JSDwmV0As9GaiUsvbm0= github.com/go-viper/mapstructure/v2 v2.2.1 h1:ZAaOCxANMuZx5RCeg0mBdEZk7DZasvvZIxtHqx8aGss= github.com/go-viper/mapstructure/v2 v2.2.1/go.mod h1:oJDH3BJKyqBA2TXFhDsKDGDTlndYOZ6rGS0BRZIxGhM= github.com/golang-jwt/jwt/v5 v5.2.2 h1:Rl4B7itRWVtYIHFrSNd7vhTiz9UpLdi6gZhZ3wEeDy8= github.com/golang-jwt/jwt/v5 v5.2.2/go.mod h1:pqrtFR0X4osieyHYxtmOUWsAWrfe1Q5UVIyoH402zdk= github.com/golang/protobuf v1.5.4 h1:i7eJL8qZTpSEXOPTxNKhASYpMn+8e5Q6AdndVa1dWek= github.com/golang/protobuf v1.5.4/go.mod h1:lnTiLA8Wa4RWRcIUkrtSVa5nRhsEGBg48fD6rSs7xps= +github.com/google/go-cmp v0.5.6/go.mod h1:v8dTdLbMG2kIc/vJvl+f65V22dbkXbowE6jgT/gNBxE= github.com/google/go-cmp v0.7.0 h1:wk8382ETsv4JYUZwIsn6YpYiWiBsYLSJiTsyBybVuN8= github.com/google/go-cmp v0.7.0/go.mod h1:pXiqmnSA92OHEEa9HXL2W4E7lf9JzCmGVUdgjX3N/iU= github.com/google/s2a-go v0.1.8 h1:zZDs9gcbt9ZPLV0ndSyQk6Kacx2g/X+SKYovpnz3SMM= @@ -156,6 +161,8 @@ github.com/kylelemons/godebug v1.1.0 h1:RPNrshWIDI6G2gRW9EHilWtl7Z6Sb1BR0xunSBf0 github.com/kylelemons/godebug v1.1.0/go.mod h1:9/0rRGxNHcop5bhtWyNeEfOS8JIWk580+fNqagV/RAw= github.com/lucasb-eyer/go-colorful v1.2.0 h1:1nnpGOrhyZZuNyfu1QjKiUICQ74+3FNCN69Aj6K7nkY= github.com/lucasb-eyer/go-colorful v1.2.0/go.mod h1:R4dSotOR9KMtayYi1e77YzuveK+i7ruzyGqttikkLy0= +github.com/lufia/plan9stats v0.0.0-20211012122336-39d0f177ccd0 h1:6E+4a0GO5zZEnZ81pIr0yLvtUWk2if982qA3F3QD6H4= +github.com/lufia/plan9stats v0.0.0-20211012122336-39d0f177ccd0/go.mod h1:zJYVVT2jmtg6P3p1VtQj7WsuWi/y4VnjVBn7F8KPB3I= github.com/mark3labs/mcp-go v0.17.0 h1:5Ps6T7qXr7De/2QTqs9h6BKeZ/qdeUeGrgM5lPzi930= github.com/mark3labs/mcp-go v0.17.0/go.mod h1:KmJndYv7GIgcPVwEKJjNcbhVQ+hJGJhrCCB/9xITzpE= github.com/mattn/go-isatty v0.0.20 h1:xfD0iDuEKnDkl03q4limB+vH+GxLEtL/jb4xVJSWWEY= @@ -195,6 +202,8 @@ github.com/pkg/browser v0.0.0-20240102092130-5ac0b6a4141c/go.mod h1:7rwL4CYBLnjL github.com/pkg/errors v0.8.1/go.mod h1:bwawxfHBFNV+L2hUp1rHADufV3IMtnDRdf1r5NINEl0= github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZbAQM= github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4= +github.com/power-devops/perfstat v0.0.0-20210106213030-5aafc221ea8c h1:ncq/mPwQF4JjgDlrVEn3C11VoGHZN7m8qihwgMEtzYw= +github.com/power-devops/perfstat v0.0.0-20210106213030-5aafc221ea8c/go.mod h1:OmDBASR4679mdNQnz2pUhc2G8CO2JrUAVFDRBDP/hJE= github.com/pressly/goose/v3 v3.24.2 h1:c/ie0Gm8rnIVKvnDQ/scHErv46jrDv9b4I0WRcFJzYU= github.com/pressly/goose/v3 v3.24.2/go.mod h1:kjefwFB0eR4w30Td2Gj2Mznyw94vSP+2jJYkOVNbD1k= github.com/remyoudompheng/bigfft v0.0.0-20230129092748-24d4a6f8daec h1:W09IVJc94icq4NjY3clb7Lk8O1qJ8BdBEF8z0ibU0rE= @@ -219,6 +228,8 @@ github.com/sergi/go-diff v1.3.2-0.20230802210424-5b0b94c5c0d3 h1:n661drycOFuPLCN github.com/sergi/go-diff v1.3.2-0.20230802210424-5b0b94c5c0d3/go.mod h1:A0bzQcvG0E7Rwjx0REVgAGH58e96+X0MeOfepqsbeW4= github.com/sethvargo/go-retry v0.3.0 h1:EEt31A35QhrcRZtrYFDTBg91cqZVnFL2navjDrah2SE= github.com/sethvargo/go-retry v0.3.0/go.mod h1:mNX17F0C/HguQMyMyJxcnU471gOZGxCLyYaFyAZraas= +github.com/shirou/gopsutil/v4 v4.25.5 h1:rtd9piuSMGeU8g1RMXjZs9y9luK5BwtnG7dZaQUJAsc= +github.com/shirou/gopsutil/v4 v4.25.5/go.mod h1:PfybzyydfZcN+JMMjkF6Zb8Mq1A/VcogFFg7hj50W9c= github.com/sourcegraph/conc v0.3.0 h1:OQTbbt6P72L20UqAkXXuLOj79LfEanQ+YQFNpLA9ySo= github.com/sourcegraph/conc v0.3.0/go.mod h1:Sdozi7LEKbFPqYX2/J+iBAM6HpqSLTASQIKqDmF7Mt0= github.com/spf13/afero v1.12.0 h1:UcOPyRBYczmFn6yvphxkn9ZEOY65cpwGKb5mL36mrqs= @@ -255,6 +266,10 @@ github.com/tidwall/pretty v1.2.1 h1:qjsOFOWWQl+N3RsoF5/ssm1pHmJJwhjlSbZ51I6wMl4= github.com/tidwall/pretty v1.2.1/go.mod h1:ITEVvHYasfjBbM0u2Pg8T2nJnzm8xPwvNhhsoaGGjNU= github.com/tidwall/sjson v1.2.5 h1:kLy8mja+1c9jlljvWTlSazM7cKDRfJuR/bOJhcY5NcY= github.com/tidwall/sjson v1.2.5/go.mod h1:Fvgq9kS/6ociJEDnK0Fk1cpYF4FIW6ZF7LAe+6jwd28= +github.com/tklauser/go-sysconf v0.3.12 h1:0QaGUFOdQaIVdPgfITYzaTegZvdCjmYO52cSFAEVmqU= +github.com/tklauser/go-sysconf v0.3.12/go.mod h1:Ho14jnntGE1fpdOqQEEaiKRpvIavV0hSfmBq8nJbHYI= +github.com/tklauser/numcpus v0.6.1 h1:ng9scYS7az0Bk4OZLvrNXNSAO2Pxr1XXRAPyjhIx+Fk= +github.com/tklauser/numcpus v0.6.1/go.mod h1:1XfjsgE2zo8GVw7POkMbHENHzVg3GzmoZ9fESEdAacY= github.com/xo/terminfo v0.0.0-20220910002029-abceb7e1c41e h1:JVG44RsyaB9T2KIHavMF/ppJZNG9ZpyihvCd0w101no= github.com/xo/terminfo v0.0.0-20220910002029-abceb7e1c41e/go.mod h1:RbqR21r5mrJuqunuUZ/Dhy/avygyECGrLceyNeo4LiM= github.com/yosida95/uritemplate/v3 v3.0.2 h1:Ed3Oyj9yrmi9087+NczuL5BwkIc4wvTb5zIM+UJPGz4= @@ -265,6 +280,8 @@ github.com/yuin/goldmark v1.7.8 h1:iERMLn0/QJeHFhxSt3p6PeN9mGnvIKSpG9YYorDMnic= github.com/yuin/goldmark v1.7.8/go.mod h1:uzxRWxtg69N339t3louHJ7+O03ezfj6PlliRlaOzY1E= github.com/yuin/goldmark-emoji v1.0.5 h1:EMVWyCGPlXJfUXBXpuMu+ii3TIaxbVBnEX9uaDC4cIk= github.com/yuin/goldmark-emoji v1.0.5/go.mod h1:tTkZEbwu5wkPmgTcitqddVxY9osFZiavD+r4AzQrh1U= +github.com/yusufpapurcu/wmi v1.2.4 h1:zFUKzehAFReQwLys1b/iSMl+JQGSCSjtVqQn9bBrPo0= +github.com/yusufpapurcu/wmi v1.2.4/go.mod h1:SBZ9tNy3G9/m5Oi98Zks0QjeHVDvuK0qfxQmPyzfmi0= go.opentelemetry.io/auto/sdk v1.1.0 h1:cH53jehLUN6UFLY71z+NDOiNJqDdPRaXzTel0sJySYA= go.opentelemetry.io/auto/sdk v1.1.0/go.mod h1:3wSPjt5PWp2RhlCcmmOial7AvC4DQqZb7a7wCow3W8A= go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp v0.54.0 h1:TT4fX+nBOA/+LUkobKGW1ydGcn+G3vRw9+g5HwCphpk= @@ -311,7 +328,9 @@ golang.org/x/sync v0.1.0/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= golang.org/x/sync v0.13.0 h1:AauUjRAJ9OSnvULf/ARrrVywoJDy0YS2AwQ98I37610= golang.org/x/sync v0.13.0/go.mod h1:1dzgHSNfp02xaA81J2MS99Qcpr2w7fw1gpm99rleRqA= golang.org/x/sys v0.0.0-20190215142949-d0b11bdaac8a/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY= +golang.org/x/sys v0.0.0-20190916202348-b4ddaad3f8a3/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= golang.org/x/sys v0.0.0-20201119102817-f84b799fce68/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= +golang.org/x/sys v0.0.0-20201204225414-ed752295db88/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= golang.org/x/sys v0.0.0-20210615035016-665e8c7367d1/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.0.0-20220520151302-bc2c85ada10a/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.0.0-20220722155257-8c9f86f7a55f/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= @@ -320,6 +339,7 @@ golang.org/x/sys v0.5.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.6.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.7.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.8.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= +golang.org/x/sys v0.11.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.17.0/go.mod h1:/VUhepiaJMQUp4+oa/7Zr1D23ma6VTLIYjOOTFZPUcA= golang.org/x/sys v0.19.0/go.mod h1:/VUhepiaJMQUp4+oa/7Zr1D23ma6VTLIYjOOTFZPUcA= golang.org/x/sys v0.20.0/go.mod h1:/VUhepiaJMQUp4+oa/7Zr1D23ma6VTLIYjOOTFZPUcA= @@ -347,6 +367,7 @@ golang.org/x/tools v0.0.0-20191119224855-298f0cb1881e/go.mod h1:b+2E5dAYhXwXZwtn golang.org/x/tools v0.1.12/go.mod h1:hNGJHUnrk76NpqgfD5Aqm5Crs+Hm0VOH/i9J2+nxYbc= golang.org/x/tools v0.6.0/go.mod h1:Xwgl3UAJ/d3gWutnCtw505GrjyAbvKui8lOU390QaIU= golang.org/x/xerrors v0.0.0-20190717185122-a985d3407aa7/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= +golang.org/x/xerrors v0.0.0-20191204190536-9bdfabe68543/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= google.golang.org/genai v1.3.0 h1:tXhPJF30skOjnnDY7ZnjK3q7IKy4PuAlEA0fk7uEaEI= google.golang.org/genai v1.3.0/go.mod h1:TyfOKRz/QyCaj6f/ZDt505x+YreXnY40l2I6k8TvgqY= google.golang.org/genproto/googleapis/rpc v0.0.0-20250324211829-b45e905df463 h1:e0AIkUUhxyBKh6ssZNrAMeqhA7RKUj42346d1y02i2g= diff --git a/internal/fsext/fileutil.go b/internal/fsext/fileutil.go index 8230e3a3c2f965bef22131678b146173d81935a1..8abc3cebe98297b27c0e93c52881761fc971f9ec 100644 --- a/internal/fsext/fileutil.go +++ b/internal/fsext/fileutil.go @@ -99,8 +99,8 @@ func SkipHidden(path string) bool { "jspm_packages": true, } - parts := strings.Split(path, string(os.PathSeparator)) - for _, part := range parts { + parts := strings.SplitSeq(path, string(os.PathSeparator)) + for part := range parts { if commonIgnoredDirs[part] { return true } diff --git a/internal/llm/tools/shell/shell.go b/internal/llm/tools/shell/shell.go index 38c3021a89a33579f97ea2610a824ba0f5b40dfb..da23a03d845c2727765716a56c25782f1c20282a 100644 --- a/internal/llm/tools/shell/shell.go +++ b/internal/llm/tools/shell/shell.go @@ -14,6 +14,8 @@ import ( "time" "github.com/charmbracelet/crush/internal/config" + "github.com/charmbracelet/crush/internal/logging" + "github.com/shirou/gopsutil/v4/process" ) type PersistentShell struct { @@ -216,10 +218,7 @@ echo $EXEC_EXIT_CODE > %s // Exponential backoff to reduce CPU usage for longer-running commands if pollInterval < maxPollInterval { - pollInterval = time.Duration(float64(pollInterval) * 1.5) - if pollInterval > maxPollInterval { - pollInterval = maxPollInterval - } + pollInterval = min(time.Duration(float64(pollInterval)*1.5), maxPollInterval) ticker.Reset(pollInterval) } } @@ -257,23 +256,21 @@ func (s *PersistentShell) killChildren() { if s.cmd == nil || s.cmd.Process == nil { return } + p, err := process.NewProcess(int32(s.cmd.Process.Pid)) + if err != nil { + logging.WarnPersist("could not kill persistent shell child processes", "err", err) + return + } - pgrepCmd := exec.Command("pgrep", "-P", fmt.Sprintf("%d", s.cmd.Process.Pid)) - output, err := pgrepCmd.Output() + children, err := p.Children() if err != nil { + logging.WarnPersist("could not kill persistent shell child processes", "err", err) return } - for pidStr := range strings.SplitSeq(string(output), "\n") { - if pidStr = strings.TrimSpace(pidStr); pidStr != "" { - var pid int - fmt.Sscanf(pidStr, "%d", &pid) - if pid > 0 { - proc, err := os.FindProcess(pid) - if err == nil { - proc.Signal(syscall.SIGTERM) - } - } + for _, child := range children { + if err := child.SendSignal(syscall.SIGTERM); err != nil { + logging.WarnPersist("could not kill persistent shell child processes", "err", err, "pid", child.Pid) } } } @@ -307,7 +304,9 @@ func (s *PersistentShell) Close() { s.stdin.Write([]byte("exit\n")) - s.cmd.Process.Kill() + if err := s.cmd.Process.Kill(); err != nil { + logging.WarnPersist("could not kill persistent shell", "err", err) + } s.isAlive = false } From 81460061ad64f6643cc472111b90b5b1967cb664 Mon Sep 17 00:00:00 2001 From: Carlos Alexandro Becker Date: Fri, 20 Jun 2025 11:20:15 -0300 Subject: [PATCH 3/3] fix: improve shell --- internal/llm/tools/shell/shell.go | 98 ++++++++++++------------------- 1 file changed, 37 insertions(+), 61 deletions(-) diff --git a/internal/llm/tools/shell/shell.go b/internal/llm/tools/shell/shell.go index da23a03d845c2727765716a56c25782f1c20282a..25ba75a7877a7e4009991c78e6f09774c0d1bb79 100644 --- a/internal/llm/tools/shell/shell.go +++ b/internal/llm/tools/shell/shell.go @@ -1,6 +1,7 @@ package shell import ( + "cmp" "context" "errors" "fmt" @@ -67,16 +68,15 @@ func newPersistentShell(cwd string) *PersistentShell { shellArgs = cfg.Shell.Args } - if shellPath == "" { - shellPath = os.Getenv("SHELL") - if shellPath == "" { - shellPath = "/bin/bash" - } + shellPath = cmp.Or(shellPath, os.Getenv("SHELL"), "/bin/bash") + if !strings.HasSuffix(shellPath, "bash") && !strings.HasSuffix(shellPath, "zsh") { + logging.Warn("only bash and zsh are supported at this time", "shell", shellPath) + shellPath = "/bin/bash" } // Default shell args if len(shellArgs) == 0 { - shellArgs = []string{"-l"} + shellArgs = []string{"--login"} } cmd := exec.Command(shellPath, shellArgs...) @@ -127,12 +127,15 @@ func newPersistentShell(cwd string) *PersistentShell { func (s *PersistentShell) processCommands() { for cmd := range s.commandQueue { - result := s.execCommand(cmd.command, cmd.timeout, cmd.ctx) - cmd.resultChan <- result + cmd.resultChan <- s.execCommand(cmd.ctx, cmd.command, cmd.timeout) } } -func (s *PersistentShell) execCommand(command string, timeout time.Duration, ctx context.Context) commandResult { +const runBashCommandFormat = `%s %q 2>%q +echo $? >%q +pwd >%q` + +func (s *PersistentShell) execCommand(ctx context.Context, command string, timeout time.Duration) commandResult { s.mu.Lock() defer s.mu.Unlock() @@ -144,34 +147,22 @@ func (s *PersistentShell) execCommand(command string, timeout time.Duration, ctx } } - tempDir := os.TempDir() - stdoutFile := filepath.Join(tempDir, fmt.Sprintf("crush-stdout-%d", time.Now().UnixNano())) - stderrFile := filepath.Join(tempDir, fmt.Sprintf("crush-stderr-%d", time.Now().UnixNano())) - statusFile := filepath.Join(tempDir, fmt.Sprintf("crush-status-%d", time.Now().UnixNano())) - cwdFile := filepath.Join(tempDir, fmt.Sprintf("crush-cwd-%d", time.Now().UnixNano())) + tmp := os.TempDir() + now := time.Now().UnixNano() + stdoutFile := filepath.Join(tmp, fmt.Sprintf("crush-stdout-%d", now)) + stderrFile := filepath.Join(tmp, fmt.Sprintf("crush-stderr-%d", now)) + statusFile := filepath.Join(tmp, fmt.Sprintf("crush-status-%d", now)) + cwdFile := filepath.Join(tmp, fmt.Sprintf("crush-cwd-%d", now)) defer func() { - os.Remove(stdoutFile) - os.Remove(stderrFile) - os.Remove(statusFile) - os.Remove(cwdFile) + _ = os.Remove(stdoutFile) + _ = os.Remove(stderrFile) + _ = os.Remove(statusFile) + _ = os.Remove(cwdFile) }() - fullCommand := fmt.Sprintf(` -eval %s < /dev/null > %s 2> %s -EXEC_EXIT_CODE=$? -pwd > %s -echo $EXEC_EXIT_CODE > %s -`, - shellQuote(command), - shellQuote(stdoutFile), - shellQuote(stderrFile), - shellQuote(cwdFile), - shellQuote(statusFile), - ) - - _, err := s.stdin.Write([]byte(fullCommand + "\n")) - if err != nil { + script := fmt.Sprintf(runBashCommandFormat, command, stdoutFile, stderrFile, statusFile, cwdFile) + if _, err := s.stdin.Write([]byte(script + "\n")); err != nil { return commandResult{ stderr: fmt.Sprintf("Failed to write command to shell: %v", err), exitCode: 1, @@ -180,18 +171,18 @@ echo $EXEC_EXIT_CODE > %s } interrupted := false - - startTime := time.Now() - done := make(chan bool) go func() { // Use exponential backoff polling - pollInterval := 1 * time.Millisecond - maxPollInterval := 100 * time.Millisecond + pollInterval := 10 * time.Millisecond + maxPollInterval := time.Second ticker := time.NewTicker(pollInterval) defer ticker.Stop() + timeoutTicker := time.NewTicker(cmp.Or(timeout, time.Hour*99999)) + defer timeoutTicker.Stop() + for { select { case <-ctx.Done(): @@ -200,22 +191,18 @@ echo $EXEC_EXIT_CODE > %s done <- true return + case <-timeoutTicker.C: + s.killChildren() + interrupted = true + done <- true + return + case <-ticker.C: - if fileExists(statusFile) && fileSize(statusFile) > 0 { + if fileSize(statusFile) > 0 { done <- true return } - if timeout > 0 { - elapsed := time.Since(startTime) - if elapsed > timeout { - s.killChildren() - interrupted = true - done <- true - return - } - } - // Exponential backoff to reduce CPU usage for longer-running commands if pollInterval < maxPollInterval { pollInterval = min(time.Duration(float64(pollInterval)*1.5), maxPollInterval) @@ -280,12 +267,10 @@ func (s *PersistentShell) Exec(ctx context.Context, command string, timeoutMs in return "", "Shell is not alive", 1, false, errors.New("shell is not alive") } - timeout := time.Duration(timeoutMs) * time.Millisecond - resultChan := make(chan commandResult) s.commandQueue <- &commandExecution{ command: command, - timeout: timeout, + timeout: time.Duration(timeoutMs) * time.Millisecond, resultChan: resultChan, ctx: ctx, } @@ -310,10 +295,6 @@ func (s *PersistentShell) Close() { s.isAlive = false } -func shellQuote(s string) string { - return "'" + strings.ReplaceAll(s, "'", "'\\''") + "'" -} - func readFileOrEmpty(path string) string { content, err := os.ReadFile(path) if err != nil { @@ -322,11 +303,6 @@ func readFileOrEmpty(path string) string { return string(content) } -func fileExists(path string) bool { - _, err := os.Stat(path) - return err == nil -} - func fileSize(path string) int64 { info, err := os.Stat(path) if err != nil {