这是indexloc提供的服务，不要输入任何密码
Skip to content

feat(tools): add support for streaming command output in bash_tool.go #177

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

feat(tools): add support for streaming command output in bash_tool.go #177

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
143f42d
feat(bash_tool): enhance command execution with streaming support and…
Vinay-Khanagavi May 8, 2025
5d20892
refactor(bash_tool): standardize formatting and improve readability o…
Vinay-Khanagavi May 8, 2025
e16ac64
feat(tools): add command validation for unattended mode to prevent un…
Vinay-Khanagavi May 16, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
79 changes: 78 additions & 1 deletion pkg/tools/bash_tool.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,10 +15,13 @@
package tools

import (
"bufio"
"bytes"
"context"
"fmt"
"os"
"os/exec"
"os/signal"
"path/filepath"
"runtime"
"strings"
Expand Down Expand Up @@ -122,12 +125,86 @@ type ExecResult struct {
}

func executeCommand(cmd *exec.Cmd) (*ExecResult, error) {
// Detect streaming commands (e.g., kubectl logs -f, tail -f)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it will be good to add the original github issue that prompted this change.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yup! I’ll add a reference to the original GitHub issue (e.g., Closes #122) in the code comments,
Let me know if you have a preferred location for this reference (code comment vs. PR body).

isStreaming := false
for _, arg := range cmd.Args {
if arg == "-f" || arg == "--follow" {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suspect it will treat kubectl apply -f a streaming command. Probably better to be very specific here and we can check for presense of ("kubectl logs" and "-f") or ("kubectl tail" and "-f") ...and so on...

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For streaming detection, do you prefer I check specifically for commands like ( "kubectl logs -f") or ("kubectl tail -f") (i.e., both the base command and the -f flag), instead of just any command with -f?
Let me know if you have a preferred way to match these patterns in the codebase.

isStreaming = true
break
}
}

results := &ExecResult{}

if isStreaming {
// Print streaming message
fmt.Fprintln(os.Stdout, "\nStreaming output... Press CTRL-C to stop streaming and return to the prompt.")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Avoid writing to the os.Stdout directly. Use the ui instance to write to the screen. You will have to make ui instance available from the caller to all the way here.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for pointing out the need to use the ui instance instead of writing directly to os.Stdout.

Do you have any suggestions or examples from the codebase on the best way to pass the ui instance down to executeCommand (and the goroutines), especially since it’s currently not available at this level?

Would you recommend passing it as a parameter, using context, or is there a preferred pattern in this project?


stdoutPipe, err := cmd.StdoutPipe()
if err != nil {
results.Error = err.Error()
return results, err
}
stderrPipe, err := cmd.StderrPipe()
if err != nil {
results.Error = err.Error()
return results, err
}

if err := cmd.Start(); err != nil {
results.Error = err.Error()
return results, err
}

// Channel to catch interrupt
interrupt := make(chan os.Signal, 1)
signal.Notify(interrupt, os.Interrupt)
defer signal.Stop(interrupt)

// Read stdout and stderr concurrently
stdoutDone := make(chan struct{})
stderrDone := make(chan struct{})

go func() {
scanner := bufio.NewScanner(stdoutPipe)
for scanner.Scan() {
fmt.Fprintln(os.Stdout, scanner.Text())
}
stdoutDone <- struct{}{}
}()
go func() {
scanner := bufio.NewScanner(stderrPipe)
for scanner.Scan() {
fmt.Fprintln(os.Stderr, scanner.Text())
}
stderrDone <- struct{}{}
}()

// Wait for either interrupt or process exit
select {
case <-interrupt:
cmd.Process.Signal(os.Interrupt)
results.Error = "streaming interrupted by user"
// Wait for goroutines to finish
<-stdoutDone
<-stderrDone
return results, nil
case <-stdoutDone:
<-stderrDone
cmd.Wait()
return results, nil
case <-stderrDone:
<-stdoutDone
cmd.Wait()
return results, nil
}
}
// Non-streaming: collect output as before
var stdout bytes.Buffer
cmd.Stdout = &stdout
var stderr bytes.Buffer
cmd.Stderr = &stderr

results := &ExecResult{}
if err := cmd.Run(); err != nil {
if exitError, ok := err.(*exec.ExitError); ok {
results.ExitCode = exitError.ExitCode()
Expand Down
16 changes: 16 additions & 0 deletions pkg/tools/tools.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -140,6 +140,22 @@ func (t *ToolCall) InvokeTool(ctx context.Context, opt InvokeToolOptions) (any,
ctx = context.WithValue(ctx, "kubeconfig", opt.Kubeconfig)
ctx = context.WithValue(ctx, "work_dir", opt.WorkDir)

command := t.arguments["command"].(string)
blockingPatterns := []struct {
cmd string
flag string
msg string
}{
{"kubectl get", "-w", "watch mode (-w) is not supported in unattended mode. Please run without -w or use a different approach."},
{"kubectl logs", "-f", "log streaming (-f) is not supported in unattended mode. Please run without -f or use a different approach."},
}

for _, pattern := range blockingPatterns {
if strings.Contains(command, pattern.cmd) && strings.Contains(command, pattern.flag) {
return &ExecResult{Error: pattern.msg}, nil
}
}

response, err := t.tool.Run(ctx, t.arguments)

{
Expand Down