diff --git a/pkg/tools/builtin/file.go b/pkg/tools/builtin/file.go index cb4a99d0..43d0d350 100644 --- a/pkg/tools/builtin/file.go +++ b/pkg/tools/builtin/file.go @@ -2,10 +2,13 @@ package builtin import ( + "bufio" "context" "fmt" + "io" "os" "path/filepath" + "strings" "github.com/tombee/conductor/pkg/errors" "github.com/tombee/conductor/pkg/security" @@ -25,6 +28,108 @@ type FileTool struct { securityConfig *security.FileSecurityConfig } +// getIntParam extracts an integer parameter from inputs map with validation. +// Returns the value, whether it was found, and any validation error. +func getIntParam(inputs map[string]interface{}, name string) (int, bool, error) { + val, exists := inputs[name] + if !exists { + return 0, false, nil + } + + // Handle nil explicitly (means parameter not set) + if val == nil { + return 0, false, nil + } + + // Try to extract as int + switch v := val.(type) { + case int: + if v < 0 { + return 0, false, fmt.Errorf("%s must be a non-negative integer", name) + } + return v, true, nil + case float64: + // JSON unmarshaling typically produces float64 for numbers + if v < 0 { + return 0, false, fmt.Errorf("%s must be a non-negative integer", name) + } + return int(v), true, nil + default: + return 0, false, fmt.Errorf("%s must be an integer, got %T", name, val) + } +} + +// readWithLimits reads a file with optional line-based limits for memory efficiency. +// maxLines <= 0 means unlimited (read entire file). +// offset specifies how many lines to skip before reading (0-indexed). +func readWithLimits(reader io.Reader, maxLines, offset int) (content string, linesRead int, err error) { + scanner := bufio.NewScanner(reader) + var lines []string + currentLine := 0 + + for scanner.Scan() { + if offset > 0 && currentLine < offset { + currentLine++ + continue + } + + if maxLines > 0 && linesRead >= maxLines { + break + } + + lines = append(lines, scanner.Text()) + linesRead++ + currentLine++ + } + + if err := scanner.Err(); err != nil { + return "", 0, err + } + + return strings.Join(lines, "\n"), linesRead, nil +} + +// countFileLines efficiently counts the total number of lines in a file. +func countFileLines(path string) (int, error) { + file, err := os.Open(path) + if err != nil { + return 0, err + } + defer file.Close() + + lineCount := 0 + scanner := bufio.NewScanner(file) + for scanner.Scan() { + lineCount++ + } + + if err := scanner.Err(); err != nil { + return 0, err + } + + return lineCount, nil +} + +// buildTruncationMetadata creates the structured truncation metadata. +func buildTruncationMetadata(truncated bool, linesShown, totalLines, startLine int) map[string]interface{} { + metadata := map[string]interface{}{ + "truncated": truncated, + "lines_shown": linesShown, + "total_lines": totalLines, + "start_line": startLine, + } + + if truncated { + metadata["end_line"] = startLine + linesShown - 1 + metadata["more_content"] = true + } else if linesShown > 0 { + metadata["end_line"] = startLine + linesShown - 1 + metadata["more_content"] = false + } + + return metadata +} + // NewFileTool creates a new file tool with default settings. func NewFileTool() *FileTool { return &FileTool{ @@ -115,6 +220,14 @@ func (t *FileTool) Schema() *tools.Schema { Type: "string", Description: "The content to write (required for write operation)", }, + "max_lines": { + Type: "integer", + Description: "Maximum number of lines to read. If not specified, reads entire file. Only applies to read operations.", + }, + "offset": { + Type: "integer", + Description: "Number of lines to skip before reading (0-indexed). Default: 0. Only applies to read operations.", + }, }, Required: []string{"operation", "path"}, }, @@ -133,6 +246,10 @@ func (t *FileTool) Schema() *tools.Schema { Type: "string", Description: "Error message if operation failed", }, + "metadata": { + Type: "object", + Description: "Additional metadata about the operation (e.g., truncation info for read operations)", + }, }, }, } @@ -163,11 +280,35 @@ func (t *FileTool) Execute(ctx context.Context, inputs map[string]interface{}) ( // Execute based on operation switch operation { case "read": + // Extract and validate max_lines parameter + maxLines := 0 // 0 means unlimited + if ml, found, err := getIntParam(inputs, "max_lines"); err != nil { + return nil, &errors.ValidationError{ + Field: "max_lines", + Message: err.Error(), + Suggestion: "Provide a non-negative integer for max_lines", + } + } else if found { + maxLines = ml + } + + // Extract and validate offset parameter + offset := 0 // Default to 0 + if off, found, err := getIntParam(inputs, "offset"); err != nil { + return nil, &errors.ValidationError{ + Field: "offset", + Message: err.Error(), + Suggestion: "Provide a non-negative integer for offset", + } + } else if found { + offset = off + } + // Validate path for read access if err := t.validatePath(path, security.ActionRead); err != nil { return nil, fmt.Errorf("read access validation failed for path %s: %w", path, err) } - return t.read(ctx, path) + return t.read(ctx, path, maxLines, offset) case "write": content, ok := inputs["content"].(string) if !ok { @@ -192,10 +333,10 @@ func (t *FileTool) Execute(ctx context.Context, inputs map[string]interface{}) ( } // read reads a file and returns its content. -func (t *FileTool) read(ctx context.Context, path string) (map[string]interface{}, error) { +func (t *FileTool) read(ctx context.Context, path string, maxLines, offset int) (map[string]interface{}, error) { // Use secure file opening if security config available if t.securityConfig != nil && t.securityConfig.UseFileDescriptors { - return t.readSecure(ctx, path) + return t.readSecure(ctx, path, maxLines, offset) } // Check file size @@ -207,14 +348,59 @@ func (t *FileTool) read(ctx context.Context, path string) (map[string]interface{ }, nil } - if info.Size() > t.maxFileSize { + // Only check max file size for unlimited reads (backward compatibility) + if maxLines <= 0 && info.Size() > t.maxFileSize { return map[string]interface{}{ "success": false, "error": fmt.Sprintf("file size (%d bytes) exceeds maximum allowed size (%d bytes)", info.Size(), t.maxFileSize), }, nil } - // Read file + // Handle line-limited reads + if maxLines > 0 || offset > 0 { + file, err := os.Open(path) + if err != nil { + return map[string]interface{}{ + "success": false, + "error": fmt.Sprintf("failed to open file: %v", err), + }, nil + } + defer file.Close() + + content, linesRead, err := readWithLimits(file, maxLines, offset) + if err != nil { + return map[string]interface{}{ + "success": false, + "error": fmt.Sprintf("failed to read file: %v", err), + }, nil + } + + // Count total lines if we read with limits and got results + totalLines := linesRead + truncated := false + if maxLines > 0 { + // Count total lines to determine if truncated + total, err := countFileLines(path) + if err == nil { + totalLines = total + truncated = (offset + linesRead) < total + } + } + + result := map[string]interface{}{ + "success": true, + "content": content, + } + + // Add metadata if limits were specified + if maxLines > 0 || offset > 0 { + result["metadata"] = buildTruncationMetadata(truncated, linesRead, totalLines, offset) + } + + return result, nil + } + + // Unlimited read (backward compatibility path) content, err := os.ReadFile(path) if err != nil { return map[string]interface{}{ @@ -230,7 +416,7 @@ func (t *FileTool) read(ctx context.Context, path string) (map[string]interface{ } // readSecure reads a file using secure file descriptor approach. -func (t *FileTool) readSecure(ctx context.Context, path string) (map[string]interface{}, error) { +func (t *FileTool) readSecure(ctx context.Context, path string, maxLines, offset int) (map[string]interface{}, error) { // Open file securely file, err := t.securityConfig.OpenFileSecure(path, os.O_RDONLY, 0) if err != nil { @@ -256,14 +442,50 @@ func (t *FileTool) readSecure(ctx context.Context, path string) (map[string]inte maxSize = t.securityConfig.MaxFileSize } - if info.Size() > maxSize { + // Only check max file size for unlimited reads (backward compatibility) + if maxLines <= 0 && info.Size() > maxSize { return map[string]interface{}{ "success": false, "error": fmt.Sprintf("file size (%d bytes) exceeds maximum allowed size (%d bytes)", info.Size(), maxSize), }, nil } - // Read content + // Handle line-limited reads + if maxLines > 0 || offset > 0 { + content, linesRead, err := readWithLimits(file, maxLines, offset) + if err != nil { + return map[string]interface{}{ + "success": false, + "error": fmt.Sprintf("failed to read file: %v", err), + }, nil + } + + // Count total lines if we read with limits and got results + totalLines := linesRead + truncated := false + if maxLines > 0 { + // Count total lines to determine if truncated + total, err := countFileLines(path) + if err == nil { + totalLines = total + truncated = (offset + linesRead) < total + } + } + + result := map[string]interface{}{ + "success": true, + "content": content, + } + + // Add metadata if limits were specified + if maxLines > 0 || offset > 0 { + result["metadata"] = buildTruncationMetadata(truncated, linesRead, totalLines, offset) + } + + return result, nil + } + + // Unlimited read (backward compatibility path) content := make([]byte, info.Size()) _, err = file.Read(content) if err != nil { diff --git a/pkg/tools/builtin/file_test.go b/pkg/tools/builtin/file_test.go index dd2e82ea..c8047fdf 100644 --- a/pkg/tools/builtin/file_test.go +++ b/pkg/tools/builtin/file_test.go @@ -873,3 +873,529 @@ func TestFileTool_DirectoryPermissions(t *testing.T) { }) } } + +// Tests for line-limited reading functionality + +func TestFileTool_ReadWithMaxLines(t *testing.T) { + tmpDir := t.TempDir() + testFile := filepath.Join(tmpDir, "test.txt") + + // Create a file with 10 lines + content := "line 1\nline 2\nline 3\nline 4\nline 5\nline 6\nline 7\nline 8\nline 9\nline 10" + err := os.WriteFile(testFile, []byte(content), 0644) + if err != nil { + t.Fatalf("Failed to create test file: %v", err) + } + + tool := NewFileTool() + ctx := context.Background() + + tests := []struct { + name string + maxLines int + expectedLines int + expectedContent string + expectTruncated bool + }{ + { + name: "read 5 lines from 10", + maxLines: 5, + expectedLines: 5, + expectedContent: "line 1\nline 2\nline 3\nline 4\nline 5", + expectTruncated: true, + }, + { + name: "read all lines (max_lines equals total)", + maxLines: 10, + expectedLines: 10, + expectedContent: content, + expectTruncated: false, + }, + { + name: "read more lines than available", + maxLines: 20, + expectedLines: 10, + expectedContent: content, + expectTruncated: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result, err := tool.Execute(ctx, map[string]interface{}{ + "operation": "read", + "path": testFile, + "max_lines": float64(tt.maxLines), // JSON unmarshaling produces float64 + }) + if err != nil { + t.Fatalf("Execute() error = %v", err) + } + + if success, ok := result["success"].(bool); !ok || !success { + t.Errorf("Read failed: %v", result) + } + + readContent, ok := result["content"].(string) + if !ok { + t.Fatal("Content is not a string") + } + + if readContent != tt.expectedContent { + t.Errorf("Content = %q, want %q", readContent, tt.expectedContent) + } + + // Check metadata + metadata, ok := result["metadata"].(map[string]interface{}) + if !ok { + t.Fatal("Metadata is missing or not a map") + } + + truncated, ok := metadata["truncated"].(bool) + if !ok { + t.Fatal("Metadata truncated field missing or not bool") + } + if truncated != tt.expectTruncated { + t.Errorf("Truncated = %v, want %v", truncated, tt.expectTruncated) + } + + linesShown, ok := metadata["lines_shown"].(int) + if !ok { + t.Fatal("Metadata lines_shown field missing or not int") + } + if linesShown != tt.expectedLines { + t.Errorf("lines_shown = %d, want %d", linesShown, tt.expectedLines) + } + + totalLines, ok := metadata["total_lines"].(int) + if !ok { + t.Fatal("Metadata total_lines field missing or not int") + } + if totalLines != 10 { + t.Errorf("total_lines = %d, want 10", totalLines) + } + }) + } +} + +func TestFileTool_ReadWithOffset(t *testing.T) { + tmpDir := t.TempDir() + testFile := filepath.Join(tmpDir, "test.txt") + + // Create a file with 10 lines + content := "line 1\nline 2\nline 3\nline 4\nline 5\nline 6\nline 7\nline 8\nline 9\nline 10" + err := os.WriteFile(testFile, []byte(content), 0644) + if err != nil { + t.Fatalf("Failed to create test file: %v", err) + } + + tool := NewFileTool() + ctx := context.Background() + + tests := []struct { + name string + offset int + expectedContent string + expectedLines int + expectMetadata bool + }{ + { + name: "skip 5 lines", + offset: 5, + expectedContent: "line 6\nline 7\nline 8\nline 9\nline 10", + expectedLines: 5, + expectMetadata: true, + }, + { + name: "skip 9 lines (read last line)", + offset: 9, + expectedContent: "line 10", + expectedLines: 1, + expectMetadata: true, + }, + { + name: "offset equals file length", + offset: 10, + expectedContent: "", + expectedLines: 0, + expectMetadata: true, + }, + { + name: "offset beyond file length", + offset: 20, + expectedContent: "", + expectedLines: 0, + expectMetadata: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result, err := tool.Execute(ctx, map[string]interface{}{ + "operation": "read", + "path": testFile, + "offset": float64(tt.offset), + }) + if err != nil { + t.Fatalf("Execute() error = %v", err) + } + + if success, ok := result["success"].(bool); !ok || !success { + t.Errorf("Read failed: %v", result) + } + + readContent, ok := result["content"].(string) + if !ok { + t.Fatal("Content is not a string") + } + + if readContent != tt.expectedContent { + t.Errorf("Content = %q, want %q", readContent, tt.expectedContent) + } + + // Check metadata if expected + if tt.expectMetadata { + metadata, ok := result["metadata"].(map[string]interface{}) + if !ok { + t.Fatal("Metadata is missing or not a map") + } + + linesShown, ok := metadata["lines_shown"].(int) + if !ok { + t.Fatal("Metadata lines_shown field missing or not int") + } + if linesShown != tt.expectedLines { + t.Errorf("lines_shown = %d, want %d", linesShown, tt.expectedLines) + } + + startLine, ok := metadata["start_line"].(int) + if !ok { + t.Fatal("Metadata start_line field missing or not int") + } + if startLine != tt.offset { + t.Errorf("start_line = %d, want %d", startLine, tt.offset) + } + } + }) + } +} + +func TestFileTool_ReadWithMaxLinesAndOffset(t *testing.T) { + tmpDir := t.TempDir() + testFile := filepath.Join(tmpDir, "test.txt") + + // Create a file with 10 lines + content := "line 1\nline 2\nline 3\nline 4\nline 5\nline 6\nline 7\nline 8\nline 9\nline 10" + err := os.WriteFile(testFile, []byte(content), 0644) + if err != nil { + t.Fatalf("Failed to create test file: %v", err) + } + + tool := NewFileTool() + ctx := context.Background() + + tests := []struct { + name string + offset int + maxLines int + expectedContent string + expectedLines int + expectTruncated bool + }{ + { + name: "skip 3, read 4", + offset: 3, + maxLines: 4, + expectedContent: "line 4\nline 5\nline 6\nline 7", + expectedLines: 4, + expectTruncated: true, + }, + { + name: "skip 5, read 10 (but only 5 available)", + offset: 5, + maxLines: 10, + expectedContent: "line 6\nline 7\nline 8\nline 9\nline 10", + expectedLines: 5, + expectTruncated: false, + }, + { + name: "skip 8, read 1", + offset: 8, + maxLines: 1, + expectedContent: "line 9", + expectedLines: 1, + expectTruncated: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result, err := tool.Execute(ctx, map[string]interface{}{ + "operation": "read", + "path": testFile, + "offset": float64(tt.offset), + "max_lines": float64(tt.maxLines), + }) + if err != nil { + t.Fatalf("Execute() error = %v", err) + } + + if success, ok := result["success"].(bool); !ok || !success { + t.Errorf("Read failed: %v", result) + } + + readContent, ok := result["content"].(string) + if !ok { + t.Fatal("Content is not a string") + } + + if readContent != tt.expectedContent { + t.Errorf("Content = %q, want %q", readContent, tt.expectedContent) + } + + // Check metadata + metadata, ok := result["metadata"].(map[string]interface{}) + if !ok { + t.Fatal("Metadata is missing or not a map") + } + + truncated, ok := metadata["truncated"].(bool) + if !ok { + t.Fatal("Metadata truncated field missing or not bool") + } + if truncated != tt.expectTruncated { + t.Errorf("Truncated = %v, want %v", truncated, tt.expectTruncated) + } + + linesShown, ok := metadata["lines_shown"].(int) + if !ok { + t.Fatal("Metadata lines_shown field missing or not int") + } + if linesShown != tt.expectedLines { + t.Errorf("lines_shown = %d, want %d", linesShown, tt.expectedLines) + } + }) + } +} + +func TestFileTool_ParameterValidation(t *testing.T) { + tmpDir := t.TempDir() + testFile := filepath.Join(tmpDir, "test.txt") + + err := os.WriteFile(testFile, []byte("test content"), 0644) + if err != nil { + t.Fatalf("Failed to create test file: %v", err) + } + + tool := NewFileTool() + ctx := context.Background() + + tests := []struct { + name string + maxLines interface{} + offset interface{} + expectError bool + errorField string + }{ + { + name: "negative max_lines", + maxLines: float64(-5), + expectError: true, + errorField: "max_lines", + }, + { + name: "negative offset", + offset: float64(-3), + expectError: true, + errorField: "offset", + }, + { + name: "invalid max_lines type (string)", + maxLines: "not a number", + expectError: true, + errorField: "max_lines", + }, + { + name: "invalid offset type (string)", + offset: "not a number", + expectError: true, + errorField: "offset", + }, + { + name: "valid max_lines as int", + maxLines: 5, + expectError: false, + }, + { + name: "valid offset as int", + offset: 2, + expectError: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + inputs := map[string]interface{}{ + "operation": "read", + "path": testFile, + } + if tt.maxLines != nil { + inputs["max_lines"] = tt.maxLines + } + if tt.offset != nil { + inputs["offset"] = tt.offset + } + + result, err := tool.Execute(ctx, inputs) + + if tt.expectError { + if err == nil { + t.Error("Expected error but got none") + } + } else { + if err != nil { + t.Errorf("Unexpected error: %v", err) + } + if success, ok := result["success"].(bool); !ok || !success { + t.Errorf("Read failed: %v", result) + } + } + }) + } +} + +func TestFileTool_BackwardCompatibility(t *testing.T) { + tmpDir := t.TempDir() + testFile := filepath.Join(tmpDir, "test.txt") + + content := "line 1\nline 2\nline 3\nline 4\nline 5" + err := os.WriteFile(testFile, []byte(content), 0644) + if err != nil { + t.Fatalf("Failed to create test file: %v", err) + } + + tool := NewFileTool() + ctx := context.Background() + + t.Run("read without parameters returns full file", func(t *testing.T) { + result, err := tool.Execute(ctx, map[string]interface{}{ + "operation": "read", + "path": testFile, + }) + if err != nil { + t.Fatalf("Execute() error = %v", err) + } + + if success, ok := result["success"].(bool); !ok || !success { + t.Errorf("Read failed: %v", result) + } + + readContent, ok := result["content"].(string) + if !ok { + t.Fatal("Content is not a string") + } + + if readContent != content { + t.Errorf("Content = %q, want %q", readContent, content) + } + + // Should not have metadata for unlimited read + if _, hasMetadata := result["metadata"]; hasMetadata { + t.Error("Unexpected metadata for unlimited read") + } + }) + + t.Run("explicit nil max_lines means unlimited", func(t *testing.T) { + result, err := tool.Execute(ctx, map[string]interface{}{ + "operation": "read", + "path": testFile, + "max_lines": nil, + }) + if err != nil { + t.Fatalf("Execute() error = %v", err) + } + + if success, ok := result["success"].(bool); !ok || !success { + t.Errorf("Read failed: %v", result) + } + + readContent, ok := result["content"].(string) + if !ok { + t.Fatal("Content is not a string") + } + + if readContent != content { + t.Errorf("Content = %q, want %q", readContent, content) + } + }) +} + +func TestFileTool_EmptyFileWithLimits(t *testing.T) { + tmpDir := t.TempDir() + testFile := filepath.Join(tmpDir, "empty.txt") + + // Create empty file + err := os.WriteFile(testFile, []byte(""), 0644) + if err != nil { + t.Fatalf("Failed to create test file: %v", err) + } + + tool := NewFileTool() + ctx := context.Background() + + tests := []struct { + name string + maxLines int + offset int + }{ + {"empty with max_lines", 10, 0}, + {"empty with offset", 0, 5}, + {"empty with both", 10, 5}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + inputs := map[string]interface{}{ + "operation": "read", + "path": testFile, + } + if tt.maxLines > 0 { + inputs["max_lines"] = float64(tt.maxLines) + } + if tt.offset > 0 { + inputs["offset"] = float64(tt.offset) + } + + result, err := tool.Execute(ctx, inputs) + if err != nil { + t.Fatalf("Execute() error = %v", err) + } + + if success, ok := result["success"].(bool); !ok || !success { + t.Errorf("Read failed: %v", result) + } + + readContent, ok := result["content"].(string) + if !ok { + t.Fatal("Content is not a string") + } + + if readContent != "" { + t.Errorf("Expected empty content, got %q", readContent) + } + + // Check metadata indicates empty file + metadata, ok := result["metadata"].(map[string]interface{}) + if !ok { + t.Fatal("Metadata is missing or not a map") + } + + linesShown, ok := metadata["lines_shown"].(int) + if !ok { + t.Fatal("Metadata lines_shown field missing or not int") + } + if linesShown != 0 { + t.Errorf("lines_shown = %d, want 0", linesShown) + } + }) + } +}