Skip to content

Commit f061db7

Browse files
fix(local): replace bufio.Scanner with bufio.Reader, implement Execute, and merge stderr to stdout in code template (#735)
- Replace bufio.Scanner with bufio.Reader in streamStdout to handle long lines - Implement Local.Execute using exec.CommandContext with /bin/sh -c - Merge stderr output into stdout with labeled sections in executePythonCodeTemplate - Add unit tests for Execute method
1 parent 7529d30 commit f061db7

File tree

5 files changed

+186
-46
lines changed

5 files changed

+186
-46
lines changed

adk/backend/agentkit/code_template.go

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -246,7 +246,11 @@ try:
246246
247247
# Check for stderr
248248
if result.stderr:
249-
print(f"Error executing command: {{result.stderr}}", file=sys.stderr)
249+
output_parts = []
250+
if result.stdout:
251+
output_parts.append(f"[stdout]:\n{{result.stdout.rstrip()}}")
252+
output_parts.append(f"[stderr]:\n{{result.stderr.rstrip()}}")
253+
print('\n'.join(output_parts), end='')
250254
sys.exit(result.returncode if result.returncode != 0 else 1)
251255
252256
# Print stdout

adk/backend/agentkit/sandbox.go

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -328,7 +328,7 @@ func (s *SandboxTool) GlobInfo(ctx context.Context, req *filesystem.GlobInfoRequ
328328
return nil, fmt.Errorf("failed to parse glob output: %w", err)
329329
}
330330
return files, nil
331-
331+
332332
}
333333

334334
// Write creates file content.
@@ -582,12 +582,9 @@ func (s *SandboxTool) Execute(ctx context.Context, input *filesystem.ExecuteRequ
582582
return nil, fmt.Errorf("failed to execute command script: %w", err)
583583
}
584584

585-
if exitCode != nil && *exitCode != 0 {
586-
return nil, fmt.Errorf("command exited with non-zero code %d: %s", *exitCode, output)
587-
}
588-
589585
return &filesystem.ExecuteResponse{
590-
Output: output,
586+
Output: output,
587+
ExitCode: exitCode,
591588
}, nil
592589
}
593590

adk/backend/agentkit/sandbox_test.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -321,9 +321,9 @@ func TestArkSandbox_FileSystemMethods(t *testing.T) {
321321
w.WriteHeader(http.StatusOK)
322322
w.Write(createMockResponse(t, false, "command failed", "Error", "1"))
323323
}
324-
_, err := s.Execute(context.Background(), &filesystem.ExecuteRequest{Command: "exit 1"})
325-
require.Error(t, err)
326-
assert.Contains(t, err.Error(), "command exited with non-zero code -1: command failed")
324+
resp, err := s.Execute(context.Background(), &filesystem.ExecuteRequest{Command: "exit 1"})
325+
require.Nil(t, err)
326+
assert.Contains(t, resp.Output, "command failed")
327327
})
328328

