fix(@-picker): pick up files and folders created mid-session
fileSuggester.scan() cached the directory listing keyed only on the absolute path, and the only invalidation paths (Reset / Invalidate / SetCWD) never fired for in-place filesystem changes. Once the picker had scanned the cwd, any later mkdir or touch was invisible until zot restarted. Key the cache on (path, mtime) instead. Stat the browse directory on every scan and re-read when its ModTime has moved; OSes bump dir mtime on every entry add / remove / rename, so the picker now reflects the filesystem within a keystroke. Stat is one cheap syscall and only runs while the popup is open, so the input loop stays responsive on large repos. Added TestFileSuggesterPicksUpNewEntries to pin the behaviour: scan, sleep past the fs mtime tick, mkdir, scan again, expect the new dir.
- commit
- 4f008e887136cbf4cee14e19e81412d4e19990b7
- author
- patriceckhart · · 2026-05-19 16:37 UTC
- parent
- 81c913a
- signature
- unsigned
2 files · +95 -2
M internal/agent/modes/file_suggest.go +22 -2
| @@ -6,6 +6,7 @@ import ( | |||
| 6 | 6 | "regexp" | |
| 7 | 7 | "sort" | |
| 8 | 8 | "strings" | |
| 9 | + | "time" | |
| 9 | 10 | ||
| 10 | 11 | "github.com/patriceckhart/zot/internal/tui" | |
| 11 | 12 | ) | |
| @@ -21,6 +22,13 @@ type fileSuggester struct { | |||
| 21 | 22 | browseRel string // relative path from cwd we're currently browsing ("" = cwd itself) | |
| 22 | 23 | cachedDir string // absolute directory we last scanned | |
| 23 | 24 | cachedAll []fileEntry | |
| 25 | + | // cachedMTime is the mtime of cachedDir at the time of the scan. | |
| 26 | + | // scan() compares the current mtime against this on every call and | |
| 27 | + | // re-reads the directory if it has changed, so files or folders | |
| 28 | + | // created mid-session show up in the picker without restarting zot. | |
| 29 | + | // Stat is cheap (single syscall) so doing it per keystroke while | |
| 30 | + | // the popup is open does not impact responsiveness. | |
| 31 | + | cachedMTime time.Time | |
| 24 | 32 | } | |
| 25 | 33 | ||
| 26 | 34 | type fileEntry struct { | |
| @@ -49,10 +57,21 @@ func (s *fileSuggester) browseDir() string { | |||
| 49 | 57 | return filepath.Join(s.cwd, s.browseRel) | |
| 50 | 58 | } | |
| 51 | 59 | ||
| 52 | - | // scan reads entries from the current browse directory (cached). | |
| 60 | + | // scan reads entries from the current browse directory. | |
| 61 | + | // | |
| 62 | + | // Results are cached by absolute path + mtime: a repeated call against | |
| 63 | + | // the same directory returns the cached slice when nothing has changed | |
| 64 | + | // on disk, and re-reads when an entry was added, removed, or renamed | |
| 65 | + | // (any of which bumps the directory's mtime on every filesystem zot | |
| 66 | + | // supports). A failed stat falls through to a fresh ReadDir rather | |
| 67 | + | // than returning a stale cache so transient errors self-heal. | |
| 53 | 68 | func (s *fileSuggester) scan() []fileEntry { | |
| 54 | 69 | dir := s.browseDir() | |
| 55 | - | if s.cachedDir == dir && s.cachedAll != nil { | |
| 70 | + | var mtime time.Time | |
| 71 | + | if info, err := os.Stat(dir); err == nil { | |
| 72 | + | mtime = info.ModTime() | |
| 73 | + | } | |
| 74 | + | if s.cachedDir == dir && s.cachedAll != nil && !mtime.IsZero() && mtime.Equal(s.cachedMTime) { | |
| 56 | 75 | return s.cachedAll | |
| 57 | 76 | } | |
| 58 | 77 | entries, err := os.ReadDir(dir) | |
| @@ -80,6 +99,7 @@ func (s *fileSuggester) scan() []fileEntry { | |||
| 80 | 99 | }) | |
| 81 | 100 | s.cachedAll = all | |
| 82 | 101 | s.cachedDir = dir | |
| 102 | + | s.cachedMTime = mtime | |
| 83 | 103 | return all | |
| 84 | 104 | } | |
| 85 | 105 | ||
| @@ -6,6 +6,7 @@ import ( | |||||
| 6 | "regexp" | 6 | "regexp" | ||
| 7 | "sort" | 7 | "sort" | ||
| 8 | "strings" | 8 | "strings" | ||
| 9 | + | "time" | |||
| 9 | 10 | ||||
| 10 | "github.com/patriceckhart/zot/internal/tui" | 11 | "github.com/patriceckhart/zot/internal/tui" | ||
| 11 | ) | 12 | ) | ||
| @@ -21,6 +22,13 @@ type fileSuggester struct { | |||||
| 21 | browseRel string // relative path from cwd we're currently browsing ("" = cwd itself) | 22 | browseRel string // relative path from cwd we're currently browsing ("" = cwd itself) | ||
| 22 | cachedDir string // absolute directory we last scanned | 23 | cachedDir string // absolute directory we last scanned | ||
| 23 | cachedAll []fileEntry | 24 | cachedAll []fileEntry | ||
| 25 | + | // cachedMTime is the mtime of cachedDir at the time of the scan. | |||
| 26 | + | // scan() compares the current mtime against this on every call and | |||
| 27 | + | // re-reads the directory if it has changed, so files or folders | |||
| 28 | + | // created mid-session show up in the picker without restarting zot. | |||
| 29 | + | // Stat is cheap (single syscall) so doing it per keystroke while | |||
| 30 | + | // the popup is open does not impact responsiveness. | |||
| 31 | + | cachedMTime time.Time | |||
| 24 | } | 32 | } | ||
| 25 | 33 | ||||
| 26 | type fileEntry struct { | 34 | type fileEntry struct { | ||
| @@ -49,10 +57,21 @@ func (s *fileSuggester) browseDir() string { | |||||
| 49 | return filepath.Join(s.cwd, s.browseRel) | 57 | return filepath.Join(s.cwd, s.browseRel) | ||
| 50 | } | 58 | } | ||
| 51 | 59 | ||||
| 52 | - | // scan reads entries from the current browse directory (cached). | 60 | + | // scan reads entries from the current browse directory. |
| 61 | + | // | |||
| 62 | + | // Results are cached by absolute path + mtime: a repeated call against | |||
| 63 | + | // the same directory returns the cached slice when nothing has changed | |||
| 64 | + | // on disk, and re-reads when an entry was added, removed, or renamed | |||
| 65 | + | // (any of which bumps the directory's mtime on every filesystem zot | |||
| 66 | + | // supports). A failed stat falls through to a fresh ReadDir rather | |||
| 67 | + | // than returning a stale cache so transient errors self-heal. | |||
| 53 | func (s *fileSuggester) scan() []fileEntry { | 68 | func (s *fileSuggester) scan() []fileEntry { | ||
| 54 | dir := s.browseDir() | 69 | dir := s.browseDir() | ||
| 55 | - | if s.cachedDir == dir && s.cachedAll != nil { | 70 | + | var mtime time.Time |
| 71 | + | if info, err := os.Stat(dir); err == nil { | |||
| 72 | + | mtime = info.ModTime() | |||
| 73 | + | } | |||
| 74 | + | if s.cachedDir == dir && s.cachedAll != nil && !mtime.IsZero() && mtime.Equal(s.cachedMTime) { | |||
| 56 | return s.cachedAll | 75 | return s.cachedAll | ||
| 57 | } | 76 | } | ||
| 58 | entries, err := os.ReadDir(dir) | 77 | entries, err := os.ReadDir(dir) | ||
| @@ -80,6 +99,7 @@ func (s *fileSuggester) scan() []fileEntry { | |||||
| 80 | }) | 99 | }) | ||
| 81 | s.cachedAll = all | 100 | s.cachedAll = all | ||
| 82 | s.cachedDir = dir | 101 | s.cachedDir = dir | ||
| 102 | + | s.cachedMTime = mtime | |||
| 83 | return all | 103 | return all | ||
| 84 | } | 104 | } | ||
| 85 | 105 | ||||
M internal/agent/modes/file_suggest_test.go +73 -0
| @@ -1,8 +1,11 @@ | |||
| 1 | 1 | package modes | |
| 2 | 2 | ||
| 3 | 3 | import ( | |
| 4 | + | "os" | |
| 4 | 5 | "path/filepath" | |
| 6 | + | "sort" | |
| 5 | 7 | "testing" | |
| 8 | + | "time" | |
| 6 | 9 | ) | |
| 7 | 10 | ||
| 8 | 11 | func TestExpandFileChipsAtPickerShape(t *testing.T) { | |
| @@ -24,3 +27,73 @@ func TestExpandFileChipsLeavesEditorPlaceholderShapeAlone(t *testing.T) { | |||
| 24 | 27 | t.Fatalf("editor placeholder was changed: %q", got) | |
| 25 | 28 | } | |
| 26 | 29 | } | |
| 30 | + | ||
| 31 | + | // TestFileSuggesterPicksUpNewEntries pins the cache-invalidation bug | |
| 32 | + | // fix: creating a subdirectory after the picker has already scanned | |
| 33 | + | // the cwd must surface that subdirectory on the next scan, without | |
| 34 | + | // any explicit invalidation call. The cache is keyed on the dir's | |
| 35 | + | // mtime, which the OS bumps on every entry add/remove/rename. | |
| 36 | + | func TestFileSuggesterPicksUpNewEntries(t *testing.T) { | |
| 37 | + | tmp := t.TempDir() | |
| 38 | + | if err := os.WriteFile(filepath.Join(tmp, "existing.txt"), []byte("x"), 0o644); err != nil { | |
| 39 | + | t.Fatal(err) | |
| 40 | + | } | |
| 41 | + | ||
| 42 | + | s := newFileSuggester() | |
| 43 | + | s.SetCWD(tmp) | |
| 44 | + | ||
| 45 | + | first := s.scan() | |
| 46 | + | if !containsEntry(first, "existing.txt", false) { | |
| 47 | + | t.Fatalf("first scan missing existing.txt: %#v", first) | |
| 48 | + | } | |
| 49 | + | if containsEntry(first, "test", true) { | |
| 50 | + | t.Fatalf("first scan unexpectedly saw the not-yet-created test/: %#v", first) | |
| 51 | + | } | |
| 52 | + | ||
| 53 | + | // Sleep one filesystem tick: HFS+/APFS/ext4 all bump directory | |
| 54 | + | // mtime with at-least 1s resolution depending on mount options. | |
| 55 | + | // Without this sleep on coarse-resolution filesystems the mtime | |
| 56 | + | // after Mkdir can equal the mtime captured during the first scan | |
| 57 | + | // and the cache would (correctly) be retained. | |
| 58 | + | time.Sleep(1100 * time.Millisecond) | |
| 59 | + | if err := os.Mkdir(filepath.Join(tmp, "test"), 0o755); err != nil { | |
| 60 | + | t.Fatal(err) | |
| 61 | + | } | |
| 62 | + | ||
| 63 | + | second := s.scan() | |
| 64 | + | if !containsEntry(second, "test", true) { | |
| 65 | + | t.Fatalf("second scan did not pick up the newly created test/: %#v", second) | |
| 66 | + | } | |
| 67 | + | if !containsEntry(second, "existing.txt", false) { | |
| 68 | + | t.Fatalf("second scan dropped existing.txt: %#v", second) | |
| 69 | + | } | |
| 70 | + | ||
| 71 | + | // Directories sort before files. | |
| 72 | + | sorted := make([]string, 0, len(second)) | |
| 73 | + | for _, e := range second { | |
| 74 | + | sorted = append(sorted, e.name) | |
| 75 | + | } | |
| 76 | + | if !sort.IsSorted(byDirsFirst(second)) { | |
| 77 | + | t.Fatalf("entries are not dirs-first / case-insensitive sorted: %v", sorted) | |
| 78 | + | } | |
| 79 | + | } | |
| 80 | + | ||
| 81 | + | func containsEntry(entries []fileEntry, name string, isDir bool) bool { | |
| 82 | + | for _, e := range entries { | |
| 83 | + | if e.name == name && e.isDir == isDir { | |
| 84 | + | return true | |
| 85 | + | } | |
| 86 | + | } | |
| 87 | + | return false | |
| 88 | + | } | |
| 89 | + | ||
| 90 | + | type byDirsFirst []fileEntry | |
| 91 | + | ||
| 92 | + | func (b byDirsFirst) Len() int { return len(b) } | |
| 93 | + | func (b byDirsFirst) Swap(i, j int) { b[i], b[j] = b[j], b[i] } | |
| 94 | + | func (b byDirsFirst) Less(i, j int) bool { | |
| 95 | + | if b[i].isDir != b[j].isDir { | |
| 96 | + | return b[i].isDir | |
| 97 | + | } | |
| 98 | + | return b[i].name < b[j].name | |
| 99 | + | } | |
| @@ -1,8 +1,11 @@ | |||||
| 1 | package modes | 1 | package modes | ||
| 2 | 2 | ||||
| 3 | import ( | 3 | import ( | ||
| 4 | + | "os" | |||
| 4 | "path/filepath" | 5 | "path/filepath" | ||
| 6 | + | "sort" | |||
| 5 | "testing" | 7 | "testing" | ||
| 8 | + | "time" | |||
| 6 | ) | 9 | ) | ||
| 7 | 10 | ||||
| 8 | func TestExpandFileChipsAtPickerShape(t *testing.T) { | 11 | func TestExpandFileChipsAtPickerShape(t *testing.T) { | ||
| @@ -24,3 +27,73 @@ func TestExpandFileChipsLeavesEditorPlaceholderShapeAlone(t *testing.T) { | |||||
| 24 | t.Fatalf("editor placeholder was changed: %q", got) | 27 | t.Fatalf("editor placeholder was changed: %q", got) | ||
| 25 | } | 28 | } | ||
| 26 | } | 29 | } | ||
| 30 | + | ||||
| 31 | + | // TestFileSuggesterPicksUpNewEntries pins the cache-invalidation bug | |||
| 32 | + | // fix: creating a subdirectory after the picker has already scanned | |||
| 33 | + | // the cwd must surface that subdirectory on the next scan, without | |||
| 34 | + | // any explicit invalidation call. The cache is keyed on the dir's | |||
| 35 | + | // mtime, which the OS bumps on every entry add/remove/rename. | |||
| 36 | + | func TestFileSuggesterPicksUpNewEntries(t *testing.T) { | |||
| 37 | + | tmp := t.TempDir() | |||
| 38 | + | if err := os.WriteFile(filepath.Join(tmp, "existing.txt"), []byte("x"), 0o644); err != nil { | |||
| 39 | + | t.Fatal(err) | |||
| 40 | + | } | |||
| 41 | + | ||||
| 42 | + | s := newFileSuggester() | |||
| 43 | + | s.SetCWD(tmp) | |||
| 44 | + | ||||
| 45 | + | first := s.scan() | |||
| 46 | + | if !containsEntry(first, "existing.txt", false) { | |||
| 47 | + | t.Fatalf("first scan missing existing.txt: %#v", first) | |||
| 48 | + | } | |||
| 49 | + | if containsEntry(first, "test", true) { | |||
| 50 | + | t.Fatalf("first scan unexpectedly saw the not-yet-created test/: %#v", first) | |||
| 51 | + | } | |||
| 52 | + | ||||
| 53 | + | // Sleep one filesystem tick: HFS+/APFS/ext4 all bump directory | |||
| 54 | + | // mtime with at-least 1s resolution depending on mount options. | |||
| 55 | + | // Without this sleep on coarse-resolution filesystems the mtime | |||
| 56 | + | // after Mkdir can equal the mtime captured during the first scan | |||
| 57 | + | // and the cache would (correctly) be retained. | |||
| 58 | + | time.Sleep(1100 * time.Millisecond) | |||
| 59 | + | if err := os.Mkdir(filepath.Join(tmp, "test"), 0o755); err != nil { | |||
| 60 | + | t.Fatal(err) | |||
| 61 | + | } | |||
| 62 | + | ||||
| 63 | + | second := s.scan() | |||
| 64 | + | if !containsEntry(second, "test", true) { | |||
| 65 | + | t.Fatalf("second scan did not pick up the newly created test/: %#v", second) | |||
| 66 | + | } | |||
| 67 | + | if !containsEntry(second, "existing.txt", false) { | |||
| 68 | + | t.Fatalf("second scan dropped existing.txt: %#v", second) | |||
| 69 | + | } | |||
| 70 | + | ||||
| 71 | + | // Directories sort before files. | |||
| 72 | + | sorted := make([]string, 0, len(second)) | |||
| 73 | + | for _, e := range second { | |||
| 74 | + | sorted = append(sorted, e.name) | |||
| 75 | + | } | |||
| 76 | + | if !sort.IsSorted(byDirsFirst(second)) { | |||
| 77 | + | t.Fatalf("entries are not dirs-first / case-insensitive sorted: %v", sorted) | |||
| 78 | + | } | |||
| 79 | + | } | |||
| 80 | + | ||||
| 81 | + | func containsEntry(entries []fileEntry, name string, isDir bool) bool { | |||
| 82 | + | for _, e := range entries { | |||
| 83 | + | if e.name == name && e.isDir == isDir { | |||
| 84 | + | return true | |||
| 85 | + | } | |||
| 86 | + | } | |||
| 87 | + | return false | |||
| 88 | + | } | |||
| 89 | + | ||||
| 90 | + | type byDirsFirst []fileEntry | |||
| 91 | + | ||||
| 92 | + | func (b byDirsFirst) Len() int { return len(b) } | |||
| 93 | + | func (b byDirsFirst) Swap(i, j int) { b[i], b[j] = b[j], b[i] } | |||
| 94 | + | func (b byDirsFirst) Less(i, j int) bool { | |||
| 95 | + | if b[i].isDir != b[j].isDir { | |||
| 96 | + | return b[i].isDir | |||
| 97 | + | } | |||
| 98 | + | return b[i].name < b[j].name | |||
| 99 | + | } | |||