Skip to content

Commit 1db8134

Browse files
authored
Merge pull request #1264 from rumpl/agent/remove-search-files
Remove search_files tool from filesystem builtin
2 parents 78ee99b + fa58f11 commit 1db8134

File tree

10 files changed

+98
-407
lines changed

10 files changed

+98
-407
lines changed

e2e/testdata/cassettes/TestExec_Anthropic_ToolCall.yaml

Lines changed: 16 additions & 16 deletions
Large diffs are not rendered by default.

e2e/testdata/cassettes/TestExec_Gemini_ToolCall.yaml

Lines changed: 6 additions & 6 deletions
Large diffs are not rendered by default.

e2e/testdata/cassettes/TestExec_Mistral_ToolCall.yaml

Lines changed: 9 additions & 9 deletions
Large diffs are not rendered by default.

e2e/testdata/cassettes/TestExec_OpenAI_HideToolCalls.yaml

Lines changed: 20 additions & 20 deletions
Large diffs are not rendered by default.

e2e/testdata/cassettes/TestExec_OpenAI_ToolCall.yaml

Lines changed: 20 additions & 20 deletions
Large diffs are not rendered by default.

pkg/tools/builtin/filesystem.go

Lines changed: 0 additions & 75 deletions
Original file line numberDiff line numberDiff line change
@@ -12,8 +12,6 @@ import (
1212
"regexp"
1313
"strings"
1414

15-
"github.com/bmatcuk/doublestar/v4"
16-
1715
"github.com/docker/cagent/pkg/fsx"
1816
"github.com/docker/cagent/pkg/tools"
1917
)
@@ -25,7 +23,6 @@ const (
2523
ToolNameWriteFile = "write_file"
2624
ToolNameDirectoryTree = "directory_tree"
2725
ToolNameListDirectory = "list_directory"
28-
ToolNameSearchFiles = "search_files"
2926
ToolNameSearchFilesContent = "search_files_content"
3027
)
3128

@@ -114,12 +111,6 @@ type ReadMultipleFilesMeta struct {
114111
Files []ReadFileMeta `json:"files"`
115112
}
116113

117-
type SearchFilesArgs struct {
118-
Path string `json:"path" jsonschema:"The starting directory path"`
119-
Pattern string `json:"pattern" jsonschema:"The glob pattern to match file names against"`
120-
ExcludePatterns []string `json:"excludePatterns,omitempty" jsonschema:"Patterns to exclude from search"`
121-
}
122-
123114
type SearchFilesContentArgs struct {
124115
Path string `json:"path" jsonschema:"The starting directory path"`
125116
Query string `json:"query" jsonschema:"The text or regex pattern to search for"`
@@ -243,18 +234,6 @@ func (t *FilesystemTool) Tools(context.Context) ([]tools.Tool, error) {
243234
Title: "Read Multiple Files",
244235
},
245236
},
246-
{
247-
Name: ToolNameSearchFiles,
248-
Category: "filesystem",
249-
Description: "Recursively search for files and directories matching a pattern. Prints the full paths of matching files and the total number of files found. The pattern syntax is the same as Go's filepath.Match.",
250-
Parameters: tools.MustSchemaFor[SearchFilesArgs](),
251-
OutputSchema: tools.MustSchemaFor[string](),
252-
Handler: NewHandler(t.handleSearchFiles),
253-
Annotations: tools.ToolAnnotations{
254-
ReadOnlyHint: true,
255-
Title: "Search Files",
256-
},
257-
},
258237
{
259238
Name: ToolNameSearchFilesContent,
260239
Category: "filesystem",
@@ -543,55 +522,6 @@ func (t *FilesystemTool) handleReadMultipleFiles(ctx context.Context, args ReadM
543522
}, nil
544523
}
545524

