feat: US-012 - Add ShellTool tests
Added comprehensive test coverage for ShellTool (ExecTool) with 9 test cases: - TestShellTool_Success: Verifies successful command execution - TestShellTool_Failure: Verifies failed command execution with IsError flag - TestShellTool_Timeout: Verifies command timeout handling - TestShellTool_WorkingDir: Verifies custom working directory support - TestShellTool_DangerousCommand: Verifies safety guard blocks dangerous commands - TestShellTool_MissingCommand: Verifies error handling for missing command - TestShellTool_StderrCapture: Verifies stderr is captured and included - TestShellTool_OutputTruncation: Verifies long output is truncated - TestShellTool_RestrictToWorkspace: Verifies workspace restriction ShellTool implementation already conforms to ToolResult specification: - Success returns ForUser = command output - Failure returns IsError = true - ForLLM contains full output and exit code Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This commit is contained in:
@@ -182,7 +182,7 @@
|
||||
"go test ./pkg/tools -run TestShellTool passes"
|
||||
],
|
||||
"priority": 12,
|
||||
"passes": false,
|
||||
"passes": true,
|
||||
"notes": ""
|
||||
},
|
||||
{
|
||||
|
||||
@@ -18,6 +18,7 @@ Tool 返回值结构化重构 - 将 Tool 接口返回值从 (string, error) 改
|
||||
- US-009: State save atomicity - SetLastChannel
|
||||
- US-010: Update RecordLastChannel to use atomic save
|
||||
- US-011: Refactor MessageTool to use ToolResult
|
||||
- US-012: Refactor ShellTool to use ToolResult
|
||||
|
||||
### In Progress
|
||||
|
||||
@@ -219,4 +220,29 @@ Tool 返回值结构化重构 - 将 Tool 接口返回值从 (string, error) 改
|
||||
- **Gotchas encountered:** 测试文件使用 `map[string]interface{}` 作为 args 参数类型,与 Tool.Execute 签名一致。
|
||||
- **Useful context:** MessageTool 的设计模式是"用户已直接收到消息,所以 ForUser 为空且 Silent=true",避免重复发送。
|
||||
|
||||
---
|
||||
|
||||
## [2026-02-12] - US-012
|
||||
- What was implemented:
|
||||
- ShellTool 已经完全使用 ToolResult 返回值(无需修改实现)
|
||||
- 添加了完整的测试文件 `pkg/tools/shell_test.go`,包含 9 个测试用例
|
||||
- 测试覆盖了所有验收标准:
|
||||
- 成功返回 ForUser = 命令输出
|
||||
- 失败返回 IsError = true
|
||||
- ForLLM 包含完整输出和退出码
|
||||
- 自定义工作目录测试
|
||||
- 危险命令阻止测试
|
||||
- 缺少命令错误处理
|
||||
- Stderr 捕获测试
|
||||
- 输出截断测试
|
||||
- 工作空间限制测试
|
||||
|
||||
- Files changed:
|
||||
- `pkg/tools/shell_test.go` (新增)
|
||||
|
||||
- **Learnings for future iterations:**
|
||||
- **Patterns discovered:** ShellTool 实现已经符合 ToolResult 规范,只需要添加测试覆盖即可。测试覆盖了成功、失败、超时、自定义目录、安全防护等多种场景。
|
||||
- **Gotchas encountered:** 无
|
||||
- **Useful context:** ShellTool 的安全防护机制包括:危险命令检测(如 `rm -rf`)、工作空间路径遍历检测、命令超时控制。
|
||||
|
||||
---
|
||||
210
pkg/tools/shell_test.go
Normal file
210
pkg/tools/shell_test.go
Normal file
@@ -0,0 +1,210 @@
|
||||
package tools
|
||||
|
||||
import (
|
||||
"context"
|
||||
"os"
|
||||
"path/filepath"
|
||||
"strings"
|
||||
"testing"
|
||||
"time"
|
||||
)
|
||||
|
||||
// TestShellTool_Success verifies successful command execution
|
||||
func TestShellTool_Success(t *testing.T) {
|
||||
tool := NewExecTool("")
|
||||
|
||||
ctx := context.Background()
|
||||
args := map[string]interface{}{
|
||||
"command": "echo 'hello world'",
|
||||
}
|
||||
|
||||
result := tool.Execute(ctx, args)
|
||||
|
||||
// Success should not be an error
|
||||
if result.IsError {
|
||||
t.Errorf("Expected success, got IsError=true: %s", result.ForLLM)
|
||||
}
|
||||
|
||||
// ForUser should contain command output
|
||||
if !strings.Contains(result.ForUser, "hello world") {
|
||||
t.Errorf("Expected ForUser to contain 'hello world', got: %s", result.ForUser)
|
||||
}
|
||||
|
||||
// ForLLM should contain full output
|
||||
if !strings.Contains(result.ForLLM, "hello world") {
|
||||
t.Errorf("Expected ForLLM to contain 'hello world', got: %s", result.ForLLM)
|
||||
}
|
||||
}
|
||||
|
||||
// TestShellTool_Failure verifies failed command execution
|
||||
func TestShellTool_Failure(t *testing.T) {
|
||||
tool := NewExecTool("")
|
||||
|
||||
ctx := context.Background()
|
||||
args := map[string]interface{}{
|
||||
"command": "ls /nonexistent_directory_12345",
|
||||
}
|
||||
|
||||
result := tool.Execute(ctx, args)
|
||||
|
||||
// Failure should be marked as error
|
||||
if !result.IsError {
|
||||
t.Errorf("Expected error for failed command, got IsError=false")
|
||||
}
|
||||
|
||||
// ForUser should contain error information
|
||||
if result.ForUser == "" {
|
||||
t.Errorf("Expected ForUser to contain error info, got empty string")
|
||||
}
|
||||
|
||||
// ForLLM should contain exit code or error
|
||||
if !strings.Contains(result.ForLLM, "Exit code") && result.ForUser == "" {
|
||||
t.Errorf("Expected ForLLM to contain exit code or error, got: %s", result.ForLLM)
|
||||
}
|
||||
}
|
||||
|
||||
// TestShellTool_Timeout verifies command timeout handling
|
||||
func TestShellTool_Timeout(t *testing.T) {
|
||||
tool := NewExecTool("")
|
||||
tool.SetTimeout(100 * time.Millisecond)
|
||||
|
||||
ctx := context.Background()
|
||||
args := map[string]interface{}{
|
||||
"command": "sleep 10",
|
||||
}
|
||||
|
||||
result := tool.Execute(ctx, args)
|
||||
|
||||
// Timeout should be marked as error
|
||||
if !result.IsError {
|
||||
t.Errorf("Expected error for timeout, got IsError=false")
|
||||
}
|
||||
|
||||
// Should mention timeout
|
||||
if !strings.Contains(result.ForLLM, "timed out") && !strings.Contains(result.ForUser, "timed out") {
|
||||
t.Errorf("Expected timeout message, got ForLLM: %s, ForUser: %s", result.ForLLM, result.ForUser)
|
||||
}
|
||||
}
|
||||
|
||||
// TestShellTool_WorkingDir verifies custom working directory
|
||||
func TestShellTool_WorkingDir(t *testing.T) {
|
||||
// Create temp directory
|
||||
tmpDir := t.TempDir()
|
||||
testFile := filepath.Join(tmpDir, "test.txt")
|
||||
os.WriteFile(testFile, []byte("test content"), 0644)
|
||||
|
||||
tool := NewExecTool("")
|
||||
|
||||
ctx := context.Background()
|
||||
args := map[string]interface{}{
|
||||
"command": "cat test.txt",
|
||||
"working_dir": tmpDir,
|
||||
}
|
||||
|
||||
result := tool.Execute(ctx, args)
|
||||
|
||||
if result.IsError {
|
||||
t.Errorf("Expected success in custom working dir, got error: %s", result.ForLLM)
|
||||
}
|
||||
|
||||
if !strings.Contains(result.ForUser, "test content") {
|
||||
t.Errorf("Expected output from custom dir, got: %s", result.ForUser)
|
||||
}
|
||||
}
|
||||
|
||||
// TestShellTool_DangerousCommand verifies safety guard blocks dangerous commands
|
||||
func TestShellTool_DangerousCommand(t *testing.T) {
|
||||
tool := NewExecTool("")
|
||||
|
||||
ctx := context.Background()
|
||||
args := map[string]interface{}{
|
||||
"command": "rm -rf /",
|
||||
}
|
||||
|
||||
result := tool.Execute(ctx, args)
|
||||
|
||||
// Dangerous command should be blocked
|
||||
if !result.IsError {
|
||||
t.Errorf("Expected dangerous command to be blocked (IsError=true)")
|
||||
}
|
||||
|
||||
if !strings.Contains(result.ForLLM, "blocked") && !strings.Contains(result.ForUser, "blocked") {
|
||||
t.Errorf("Expected 'blocked' message, got ForLLM: %s, ForUser: %s", result.ForLLM, result.ForUser)
|
||||
}
|
||||
}
|
||||
|
||||
// TestShellTool_MissingCommand verifies error handling for missing command
|
||||
func TestShellTool_MissingCommand(t *testing.T) {
|
||||
tool := NewExecTool("")
|
||||
|
||||
ctx := context.Background()
|
||||
args := map[string]interface{}{}
|
||||
|
||||
result := tool.Execute(ctx, args)
|
||||
|
||||
// Should return error result
|
||||
if !result.IsError {
|
||||
t.Errorf("Expected error when command is missing")
|
||||
}
|
||||
}
|
||||
|
||||
// TestShellTool_StderrCapture verifies stderr is captured and included
|
||||
func TestShellTool_StderrCapture(t *testing.T) {
|
||||
tool := NewExecTool("")
|
||||
|
||||
ctx := context.Background()
|
||||
args := map[string]interface{}{
|
||||
"command": "sh -c 'echo stdout; echo stderr >&2'",
|
||||
}
|
||||
|
||||
result := tool.Execute(ctx, args)
|
||||
|
||||
// Both stdout and stderr should be in output
|
||||
if !strings.Contains(result.ForLLM, "stdout") {
|
||||
t.Errorf("Expected stdout in output, got: %s", result.ForLLM)
|
||||
}
|
||||
if !strings.Contains(result.ForLLM, "stderr") {
|
||||
t.Errorf("Expected stderr in output, got: %s", result.ForLLM)
|
||||
}
|
||||
}
|
||||
|
||||
// TestShellTool_OutputTruncation verifies long output is truncated
|
||||
func TestShellTool_OutputTruncation(t *testing.T) {
|
||||
tool := NewExecTool("")
|
||||
|
||||
ctx := context.Background()
|
||||
// Generate long output (>10000 chars)
|
||||
args := map[string]interface{}{
|
||||
"command": "python3 -c \"print('x' * 20000)\" || echo " + strings.Repeat("x", 20000),
|
||||
}
|
||||
|
||||
result := tool.Execute(ctx, args)
|
||||
|
||||
// Should have truncation message or be truncated
|
||||
if len(result.ForLLM) > 15000 {
|
||||
t.Errorf("Expected output to be truncated, got length: %d", len(result.ForLLM))
|
||||
}
|
||||
}
|
||||
|
||||
// TestShellTool_RestrictToWorkspace verifies workspace restriction
|
||||
func TestShellTool_RestrictToWorkspace(t *testing.T) {
|
||||
tmpDir := t.TempDir()
|
||||
tool := NewExecTool(tmpDir)
|
||||
tool.SetRestrictToWorkspace(true)
|
||||
|
||||
ctx := context.Background()
|
||||
args := map[string]interface{}{
|
||||
"command": "cat ../../etc/passwd",
|
||||
}
|
||||
|
||||
result := tool.Execute(ctx, args)
|
||||
|
||||
// Path traversal should be blocked
|
||||
if !result.IsError {
|
||||
t.Errorf("Expected path traversal to be blocked with restrictToWorkspace=true")
|
||||
}
|
||||
|
||||
if !strings.Contains(result.ForLLM, "blocked") && !strings.Contains(result.ForUser, "blocked") {
|
||||
t.Errorf("Expected 'blocked' message for path traversal, got ForLLM: %s, ForUser: %s", result.ForLLM, result.ForUser)
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user