Merge pull request #344 from rumpl/fix-exclude-patterns

Fix the exclude patterns

Author: rumpl

pkg/tools/builtin/filesystem.go

@@ -926,14 +926,20 @@ func (t *FilesystemTool) handleSearchFiles(_ context.Context, toolCall tools.Too
 			return nil // Skip disallowed paths
 		}
 
-		// Check exclude patterns
+		// Check exclude patterns against relative path from search root
+		relPath, err := filepath.Rel(args.Path, path)
+		if err != nil {
+			return nil
+		}
+
 		for _, exclude := range args.ExcludePatterns {
-			if match(exclude, filepath.Base(path)) {
+			if matchExcludePattern(exclude, relPath) {
+				if d.IsDir() {
+					return fs.SkipDir
+				}
 				return nil
 			}
 		}
-
-		// Case-insensitive match
 		if match(pattern, filepath.Base(path)) {
 			matches = append(matches, path)
 		}
@@ -980,21 +986,34 @@ func (t *FilesystemTool) handleSearchFilesContent(_ context.Context, toolCall to
 	var results []string
 
 	err := filepath.WalkDir(args.Path, func(path string, d fs.DirEntry, err error) error {
-		if err != nil || d.IsDir() {
+		if err != nil {
 			return nil
 		}
 
 		if err := t.isPathAllowed(path); err != nil {
 			return nil
 		}
 
-		// Check exclude patterns
+		// Check exclude patterns against relative path from search root
+		relPath, err := filepath.Rel(args.Path, path)
+		if err != nil {
+			return nil
+		}
+
 		for _, exclude := range args.ExcludePatterns {
-			if match(exclude, filepath.Base(path)) {
-				return nil
+			if matchExcludePattern(exclude, relPath) {
+				if d.IsDir() {
+					return fs.SkipDir // Skip entire directory
+				}
+				return nil // Skip this file
 			}
 		}
 
+		// Only process files, not directories
+		if d.IsDir() {
+			return nil
+		}
+
 		content, err := os.ReadFile(path)
 		if err != nil {
 			return nil
@@ -1077,6 +1096,47 @@ func (t *FilesystemTool) Stop() error {
 	return nil
 }
 
+// matchExcludePattern checks if a path should be excluded based on the exclude pattern
+// It supports glob patterns and directory wildcards like .git/*
+func matchExcludePattern(pattern, relPath string) bool {
+	// Normalize path separators to forward slashes for consistent matching
+	normalizedPath := filepath.ToSlash(relPath)
+	normalizedPattern := filepath.ToSlash(pattern)
+
+	// Handle directory patterns ending with /*
+	if strings.HasSuffix(normalizedPattern, "/*") {
+		dirPattern := strings.TrimSuffix(normalizedPattern, "/*")
+		// Check if path starts with the directory pattern
+		if strings.HasPrefix(normalizedPath, dirPattern+"/") || normalizedPath == dirPattern {
+			return true
+		}
+	}
+
+	// Try glob pattern matching on the full relative path
+	matched, _ := filepath.Match(normalizedPattern, normalizedPath)
+	if matched {
+		return true
+	}
+
+	// Try glob pattern matching on just the base name for backwards compatibility
+	matched, _ = filepath.Match(normalizedPattern, filepath.Base(normalizedPath))
+	if matched {
+		return true
+	}
+
+	// Check if pattern matches any parent directory path
+	pathParts := strings.Split(normalizedPath, "/")
+	for i := range pathParts {
+		subPath := strings.Join(pathParts[:i+1], "/")
+		matched, _ := filepath.Match(normalizedPattern, subPath)
+		if matched {
+			return true
+		}
+	}
+
+	return false
+}
+
 func match(pattern, name string) bool {
 	matched, _ := filepath.Match(pattern, name)
 	if matched {

pkg/tools/builtin/filesystem_test.go

@@ -841,3 +841,84 @@ func TestFilesystemTool_FilterTools(t *testing.T) {
 	require.Equal(t, "list_allowed_directories", tools[0].Function.Name)
 	require.NotNil(t, tools[0].Handler)
 }
+
+func TestMatchExcludePattern(t *testing.T) {
+	tests := []struct {
+		name     string
+		pattern  string
+		relPath  string
+		expected bool
+	}{
+		// Directory wildcard patterns
+		{
+			name:     "matches directory with wildcard",
+			pattern:  ".git/*",
+			relPath:  ".git/config",
+			expected: true,
+		},
+		{
+			name:     "matches directory itself with wildcard",
+			pattern:  ".git/*",
+			relPath:  ".git",
+			expected: true,
+		},
+		{
+			name:     "matches nested file with directory wildcard",
+			pattern:  ".git/*",
+			relPath:  ".git/hooks/pre-commit",
+			expected: true,
+		},
+		{
+			name:     "does not match different directory",
+			pattern:  ".git/*",
+			relPath:  "src/main.go",
+			expected: false,
+		},
+		// Glob patterns on full path
+		{
+			name:     "matches full path glob",
+			pattern:  "*.log",
+			relPath:  "debug.log",
+			expected: true,
+		},
+		{
+			name:     "matches nested file glob",
+			pattern:  "*.log",
+			relPath:  "logs/debug.log",
+			expected: true,
+		},
+		{
+			name:     "does not match different extension",
+			pattern:  "*.log",
+			relPath:  "main.go",
+			expected: false,
+		},
+		// Base name matching for backwards compatibility
+		{
+			name:     "matches base name glob",
+			pattern:  "*.tmp",
+			relPath:  "cache/temp.tmp",
+			expected: true,
+		},
+		{
+			name:     "matches base name exact",
+			pattern:  "README.md",
+			relPath:  "docs/README.md",
+			expected: true,
+		},
+		// Parent directory matching
+		{
+			name:     "matches parent directory",
+			pattern:  "node_modules",
+			relPath:  "node_modules/package/file.js",
+			expected: true,
+		},
+	}
+
+	for _, tc := range tests {
+		t.Run(tc.name, func(t *testing.T) {
+			result := matchExcludePattern(tc.pattern, tc.relPath)
+			assert.Equal(t, tc.expected, result, "Pattern: %s, Path: %s, IsDir: %v", tc.pattern, tc.relPath)
+		})
+	}
+}