546-
func (t *FilesystemTool) handleSearchFiles(_ context.Context, args SearchFilesArgs) (*tools.ToolCallResult, error) {
547-
resolvedPath := t.resolvePath(args.Path)
548-
549-
var matches []string
550-
551-
err := filepath.WalkDir(resolvedPath, func(path string, d fs.DirEntry, err error) error {
552-
if err != nil {
553-
return nil // Skip errors and continue
554-
}
555-
556-
// Check VCS ignore rules
557-
if t.shouldIgnorePath(path) {
558-
if d.IsDir() {
559-
return fs.SkipDir
560-
}
561-
return nil
562-
}
563-
564-
// Check exclude patterns against relative path from search root
565-
relPath, err := filepath.Rel(resolvedPath, path)
566-
if err != nil {
567-
return nil
568-
}
569-
570-
for _, exclude := range args.ExcludePatterns {
571-
if matchExcludePattern(exclude, relPath) {
572-
if d.IsDir() {
573-
return fs.SkipDir
574-
}
575-
return nil
576-
}
577-
}
578-
if match(args.Pattern, filepath.Base(path)) && !d.IsDir() {
579-
matches = append(matches, path)
580-
}
581-
582-
return nil
583-
})
584-
if err != nil {
585-
return tools.ResultError(fmt.Sprintf("Error searching files: %s", err)), nil
586-
}
587-
588-
if len(matches) == 0 {
589-
return tools.ResultSuccess("No files found"), nil
590-
}
591-
592-
return tools.ResultSuccess(fmt.Sprintf("%d files found:\n%s", len(matches), strings.Join(matches, "\n"))), nil
593-
}
594-
595525
func (t *FilesystemTool) handleSearchFilesContent(_ context.Context, args SearchFilesContentArgs) (*tools.ToolCallResult, error) {
596526
resolvedPath := t.resolvePath(args.Path)
597527

@@ -744,8 +674,3 @@ func matchExcludePattern(pattern, relPath string) bool {
744674

745675
return false
746676
}
747-
748-
func match(pattern, name string) bool {
749-
matched, _ := doublestar.Match(pattern, name)
750-
return matched || strings.Contains(strings.ToLower(name), strings.ToLower(pattern))
751-
}

pkg/tools/builtin/filesystem_test.go

Lines changed: 27 additions & 142 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,6 @@ import (
44
"encoding/json"
55
"os"
66
"path/filepath"
7-
"strings"
87
"testing"
98

109
"github.com/stretchr/testify/assert"
@@ -222,98 +221,6 @@ func TestFilesystemTool_EditFile(t *testing.T) {
222221
assert.Contains(t, result.Output, "old text not found")
223222
}
224223

225-
func TestFilesystemTool_SearchFiles(t *testing.T) {
226-
t.Parallel()
227-
tmpDir := t.TempDir()
228-
229-
require.NoError(t, os.WriteFile(filepath.Join(tmpDir, "Dockerfile"), []byte("FROM scratch"), 0o644))
230-
require.NoError(t, os.WriteFile(filepath.Join(tmpDir, "test.txt"), []byte("test"), 0o644))
231-
require.NoError(t, os.WriteFile(filepath.Join(tmpDir, "test.log"), []byte("log"), 0o644))
232-
require.NoError(t, os.WriteFile(filepath.Join(tmpDir, "data.txt"), []byte("data"), 0o644))
233-
234-
subDir := filepath.Join(tmpDir, "subdir")
235-
require.NoError(t, os.Mkdir(subDir, 0o755))
236-
require.NoError(t, os.WriteFile(filepath.Join(subDir, "test_sub.txt"), []byte("sub"), 0o644))
237-
238-
tool := NewFilesystemTool(tmpDir)
239-
tests := []struct {
240-
name string
241-
pattern string
242-
excludePatterns []string
243-
wantCount int
244-
wantContains []string
245-
wantNotContains []string
246-
}{
247-
{
248-
name: "no files found",
249-
pattern: "asdf",
250-
wantCount: 0,
251-
wantContains: []string{"No files found"},
252-
wantNotContains: nil,
253-
},
254-
{
255-
name: "all files found",
256-
pattern: "*",
257-
wantCount: 0,
258-
wantContains: []string{"5 files found:\n"},
259-
wantNotContains: nil,
260-
},
261-
{
262-
name: "all files found with dot",
263-
pattern: "*.*",
264-
wantCount: 0,
265-
wantContains: []string{"4 files found:\n"},
266-
wantNotContains: nil,
267-
},
268-
{
269-
name: "dockerfile pattern",
270-
pattern: "Dockerfile*",
271-
wantCount: 1,
272-
wantContains: []string{"1 files found:\n", "Dockerfile"},
273-
wantNotContains: nil,
274-
},
275-
{
276-
name: "test pattern finds all",
277-
pattern: "test",
278-
wantCount: 3,
279-
wantContains: []string{"3 files found:\n"},
280-
wantNotContains: nil,
281-
},
282-
{
283-
name: "test pattern with log exclusion",
284-
pattern: "test",
285-
excludePatterns: []string{"*.log"},
286-
wantCount: 2,
287-
wantContains: []string{"test.txt"},
288-
wantNotContains: []string{"test.log"},
289-
},
290-
}
291-
292-
for _, tt := range tests {
293-
t.Run(tt.name, func(t *testing.T) {
294-
t.Parallel()
295-
result, err := tool.handleSearchFiles(t.Context(), SearchFilesArgs{
296-
Path: ".",
297-
Pattern: tt.pattern,
298-
ExcludePatterns: tt.excludePatterns,
299-
})
300-
require.NoError(t, err)
301-
302-
for _, want := range tt.wantContains {
303-
assert.Contains(t, result.Output, want)
304-
}
305-
for _, notWant := range tt.wantNotContains {
306-
assert.NotContains(t, result.Output, notWant)
307-
}
308-
309-
if tt.wantCount > 0 {
310-
lines := strings.Split(strings.TrimSpace(result.Output), "\n")
311-
assert.Len(t, lines, tt.wantCount+1) // +1 for the header line
312-
}
313-
})
314-
}
315-
}
316-
317224
func TestFilesystemTool_SearchFilesContent(t *testing.T) {
318225
t.Parallel()
319226
tmpDir := t.TempDir()
@@ -356,28 +263,6 @@ func TestFilesystemTool_SearchFilesContent(t *testing.T) {
356263
assert.Contains(t, result.Output, "Invalid regex pattern")
357264
}
358265

359-
func TestFilesystemTool_SearchFiles_RecursivePattern(t *testing.T) {
360-
t.Parallel()
361-
tmpDir := t.TempDir()
362-
363-
require.NoError(t, os.Mkdir(filepath.Join(tmpDir, "child"), 0o755))
364-
require.NoError(t, os.WriteFile(filepath.Join(tmpDir, "first.txt"), []byte("first"), 0o644))
365-
require.NoError(t, os.WriteFile(filepath.Join(tmpDir, "ignored"), []byte("ignored"), 0o644))
366-
require.NoError(t, os.WriteFile(filepath.Join(tmpDir, "child", "second.txt"), []byte("second"), 0o644))
367-
require.NoError(t, os.WriteFile(filepath.Join(tmpDir, "child", "third.txt"), []byte("third"), 0o644))
368-
require.NoError(t, os.WriteFile(filepath.Join(tmpDir, "child", "ignored"), []byte("ignored"), 0o644))
369-
370-
tool := NewFilesystemTool(tmpDir)
371-
result, err := tool.handleSearchFiles(t.Context(), SearchFilesArgs{
372-
Path: ".",
373-
Pattern: "*.txt",
374-
})
375-
require.NoError(t, err)
376-
lines := strings.Split(strings.TrimSpace(result.Output), "\n")
377-
assert.Contains(t, result.Output, "3 files found:\n")
378-
assert.Len(t, lines, 3+1) // Should find first.txt, second.txt, and third.txt
379-
}
380-
381266
func TestFilesystemTool_PostEditCommands(t *testing.T) {
382267
t.Parallel()
383268
tmpDir := t.TempDir()
@@ -530,12 +415,12 @@ func TestFilesystemTool_IgnoreVCS_Default(t *testing.T) {
530415
gitDir := filepath.Join(tmpDir, ".git")
531416
require.NoError(t, os.Mkdir(gitDir, 0o755))
532417
require.NoError(t, os.WriteFile(filepath.Join(gitDir, "config"), []byte("git config"), 0o644))
533-
require.NoError(t, os.WriteFile(filepath.Join(tmpDir, "test.txt"), []byte("test content"), 0o644))
418+
require.NoError(t, os.WriteFile(filepath.Join(tmpDir, "test.txt"), []byte("findme"), 0o644))
534419

535420
tool := NewFilesystemTool(tmpDir, WithIgnoreVCS(true))
536-
result, err := tool.handleSearchFiles(t.Context(), SearchFilesArgs{
537-
Path: ".",
538-
Pattern: "*",
421+
result, err := tool.handleSearchFilesContent(t.Context(), SearchFilesContentArgs{
422+
Path: ".",
423+
Query: "findme",
539424
})
540425
require.NoError(t, err)
541426
assert.Contains(t, result.Output, "test.txt")
@@ -548,13 +433,13 @@ func TestFilesystemTool_IgnoreVCS_Disabled(t *testing.T) {
548433

549434
gitDir := filepath.Join(tmpDir, ".git")
550435
require.NoError(t, os.Mkdir(gitDir, 0o755))
551-
require.NoError(t, os.WriteFile(filepath.Join(gitDir, "config"), []byte("git config"), 0o644))
552-
require.NoError(t, os.WriteFile(filepath.Join(tmpDir, "test.txt"), []byte("test content"), 0o644))
436+
require.NoError(t, os.WriteFile(filepath.Join(gitDir, "config"), []byte("findme"), 0o644))
437+
require.NoError(t, os.WriteFile(filepath.Join(tmpDir, "test.txt"), []byte("findme"), 0o644))
553438

554439
tool := NewFilesystemTool(tmpDir, WithIgnoreVCS(false))
555-
result, err := tool.handleSearchFiles(t.Context(), SearchFilesArgs{
556-
Path: ".",
557-
Pattern: "*",
440+
result, err := tool.handleSearchFilesContent(t.Context(), SearchFilesContentArgs{
441+
Path: ".",
442+
Query: "findme",
558443
})
559444
require.NoError(t, err)
560445
assert.Contains(t, result.Output, "test.txt")
@@ -577,19 +462,19 @@ temp_*
577462
`
578463
require.NoError(t, os.WriteFile(filepath.Join(tmpDir, ".gitignore"), []byte(gitignoreContent), 0o644))
579464

580-
require.NoError(t, os.WriteFile(filepath.Join(tmpDir, "test.txt"), []byte("test"), 0o644))
581-
require.NoError(t, os.WriteFile(filepath.Join(tmpDir, "debug.log"), []byte("log"), 0o644))
582-
require.NoError(t, os.WriteFile(filepath.Join(tmpDir, "important.log"), []byte("important"), 0o644))
583-
require.NoError(t, os.WriteFile(filepath.Join(tmpDir, "temp_file.txt"), []byte("temp"), 0o644))
465+
require.NoError(t, os.WriteFile(filepath.Join(tmpDir, "test.txt"), []byte("findme"), 0o644))
466+
require.NoError(t, os.WriteFile(filepath.Join(tmpDir, "debug.log"), []byte("findme"), 0o644))
467+
require.NoError(t, os.WriteFile(filepath.Join(tmpDir, "important.log"), []byte("findme"), 0o644))
468+
require.NoError(t, os.WriteFile(filepath.Join(tmpDir, "temp_file.txt"), []byte("findme"), 0o644))
584469
require.NoError(t, os.Mkdir(filepath.Join(tmpDir, "node_modules"), 0o755))
585-
require.NoError(t, os.WriteFile(filepath.Join(tmpDir, "node_modules", "package.json"), []byte("{}"), 0o644))
470+
require.NoError(t, os.WriteFile(filepath.Join(tmpDir, "node_modules", "package.json"), []byte("findme"), 0o644))
586471
require.NoError(t, os.Mkdir(filepath.Join(tmpDir, "build"), 0o755))
587-
require.NoError(t, os.WriteFile(filepath.Join(tmpDir, "build", "output.js"), []byte("code"), 0o644))
472+
require.NoError(t, os.WriteFile(filepath.Join(tmpDir, "build", "output.js"), []byte("findme"), 0o644))
588473

589474
tool := NewFilesystemTool(tmpDir, WithIgnoreVCS(true))
590-
result, err := tool.handleSearchFiles(t.Context(), SearchFilesArgs{
591-
Path: ".",
592-
Pattern: "*",
475+
result, err := tool.handleSearchFilesContent(t.Context(), SearchFilesContentArgs{
476+
Path: ".",
477+
Query: "findme",
593478
})
594479
require.NoError(t, err)
595480
assert.Contains(t, result.Output, "test.txt")
@@ -650,17 +535,17 @@ func TestFilesystemTool_SubdirectoryGitignorePatterns(t *testing.T) {
650535
subDir := filepath.Join(tmpDir, "subdir")
651536
require.NoError(t, os.Mkdir(subDir, 0o755))
652537
require.NoError(t, os.WriteFile(filepath.Join(subDir, ".gitignore"), []byte("*.tmp\n"), 0o644))
653-
require.NoError(t, os.WriteFile(filepath.Join(tmpDir, "root.txt"), []byte("root"), 0o644))
654-
require.NoError(t, os.WriteFile(filepath.Join(tmpDir, "root.log"), []byte("log"), 0o644)) // ignored by root .gitignore
655-
require.NoError(t, os.WriteFile(filepath.Join(tmpDir, "root.tmp"), []byte("tmp"), 0o644)) // NOT ignored (subdir .gitignore doesn't apply here)
656-
require.NoError(t, os.WriteFile(filepath.Join(subDir, "sub.txt"), []byte("sub"), 0o644))
657-
require.NoError(t, os.WriteFile(filepath.Join(subDir, "sub.log"), []byte("log"), 0o644)) // ignored by root .gitignore
658-
require.NoError(t, os.WriteFile(filepath.Join(subDir, "sub.tmp"), []byte("tmp"), 0o644)) // ignored by subdir .gitignore
538+
require.NoError(t, os.WriteFile(filepath.Join(tmpDir, "root.txt"), []byte("findme"), 0o644))
539+
require.NoError(t, os.WriteFile(filepath.Join(tmpDir, "root.log"), []byte("findme"), 0o644)) // ignored by root .gitignore
540+
require.NoError(t, os.WriteFile(filepath.Join(tmpDir, "root.tmp"), []byte("findme"), 0o644)) // NOT ignored (subdir .gitignore doesn't apply here)
541+
require.NoError(t, os.WriteFile(filepath.Join(subDir, "sub.txt"), []byte("findme"), 0o644))
542+
require.NoError(t, os.WriteFile(filepath.Join(subDir, "sub.log"), []byte("findme"), 0o644)) // ignored by root .gitignore
543+
require.NoError(t, os.WriteFile(filepath.Join(subDir, "sub.tmp"), []byte("findme"), 0o644)) // ignored by subdir .gitignore
659544

660545
tool := NewFilesystemTool(tmpDir, WithIgnoreVCS(true))
661-
result, err := tool.handleSearchFiles(t.Context(), SearchFilesArgs{
662-
Path: ".",
663-
Pattern: "*",
546+
result, err := tool.handleSearchFilesContent(t.Context(), SearchFilesContentArgs{
547+
Path: ".",
548+
Query: "findme",
664549
})
665550
require.NoError(t, err)
666551

pkg/tui/components/tool/factory.go

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,6 @@ import (
99
"github.com/docker/cagent/pkg/tui/components/tool/listdirectory"
1010
"github.com/docker/cagent/pkg/tui/components/tool/readfile"
1111
"github.com/docker/cagent/pkg/tui/components/tool/readmultiplefiles"
12-
"github.com/docker/cagent/pkg/tui/components/tool/searchfiles"
1312
"github.com/docker/cagent/pkg/tui/components/tool/shell"
1413
"github.com/docker/cagent/pkg/tui/components/tool/todotool"
1514
"github.com/docker/cagent/pkg/tui/components/tool/transfertask"
@@ -74,7 +73,6 @@ func newDefaultRegistry() *Registry {
7473
{[]string{builtin.ToolNameReadFile}, readfile.New},
7574
{[]string{builtin.ToolNameReadMultipleFiles}, readmultiplefiles.New},
7675
{[]string{builtin.ToolNameListDirectory}, listdirectory.New},
77-
{[]string{builtin.ToolNameSearchFiles}, searchfiles.New},
7876
{[]string{builtin.ToolNameShell}, shell.New},
7977
{[]string{builtin.ToolNameFetch, "category:api"}, api.New},
8078
{

0 commit comments

Comments
 (0)