From 43307c43d32ec15d3c170ed85f10f5d11db372fd Mon Sep 17 00:00:00 2001 From: ml Date: Tue, 25 Aug 2026 19:10:35 +0200 Subject: [PATCH] fix: allow llm to include gitignore results in grep tool --- internal/tool/gitignore.go | 44 +++++++++++++++----- internal/tool/gitignore_test.go | 72 +++++++++++++++++++++++++++++++++ internal/tool/search.go | 34 +++++++++------- internal/tool/search_test.go | 3 ++ 4 files changed, 130 insertions(+), 23 deletions(-) diff --git a/internal/tool/gitignore.go b/internal/tool/gitignore.go index ca26d02..67d8641 100644 --- a/internal/tool/gitignore.go +++ b/internal/tool/gitignore.go @@ -103,6 +103,12 @@ func loadMergedIgnore(dir string) (*GitIgnore, error) { return merged, nil } +// loadLLMIgnore loads only the agent-specific ignore rules. It is used when a +// search explicitly opts into gitignored files, so .llmignore remains enforced. +func loadLLMIgnore(dir string) (*GitIgnore, error) { + return LoadGitIgnore(filepath.Join(dir, ".llmignore")) +} + // Matches checks whether the given relative path (e.g. "cmd/late/main.go") // should be ignored. isDir should be true for directories. // Implements "last matching pattern wins" — negated patterns (!) can @@ -223,14 +229,21 @@ func getRepoRoot() (string, *GitIgnore) { return cachedRepoRoot, cachedGitIgnore } -// getGitIgnoreForPath returns the gitignore and its originating directory -// applicable to the given search path. +// getGitIgnoreForPath returns the merged ignore rules and their originating +// directory applicable to the given search path. // -// It first walks upward from searchPath looking for nested .gitignore files, -// which is essential for monorepos where sub-projects define their own ignore -// rules. If no nested .gitignore is found, it falls back to the CWD-keyed -// cached repo root .gitignore. +// It first walks upward from searchPath looking for nested .gitignore or +// .llmignore files, which is essential for monorepos where sub-projects define +// their own ignore rules. If none is found, it falls back to the CWD-keyed +// cached repo root rules. func getGitIgnoreForPath(searchPath string) (*GitIgnore, string) { + return getIgnoreForPath(searchPath, false) +} + +// getIgnoreForPath returns the ignore rules applicable to searchPath. When +// includeGitignored is true, only .llmignore rules are loaded; otherwise the +// existing merged .gitignore + .llmignore behavior is preserved. +func getIgnoreForPath(searchPath string, includeGitignored bool) (*GitIgnore, string) { absPath, err := filepath.Abs(searchPath) if err != nil { return nil, "" @@ -239,12 +252,17 @@ func getGitIgnoreForPath(searchPath string) (*GitIgnore, string) { // Prime the CWD-keyed cache so we know the repo root boundary. cachedRoot, _ := getRepoRoot() - // Walk upward from searchPath looking for a nested .gitignore. + // Walk upward from searchPath looking for applicable nested ignore rules. // Return the closest one found (with its directory as the root so that // relative path computation in matchesGitIgnore is correct). dir := absPath for { - gi, err := loadMergedIgnore(dir) + var gi *GitIgnore + if includeGitignored { + gi, err = loadLLMIgnore(dir) + } else { + gi, err = loadMergedIgnore(dir) + } if err == nil && gi != nil { return gi, dir } @@ -263,8 +281,16 @@ func getGitIgnoreForPath(searchPath string) (*GitIgnore, string) { dir = parent } - // Fall back to cached repo root .gitignore + // Fall back to the repo root rules. The normal path uses the cache; the + // opt-in path must load .llmignore separately to avoid applying .gitignore. root, gi := getRepoRoot() + if includeGitignored && root != "" { + li, err := loadLLMIgnore(root) + if err == nil { + return li, root + } + return nil, root + } return gi, root } diff --git a/internal/tool/gitignore_test.go b/internal/tool/gitignore_test.go index 1262366..f9b0aea 100644 --- a/internal/tool/gitignore_test.go +++ b/internal/tool/gitignore_test.go @@ -366,6 +366,78 @@ func TestSearchTool_GitIgnoreDirectorySkipped(t *testing.T) { } } +func TestSearchTool_IncludeGitignored(t *testing.T) { + ResetGitIgnoreCache() + + dir := t.TempDir() + os.MkdirAll(filepath.Join(dir, ".git"), 0755) + os.WriteFile(filepath.Join(dir, ".gitignore"), []byte("temp/\n"), 0644) + os.MkdirAll(filepath.Join(dir, "temp"), 0755) + os.WriteFile(filepath.Join(dir, "temp", "reference.go"), []byte("package reference\n"), 0644) + + tool := &SearchTool{} + args := json.RawMessage(`{"pattern":"package","path":"` + filepath.Join(dir, "temp") + `","include_gitignored":true}`) + result, err := tool.Execute(context.Background(), args) + if err != nil { + t.Fatal(err) + } + + if !strings.Contains(result, "reference.go") { + t.Errorf("expected gitignored reference.go when include_gitignored is true, got: %q", result) + } +} + +func TestSearchTool_IncludeGitignoredStillHonorsLlmIgnore(t *testing.T) { + ResetGitIgnoreCache() + + dir := t.TempDir() + os.MkdirAll(filepath.Join(dir, ".git"), 0755) + os.WriteFile(filepath.Join(dir, ".gitignore"), []byte("temp/\n"), 0644) + os.WriteFile(filepath.Join(dir, ".llmignore"), []byte("temp/hidden.go\n"), 0644) + os.MkdirAll(filepath.Join(dir, "temp"), 0755) + os.WriteFile(filepath.Join(dir, "temp", "reference.go"), []byte("package reference\n"), 0644) + os.WriteFile(filepath.Join(dir, "temp", "hidden.go"), []byte("package hidden\n"), 0644) + + tool := &SearchTool{} + args := json.RawMessage(`{"pattern":"*.go","path":"` + filepath.Join(dir, "temp") + `","search_names":true,"include_gitignored":true}`) + result, err := tool.Execute(context.Background(), args) + if err != nil { + t.Fatal(err) + } + + if !strings.Contains(result, "reference.go") { + t.Errorf("expected gitignored reference.go in filename results, got: %q", result) + } + if strings.Contains(result, "hidden.go") { + t.Errorf("hidden.go should remain excluded by .llmignore, got: %q", result) + } +} + +func TestSearchTool_IncludeGitignoredStillHonorsBuiltInExclusions(t *testing.T) { + ResetGitIgnoreCache() + + dir := t.TempDir() + os.MkdirAll(filepath.Join(dir, ".git"), 0755) + os.MkdirAll(filepath.Join(dir, "node_modules", "dependency"), 0755) + os.WriteFile(filepath.Join(dir, ".git", "internal.txt"), []byte("search sentinel\n"), 0644) + os.WriteFile(filepath.Join(dir, "node_modules", "dependency", "index.js"), []byte("search sentinel\n"), 0644) + os.WriteFile(filepath.Join(dir, "visible.txt"), []byte("search sentinel\n"), 0644) + + tool := &SearchTool{} + args := json.RawMessage(`{"pattern":"search sentinel","path":"` + dir + `","include_gitignored":true}`) + result, err := tool.Execute(context.Background(), args) + if err != nil { + t.Fatal(err) + } + + if !strings.Contains(result, "visible.txt") { + t.Errorf("expected visible.txt in results, got: %q", result) + } + if strings.Contains(result, "internal.txt") || strings.Contains(result, "index.js") { + t.Errorf("built-in exclusions should remain enforced, got: %q", result) + } +} + func TestSearchTool_NoGitIgnoreDir(t *testing.T) { ResetGitIgnoreCache() diff --git a/internal/tool/search.go b/internal/tool/search.go index 7b30502..e3adf5e 100644 --- a/internal/tool/search.go +++ b/internal/tool/search.go @@ -24,7 +24,7 @@ func (t *SearchTool) Name() string { return "search_tool" } func (t *SearchTool) Description() string { return "PREFERRED over bash grep/find/rg. " + "Search files by regex/literal pattern or by name glob. Returns {path, line, content}. " + - "Honors .gitignore, permission gates, and output caps. " + + "Honors .gitignore and .llmignore, permission gates, and output caps. " + "Modes: files_with_matches (paths), content (lines+numbers), count (counts). " + "Set search_names:true to match filenames by glob (e.g. '*.go') instead of searching contents." } @@ -76,6 +76,10 @@ func (t *SearchTool) Parameters() json.RawMessage { "recursive": { "type": "boolean", "description": "Search subdirectories (default: true)." + }, + "include_gitignored": { + "type": "boolean", + "description": "If true, include files excluded by .gitignore (default: false). .llmignore and built-in exclusions remain enforced. Prefer a narrow path when enabling this." } }, "required": ["pattern"] @@ -84,17 +88,18 @@ func (t *SearchTool) Parameters() json.RawMessage { func (t *SearchTool) Execute(ctx context.Context, args json.RawMessage) (string, error) { var params struct { - Pattern string `json:"pattern"` - Path string `json:"path"` - Include string `json:"include"` - Exclude string `json:"exclude"` - OutputMode string `json:"output_mode"` - CaseSensitive bool `json:"case_sensitive"` - FixedStrings bool `json:"fixed_strings"` - SearchNames bool `json:"search_names"` - ContextLines int `json:"context_lines"` - MaxResults int `json:"max_results"` - Recursive bool `json:"recursive"` + Pattern string `json:"pattern"` + Path string `json:"path"` + Include string `json:"include"` + Exclude string `json:"exclude"` + OutputMode string `json:"output_mode"` + CaseSensitive bool `json:"case_sensitive"` + FixedStrings bool `json:"fixed_strings"` + SearchNames bool `json:"search_names"` + ContextLines int `json:"context_lines"` + MaxResults int `json:"max_results"` + Recursive bool `json:"recursive"` + IncludeGitignored bool `json:"include_gitignored"` } if err := json.Unmarshal(args, ¶ms); err != nil { return "", fmt.Errorf("invalid search parameters: %w", err) @@ -136,8 +141,9 @@ func (t *SearchTool) Execute(ctx context.Context, args json.RawMessage) (string, searchPath = params.Path } - // Load .gitignore if available (cached per process from CWD) - gi, repoRoot := getGitIgnoreForPath(searchPath) + // Load the applicable ignore rules. include_gitignored bypasses only + // .gitignore; .llmignore remains enforced as an agent-specific policy. + gi, repoRoot := getIgnoreForPath(searchPath, params.IncludeGitignored) // --- search_names: filename glob matching fast path --- if params.SearchNames { diff --git a/internal/tool/search_test.go b/internal/tool/search_test.go index e103e08..514f076 100644 --- a/internal/tool/search_test.go +++ b/internal/tool/search_test.go @@ -58,6 +58,9 @@ func TestSearchTool_Parameters_HasRequiredFields(t *testing.T) { if len(schema.Required) != 1 || schema.Required[0] != "pattern" { t.Errorf("Required = %v, want [\"pattern\"]", schema.Required) } + if property, ok := schema.Properties["include_gitignored"]; !ok || property.Type != "boolean" { + t.Errorf("include_gitignored property = %v, %v; want boolean property", property, ok) + } } func TestSearchTool_RequiresConfirmation(t *testing.T) {