329329
t.Run("Execute: RunInBackendGround returns immediately", func(t *testing.T) {

adk/backend/local/local.go

Lines changed: 72 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -396,6 +396,49 @@ func (s *Local) ExecuteStreaming(ctx context.Context, input *filesystem.ExecuteR
396396
return sr, nil
397397
}
398398

399+
func (s *Local) Execute(ctx context.Context, input *filesystem.ExecuteRequest) (result *filesystem.ExecuteResponse, err error) {
400+
if input.Command == "" {
401+
return nil, fmt.Errorf("command is required")
402+
}
403+
404+
if err := s.validateCommand(input.Command); err != nil {
405+
return nil, err
406+
}
407+
408+
cmd := exec.CommandContext(ctx, "/bin/sh", "-c", input.Command)
409+
410+
var stdoutBuf, stderrBuf strings.Builder
411+
cmd.Stdout = &stdoutBuf
412+
cmd.Stderr = &stderrBuf
413+
414+
exitCode := 0
415+
if err := cmd.Run(); err != nil {
416+
var exitError *exec.ExitError
417+
if errors.As(err, &exitError) {
418+
exitCode = exitError.ExitCode()
419+
stdoutStr := stdoutBuf.String()
420+
stderrStr := stderrBuf.String()
421+
parts := []string{fmt.Sprintf("command exited with non-zero code %d", exitCode)}
422+
if stdoutStr != "" {
423+
parts = append(parts, "[stdout]:\n"+strings.TrimSuffix(stdoutStr, ""))
424+
}
425+
if stderrStr != "" {
426+
parts = append(parts, "[stderr]:\n"+strings.TrimSuffix(stderrStr, ""))
427+
}
428+
return &filesystem.ExecuteResponse{
429+
Output: strings.Join(parts, "\n"),
430+
ExitCode: &exitCode,
431+
}, nil
432+
}
433+
return nil, fmt.Errorf("failed to execute command: %w", err)
434+
}
435+
436+
return &filesystem.ExecuteResponse{
437+
Output: stdoutBuf.String(),
438+
ExitCode: &exitCode,
439+
}, nil
440+
}
441+
399442
// initStreamingCmd creates command with stdout and stderr pipes.
400443
func (s *Local) initStreamingCmd(ctx context.Context, command string) (*exec.Cmd, io.ReadCloser, io.ReadCloser, error) {
401444
cmd := exec.CommandContext(ctx, "/bin/sh", "-c", command)
@@ -512,23 +555,27 @@ func (s *Local) readStderrAsync(stderr io.Reader) (*[]byte, <-chan error) {
512555

513556
// streamStdout streams stdout line by line to the writer.
514557
func (s *Local) streamStdout(ctx context.Context, cmd *exec.Cmd, stdout io.Reader, w *schema.StreamWriter[*filesystem.ExecuteResponse]) (bool, error) {
515-
scanner := bufio.NewScanner(stdout)
558+
reader := bufio.NewReader(stdout)
516559
hasOutput := false
517560

518-
for scanner.Scan() {
519-
hasOutput = true
520-
line := scanner.Text() + "\n"
521-
select {
522-
case <-ctx.Done():
523-
_ = cmd.Process.Kill()
524-
return hasOutput, ctx.Err()
525-
default:
526-
w.Send(&filesystem.ExecuteResponse{Output: line}, nil)
561+
for {
562+
line, err := reader.ReadString('\n')
563+
if line != "" {
564+
hasOutput = true
565+
select {
566+
case <-ctx.Done():
567+
_ = cmd.Process.Kill()
568+
return hasOutput, ctx.Err()
569+
default:
570+
w.Send(&filesystem.ExecuteResponse{Output: line}, nil)
571+
}
572+
}
573+
if err != nil {
574+
if err != io.EOF {
575+
return hasOutput, fmt.Errorf("error reading stdout: %w", err)
576+
}
577+
break
527578
}
528-
}
529-
530-
if err := scanner.Err(); err != nil {
531-
return hasOutput, fmt.Errorf("error reading stdout: %w", err)
532579
}
533580

534581
return hasOutput, nil
@@ -537,16 +584,21 @@ func (s *Local) streamStdout(ctx context.Context, cmd *exec.Cmd, stdout io.Reade
537584
// handleCmdCompletion handles command completion and sends final response.
538585
func (s *Local) handleCmdCompletion(cmd *exec.Cmd, stderrData *[]byte, hasOutput bool, w *schema.StreamWriter[*filesystem.ExecuteResponse]) {
539586
if err := cmd.Wait(); err != nil {
540-
exitCode := 0
541587
var exitError *exec.ExitError
542588
if errors.As(err, &exitError) {
543-
exitCode = exitError.ExitCode()
544-
}
545-
if len(*stderrData) > 0 {
546-
w.Send(nil, fmt.Errorf("command exited with non-zero code %d: %s", exitCode, string(*stderrData)))
589+
exitCode := exitError.ExitCode()
590+
parts := []string{fmt.Sprintf("command exited with non-zero code %d", exitCode)}
591+
if stderrStr := string(*stderrData); stderrStr != "" {
592+
parts = append(parts, "[stderr]:\n"+stderrStr)
593+
}
594+
w.Send(&filesystem.ExecuteResponse{
595+
Output: strings.Join(parts, "\n"),
596+
ExitCode: &exitCode,
597+
}, nil)
547598
return
548599
}
549-
w.Send(nil, fmt.Errorf("command exited with non-zero code %d", exitCode))
600+
601+
w.Send(nil, fmt.Errorf("command failed: %w", err))
550602
return
551603
}
552604

adk/backend/local/local_test.go

Lines changed: 103 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -595,21 +595,25 @@ func TestExecuteStreaming(t *testing.T) {
595595
assert.NoError(t, err)
596596

597597
var hasOutput bool
598-
var lastErr error
598+
var lastResp *filesystem.ExecuteResponse
599599
for {
600600
resp, err := sr.Recv()
601601
if err != nil {
602-
lastErr = err
603602
break
604603
}
605-
if resp != nil && resp.Output != "" {
606-
hasOutput = true
604+
if resp != nil {
605+
if resp.Output != "" {
606+
hasOutput = true
607+
}
608+
lastResp = resp
607609
}
608610
}
609611

610-
assert.True(t, hasOutput, "should receive output before error")
611-
assert.Error(t, lastErr, "should receive error when command fails")
612-
assert.Contains(t, lastErr.Error(), "non-zero code")
612+
assert.True(t, hasOutput, "should receive output before final response")
613+
assert.NotNil(t, lastResp)
614+
assert.NotNil(t, lastResp.ExitCode)
615+
assert.Equal(t, 1, *lastResp.ExitCode)
616+
assert.Contains(t, lastResp.Output, "non-zero code")
613617
})
614618

615619
t.Run("ExecuteStreaming with stderr output", func(t *testing.T) {
@@ -618,23 +622,28 @@ func TestExecuteStreaming(t *testing.T) {
618622
assert.NoError(t, err)
619623

620624
var outputs []string
621-
var lastErr error
625+
var lastResp *filesystem.ExecuteResponse
622626
for {
623627
resp, err := sr.Recv()
624628
if err != nil {
625-
lastErr = err
626629
break
627630
}
628-
if resp != nil && resp.Output != "" {
629-
outputs = append(outputs, strings.TrimSpace(resp.Output))
631+
if resp != nil {
632+
if resp.Output != "" {
633+
outputs = append(outputs, strings.TrimSpace(resp.Output))
634+
}
635+
lastResp = resp
630636
}
631637
}
632638

633-
assert.Len(t, outputs, 1)
634-
assert.Equal(t, "stdout", outputs[0])
635-
assert.Error(t, lastErr)
636-
assert.Contains(t, lastErr.Error(), "stderr")
637-
assert.Contains(t, lastErr.Error(), "non-zero code")
639+
assert.NotNil(t, lastResp)
640+
assert.NotNil(t, lastResp.ExitCode)
641+
assert.Equal(t, 1, *lastResp.ExitCode)
642+
assert.Contains(t, lastResp.Output, "non-zero code")
643+
assert.Contains(t, lastResp.Output, "stderr")
644+
// stdout is streamed separately
645+
assert.True(t, len(outputs) >= 1)
646+
assert.Contains(t, outputs, "stdout")
638647
})
639648

640649
t.Run("ExecuteStreaming with empty command", func(t *testing.T) {
@@ -777,3 +786,81 @@ func TestExecuteStreaming(t *testing.T) {
777786
assert.Less(t, elapsed, 2*time.Second, "background command should return immediately without waiting")
778787
})
779788
}
789+
790+
func TestExecute(t *testing.T) {
791+
ctx := context.Background()
792+
s, err := NewBackend(ctx, &Config{})
793+
assert.NoError(t, err)
794+
795+
t.Run("simple echo", func(t *testing.T) {
796+
resp, err := s.Execute(ctx, &filesystem.ExecuteRequest{Command: "echo hello"})
797+
assert.NoError(t, err)
798+
assert.Equal(t, "hello\n", resp.Output)
799+
assert.NotNil(t, resp.ExitCode)
800+
assert.Equal(t, 0, *resp.ExitCode)
801+
})
802+
803+
t.Run("multi-line output", func(t *testing.T) {
804+
resp, err := s.Execute(ctx, &filesystem.ExecuteRequest{Command: "echo line1 && echo line2 && echo line3"})
805+
assert.NoError(t, err)
806+
assert.Equal(t, "line1\nline2\nline3\n", resp.Output)
807+
assert.Equal(t, 0, *resp.ExitCode)
808+
})
809+
810+
t.Run("empty command", func(t *testing.T) {
811+
_, err := s.Execute(ctx, &filesystem.ExecuteRequest{Command: ""})
812+
assert.Error(t, err)
813+
assert.Contains(t, err.Error(), "command is required")
814+
})
815+
816+
t.Run("non-zero exit code", func(t *testing.T) {
817+
resp, err := s.Execute(ctx, &filesystem.ExecuteRequest{Command: "exit 1"})
818+
assert.NoError(t, err)
819+
assert.NotNil(t, resp)
820+
assert.NotNil(t, resp.ExitCode)
821+
assert.Equal(t, 1, *resp.ExitCode)
822+
assert.Contains(t, resp.Output, "non-zero code")
823+
})
824+
825+
t.Run("non-zero exit code with stderr", func(t *testing.T) {
826+
resp, err := s.Execute(ctx, &filesystem.ExecuteRequest{Command: "echo fail >&2 && exit 2"})
827+
assert.NoError(t, err)
828+
assert.NotNil(t, resp)
829+
assert.NotNil(t, resp.ExitCode)
830+
assert.Equal(t, 2, *resp.ExitCode)
831+
assert.Contains(t, resp.Output, "non-zero code 2")
832+
assert.Contains(t, resp.Output, "fail")
833+
})
834+
835+
t.Run("command not found", func(t *testing.T) {
836+
resp, err := s.Execute(ctx, &filesystem.ExecuteRequest{Command: "nonexistent_command_xyz"})
837+
assert.NoError(t, err)
838+
assert.NotNil(t, resp)
839+
assert.NotNil(t, resp.ExitCode)
840+
assert.NotEqual(t, 0, *resp.ExitCode)
841+
})
842+
843+
t.Run("context cancellation", func(t *testing.T) {
844+
cancelCtx, cancel := context.WithCancel(ctx)
845+
cancel()
846+
_, err := s.Execute(cancelCtx, &filesystem.ExecuteRequest{Command: "sleep 10"})
847+
assert.Error(t, err)
848+
})
849+
850+
t.Run("context cancellation during execution", func(t *testing.T) {
851+
cancelCtx, cancel := context.WithCancel(ctx)
852+
go func() {
853+
time.Sleep(100 * time.Millisecond)
854+
cancel()
855+
}()
856+
resp, err := s.Execute(cancelCtx, &filesystem.ExecuteRequest{Command: "sleep 10"})
857+
// Process killed by context cancellation produces an ExitError, returned as response
858+
if err != nil {
859+
// Non-ExitError case is also acceptable
860+
return
861+
}
862+
assert.NotNil(t, resp)
863+
assert.NotNil(t, resp.ExitCode)
864+
assert.NotEqual(t, 0, *resp.ExitCode)
865+
})
866+
}

0 commit comments

Comments
 (0)