Merge pull request #28 from rgasper/file-picker-improvements

File picker improvements

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,

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.

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 {

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 {

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 {