diff --git a/packages/agent/modes/file_suggest.go b/packages/agent/modes/file_suggest.go index e740a5f..5d396a0 100644 --- a/packages/agent/modes/file_suggest.go +++ b/packages/agent/modes/file_suggest.go @@ -17,9 +17,21 @@ import ( // recursiveScanLimits bound the recursive walk so the picker stays // responsive in very large repos. Hitting either cap stops the walk // early; the entries gathered so far are still searchable. +// +// The entry cap is set against the real bottleneck, which is not memory +// but the per-keystroke fuzzy.Find ranking that scores every entry. A +// fileEntry is ~120 bytes all-in (40-byte struct header plus the path +// string), so even 50k entries is only ~6 MB. fuzzy.Find scales roughly +// linearly: ~2 ms at 5k, ~13 ms at 50k, ~21 ms at 100k on a typical +// laptop. 50k keeps ranking under one 60 Hz frame (~16 ms) while +// comfortably holding a large monorepo once nested .gitignore pruning +// (node_modules, build outputs, tool caches) has done its job. The +// depth cap likewise guards against pathologically deep trees; 24 +// levels is far below anything a human navigates yet still reaches the +// deeply nested source files real monorepos bury. const ( - maxRecursiveEntries = 5000 - maxRecursiveDepth = 12 + maxRecursiveEntries = 50000 + maxRecursiveDepth = 24 ) // alwaysSkipDir is never descended into during a recursive scan, @@ -195,12 +207,22 @@ func (s *fileSuggester) scanRecursive() []fileEntry { return s.cachedAll } - var ig *ignore.Gitignore + var stack *ignore.Stack if s.respectGitignore { - ig = ignore.Load(root) + stack = ignore.NewStack(root) } var all []fileEntry rootSep := strings.Count(root, string(os.PathSeparator)) + // pushed mirrors the directories whose nested .gitignore we've + // brought into scope on stack but not yet dropped. WalkDir is + // depth-first in lexical order, so before each entry we pop frames + // for directories we've finished walking (those that are no longer + // an ancestor of the current path), then push the current + // directory. Honoring nested .gitignore files is what stops a + // vendored node_modules whose ignore rule lives in a subdirectory + // (e.g. .opencode/.gitignore) from swamping the entry budget before + // the walk ever reaches the user's deeply nested source file. + var pushed []string _ = filepath.WalkDir(root, func(path string, d os.DirEntry, err error) error { if err != nil { if d != nil && d.IsDir() { @@ -215,9 +237,28 @@ func (s *fileSuggester) scanRecursive() []fileEntry { if relErr != nil { return nil } + relSlash := filepath.ToSlash(rel) if s.respectGitignore { + // Drop stack frames for directories we've finished walking: + // any pushed dir that is not an ancestor of this entry's dir. + dirSlash := relSlash + if !d.IsDir() { + if i := strings.LastIndex(relSlash, "/"); i >= 0 { + dirSlash = relSlash[:i] + } else { + dirSlash = "" + } + } + for len(pushed) > 0 { + top := pushed[len(pushed)-1] + if top == dirSlash || strings.HasPrefix(dirSlash, top+"/") { + break + } + pushed = pushed[:len(pushed)-1] + stack.Pop() + } // .gitignore patterns are matched against slash-separated paths. - if ig.Match(filepath.ToSlash(rel), d.IsDir()) { + if stack.Match(relSlash, d.IsDir()) { if d.IsDir() { return filepath.SkipDir } @@ -234,6 +275,12 @@ func (s *fileSuggester) scanRecursive() []fileEntry { if strings.Count(path, string(os.PathSeparator))-rootSep >= maxRecursiveDepth { return filepath.SkipDir } + // This directory is kept; bring its nested .gitignore (if + // any) into scope for the descendants we're about to visit. + if s.respectGitignore { + stack.Push(path, rel) + pushed = append(pushed, relSlash) + } } all = append(all, fileEntry{ name: rel, diff --git a/packages/agent/modes/file_suggest_test.go b/packages/agent/modes/file_suggest_test.go index 6a43117..d9f0866 100644 --- a/packages/agent/modes/file_suggest_test.go +++ b/packages/agent/modes/file_suggest_test.go @@ -79,6 +79,61 @@ func TestFileSuggesterPicksUpNewEntries(t *testing.T) { } } +// TestFileSuggesterRecursiveHonorsNestedGitignore reproduces the +// reported bug: a vendored tool directory (.opencode) carries its own +// .gitignore that excludes node_modules, but the repo root .gitignore +// says nothing about it. Before nested .gitignore support the +// recursive walk surfaced thousands of node_modules files and a deeply +// nested real source file (eda/rjg/enk-1150/pipeline.py) could not be +// found. The walk must prune the nested-ignored tree and surface the +// deep file. +func TestFileSuggesterRecursiveHonorsNestedGitignore(t *testing.T) { + tmp := t.TempDir() + // Root .gitignore knows nothing about node_modules. + if err := os.WriteFile(filepath.Join(tmp, ".gitignore"), []byte("dist/\n"), 0o644); err != nil { + t.Fatal(err) + } + // .opencode/.gitignore ignores its own node_modules. + opencodeNM := filepath.Join(tmp, ".opencode", "node_modules", "zod", "src") + if err := os.MkdirAll(opencodeNM, 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(tmp, ".opencode", ".gitignore"), []byte("node_modules\n"), 0o644); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(opencodeNM, "pipeline.test.ts"), []byte("x"), 0o644); err != nil { + t.Fatal(err) + } + // The deeply nested real file the user is hunting for. + deep := filepath.Join(tmp, "eda", "rjg", "enk-1150") + if err := os.MkdirAll(deep, 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(deep, "pipeline.py"), []byte("x"), 0o644); err != nil { + t.Fatal(err) + } + + s := newFileSuggester() + s.SetCWD(tmp) + s.SetRecursive(true) + + all := s.scan() + for _, e := range all { + if strings.Contains(filepath.ToSlash(e.rel), "node_modules") { + t.Fatalf("recursive scan surfaced nested-gitignored node_modules: %#v", e) + } + } + rel := filepath.Join("eda", "rjg", "enk-1150", "pipeline.py") + if !containsEntry(all, rel, false) { + t.Fatalf("recursive scan missing deep pipeline.py: %#v", all) + } + // The fuzzy query should now find the real file. + got := s.matches("@pipeline.py") + if !containsEntry(got, rel, false) { + t.Fatalf("@pipeline.py did not match the deep file: %#v", got) + } +} + func containsEntry(entries []fileEntry, name string, isDir bool) bool { for _, e := range entries { if e.name == name && e.isDir == isDir { @@ -121,6 +176,54 @@ func TestFileSuggesterFuzzyMatch(t *testing.T) { } } +// TestFileSuggesterFlatBrowseIntoDirIgnoresStaleFilter reproduces the +// reported bug: in flat (non-recursive) mode, typing "@eda" to find a +// directory then opening it with Right must show that directory's +// contents, not re-apply the "eda" filter inside it (which matches +// nothing). The interactive layer clears the @-query when descending, +// so here we model that by browsing with Right and then matching an +// empty query against the new level. +func TestFileSuggesterFlatBrowseIntoDirIgnoresStaleFilter(t *testing.T) { + tmp := t.TempDir() + // eda/rjg/enk-1150 with a file inside, plus a sibling so the filter + // is meaningful at the top level. + if err := os.MkdirAll(filepath.Join(tmp, "eda", "rjg", "enk-1150"), 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(tmp, "eda", "rjg", "enk-1150", "pipeline.py"), []byte("x"), 0o644); err != nil { + t.Fatal(err) + } + if err := os.MkdirAll(filepath.Join(tmp, "unrelated"), 0o755); err != nil { + t.Fatal(err) + } + + s := newFileSuggester() + s.SetCWD(tmp) // flat mode + + // Top level: "@eda" highlights the eda/ directory. Render populates + // lastMatches, which Right/Left act on (it runs every frame before + // key handling in the live UI). + s.lastMatches = s.matches("@eda") + if !containsEntry(s.lastMatches, "eda", true) { + t.Fatalf("@eda did not match eda/: %#v", s.lastMatches) + } + s.cursor = 0 // eda/ is the (only) match, selected. + + // Open it. After the interactive layer clears the query, the picker + // is browsing eda/ with an empty filter and must show rjg/. + if !s.Right() { + t.Fatal("Right() did not open eda/") + } + if got := s.matches("@"); !containsEntry(got, "rjg", true) { + t.Fatalf("after opening eda/, empty filter did not show rjg/: %#v", got) + } + // The stale filter would have shown nothing: confirm that's the + // behavior the fix avoids. + if got := s.matches("@eda"); len(got) != 0 { + t.Fatalf("stale @eda filter inside eda/ unexpectedly matched: %#v", got) + } +} + // TestFileSuggesterRecursiveMatch verifies recursive mode flattens the // tree and matches against the cwd-relative path, so a pattern can // span directory boundaries. diff --git a/packages/agent/modes/interactive.go b/packages/agent/modes/interactive.go index f19dbdb..01db170 100644 --- a/packages/agent/modes/interactive.go +++ b/packages/agent/modes/interactive.go @@ -1605,6 +1605,18 @@ func (i *Interactive) ctrlCExitArmed() bool { return !t.IsZero() && time.Since(t) <= ctrlCExitWindow } +// clearFileSuggestQuery strips the filter the user typed after the +// last "@", leaving the bare "@" so the picker stays open. Called when +// navigating between directory levels (Right/Left): the filter applied +// to the level the user was on, not the one being entered, so carrying +// it forward would wrongly hide the new directory's contents. +func (i *Interactive) clearFileSuggestQuery() { + val := i.ed.Value() + if idx := strings.LastIndex(val, "@"); idx >= 0 { + i.ed.SetValue(val[:idx+1]) + } +} + func (i *Interactive) handleKey(ctx context.Context, k tui.Key) (done bool) { // Any key that isn't ctrl+c invalidates an armed ctrl+c-exit, so // pressing ctrl+c then typing then ctrl+c much later doesn't quit @@ -2083,12 +2095,21 @@ func (i *Interactive) handleKey(ctx context.Context, k tui.Key) (done bool) { i.fileSuggest.Down() return false case tui.KeyRight: - // Open selected directory. - i.fileSuggest.Right() + // Open selected directory. The filter the user typed picked + // that directory at the current level; once we descend it no + // longer applies to the directory's contents, so clear it. + // Otherwise typing "@eda" then right would re-filter inside + // eda/ by "eda" and show nothing. + if i.fileSuggest.Right() { + i.clearFileSuggestQuery() + } return false case tui.KeyLeft: - // Go back to parent directory. - i.fileSuggest.Left() + // Go back to parent directory. Clear the filter for the same + // reason as Right: it was scoped to the level we just left. + if i.fileSuggest.Left() { + i.clearFileSuggestQuery() + } return false case tui.KeyEnter: if entry, ok := i.fileSuggest.SelectedEntry(i.ed.Value()); ok { diff --git a/packages/ignore/gitignore.go b/packages/ignore/gitignore.go index 8c44168..e037768 100644 --- a/packages/ignore/gitignore.go +++ b/packages/ignore/gitignore.go @@ -37,6 +37,84 @@ func Load(root string) *Gitignore { return Parse(string(data)) } +// Stack tracks the chain of .gitignore files from a walk's root down to +// the directory currently being visited. Real repositories routinely +// place a .gitignore inside a subdirectory (for example a vendored tool +// directory that ignores its own node_modules), and those nested rules +// are invisible to a single root-only matcher. During a recursive walk +// such an unhonored node_modules can swamp the entry budget before the +// walk ever reaches the files the user is actually looking for, so the +// nested files must be applied. +// +// Each frame holds the matcher parsed from a directory's .gitignore +// plus that directory's path relative to the walk root (slash +// separated, "" for the root). A nested matcher's patterns are +// evaluated against the path relative to the directory that owns them, +// matching git's own semantics. +type Stack struct { + root string + frames []stackFrame +} + +type stackFrame struct { + relDir string // dir owning the .gitignore, relative to root, slash-sep + ig *Gitignore +} + +// NewStack returns a Stack seeded with the root .gitignore (if any). +func NewStack(root string) *Stack { + s := &Stack{root: root} + s.frames = append(s.frames, stackFrame{relDir: "", ig: Load(root)}) + return s +} + +// Push loads the .gitignore in dir (an absolute path under root, with +// relDir its slash-separated path relative to root) and adds it to the +// stack. Call when the walk descends into a directory that is being +// kept; pair with Pop when leaving it. A directory with no .gitignore +// still gets a frame so the push/pop bookkeeping stays balanced. +func (s *Stack) Push(absDir, relDir string) { + data, err := os.ReadFile(filepath.Join(absDir, ".gitignore")) + var ig *Gitignore + if err != nil { + ig = &Gitignore{} + } else { + ig = Parse(string(data)) + } + s.frames = append(s.frames, stackFrame{relDir: filepath.ToSlash(relDir), ig: ig}) +} + +// Pop removes the most recently pushed frame. The seeded root frame is +// never popped. +func (s *Stack) Pop() { + if len(s.frames) > 1 { + s.frames = s.frames[:len(s.frames)-1] + } +} + +// Match reports whether rel (slash-separated, relative to the walk +// root) should be ignored, consulting every .gitignore from the root +// down to the current directory. Each frame matches against rel made +// relative to the directory that owns it; a deeper frame's later rules +// win, mirroring git's nearest-file-wins ordering. +func (s *Stack) Match(rel string, isDir bool) bool { + ignored := false + for _, f := range s.frames { + sub := rel + if f.relDir != "" { + prefix := f.relDir + "/" + if !strings.HasPrefix(rel, prefix) { + continue + } + sub = rel[len(prefix):] + } + if matched, neg := f.ig.matchResult(sub, isDir); matched { + ignored = !neg + } + } + return ignored +} + // Parse builds a matcher from raw .gitignore file contents. func Parse(data string) *Gitignore { g := &Gitignore{} @@ -71,16 +149,26 @@ func Parse(data string) *Gitignore { // ignored. Later rules win, so a trailing negation can re-include a // previously ignored path. func (g *Gitignore) Match(rel string, isDir bool) bool { - ignored := false + matched, negate := g.matchResult(rel, isDir) + return matched && !negate +} + +// matchResult reports whether any rule matched rel and, if so, whether +// the winning (last) matching rule was a negation. This lets a Stack +// combine matchers across nested .gitignore files while still honoring +// negations correctly: a nested "!keep.me" must be able to re-include a +// path a parent .gitignore excluded. +func (g *Gitignore) matchResult(rel string, isDir bool) (matched, negate bool) { for _, r := range g.rules { if r.dirOnly && !isDir { continue } if r.matchPath(rel) { - ignored = !r.negate + matched = true + negate = r.negate } } - return ignored + return matched, negate } func (r rule) matchPath(rel string) bool { diff --git a/packages/ignore/gitignore_test.go b/packages/ignore/gitignore_test.go index e455cd3..a4c5d05 100644 --- a/packages/ignore/gitignore_test.go +++ b/packages/ignore/gitignore_test.go @@ -1,6 +1,10 @@ package ignore -import "testing" +import ( + "os" + "path/filepath" + "testing" +) func TestParseAndMatch(t *testing.T) { g := Parse(lines("# comment", "", ".terraform/", ".terragrunt-cache/", "node_modules/", "*.log", "/build", "!keep.log")) @@ -40,6 +44,78 @@ func TestEmptyIgnoresNothing(t *testing.T) { } } +// TestStackHonorsNestedGitignore pins the recursive-picker bug: a +// .gitignore living inside a subdirectory (here .opencode/.gitignore +// ignoring node_modules, exactly the layout that flooded the @-picker) +// must prune that subdirectory's node_modules even though the root +// .gitignore says nothing about it. +func TestStackHonorsNestedGitignore(t *testing.T) { + root := t.TempDir() + // Root .gitignore: only build/ at root, nothing about node_modules. + if err := os.WriteFile(filepath.Join(root, ".gitignore"), []byte("build/\n"), 0o644); err != nil { + t.Fatal(err) + } + opencode := filepath.Join(root, ".opencode") + if err := os.MkdirAll(opencode, 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(opencode, ".gitignore"), []byte("node_modules\n"), 0o644); err != nil { + t.Fatal(err) + } + + s := NewStack(root) + // Before descending into .opencode the nested rule is not in scope, + // so a same-named path elsewhere stays visible. + if s.Match("node_modules", true) { + t.Fatal("root-level node_modules should not be ignored by an unloaded nested rule") + } + // Descend into .opencode: push its .gitignore. + s.Push(opencode, ".opencode") + if !s.Match(".opencode/node_modules", true) { + t.Fatal("nested .opencode/.gitignore should ignore .opencode/node_modules") + } + if !s.Match(".opencode/node_modules/zod/src/v3/tests/pipeline.test.ts", false) { + t.Fatal("files under nested-ignored node_modules should be ignored") + } + // A sibling source file inside .opencode is still visible. + if s.Match(".opencode/config.json", false) { + t.Fatal(".opencode/config.json should not be ignored") + } + // Root build/ rule still applies through the stack. + if !s.Match("build", true) { + t.Fatal("root build/ rule should still apply while nested frame is pushed") + } + // Pop the nested frame: its rule no longer applies. + s.Pop() + if s.Match(".opencode/node_modules", true) { + t.Fatal("after popping, the nested rule should no longer be in scope") + } +} + +// TestStackNestedNegationReincludes verifies a nested !pattern can +// re-include a path a parent .gitignore excluded. +func TestStackNestedNegationReincludes(t *testing.T) { + root := t.TempDir() + if err := os.WriteFile(filepath.Join(root, ".gitignore"), []byte("*.log\n"), 0o644); err != nil { + t.Fatal(err) + } + sub := filepath.Join(root, "sub") + if err := os.MkdirAll(sub, 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(sub, ".gitignore"), []byte("!keep.log\n"), 0o644); err != nil { + t.Fatal(err) + } + s := NewStack(root) + s.Push(sub, "sub") + if s.Match("sub/keep.log", false) { + t.Fatal("nested !keep.log should re-include a *.log excluded by root") + } + if !s.Match("sub/other.log", false) { + t.Fatal("sub/other.log should still be excluded by root *.log") + } +} + // lines joins fixture lines with newlines for readable .gitignore // fixtures. func lines(ls ...string) string {