Show nested sub-project files in file-explorer name search#6481
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThis PR adds shared Quick Open helpers for parsing 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing Touches📝 Generate docstrings
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
src/shared/quick-open-readdir-walk.ts (1)
221-226: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winPrune excluded prefixes in the nested readdir walk
The recursive walk still traverses excluded subtrees and only filters them at
addFinalPath, so a large excluded nested repo can still burn the shared budget and fail withFile listing exceededortimed out. Pass translated nested-relativeexcludePathPrefixesinto the recursive call so those subtrees are skipped during traversal.src/main/ipc/filesystem-list-files-git-fallback.ts (1)
52-197: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚖️ Poor tradeoffNear-complete duplication of
listFilesWithGitorchestration with the relay implementation.
runGitLsFiles,killSurvivors, and thePromise.all → expandQuickOpenGitFilesWithNestedReposchain here mirrorsrc/relay/fs-handler-git-fallback.tsalmost line-for-line; the only real difference isgitSpawn(+wslDistro) vsspawn(+buildRelayCommandEnv) and the close-handling. Since the PR intent is to keep the two consistent, consider extracting the shared orchestration into a helper that takes a spawn factory, so the two copies cannot drift (see the related divergence noted in the relay file).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 625f654b-4afa-4ffc-8e0d-c46370b70fc6
📒 Files selected for processing (12)
src/main/ipc/filesystem-list-files-git-fallback-real.test.tssrc/main/ipc/filesystem-list-files-git-fallback.tssrc/main/ipc/filesystem-list-files.test.tssrc/main/ipc/filesystem-list-files.tssrc/relay/fs-handler-git-fallback.tssrc/relay/fs-handler-list-files-ignored.test.tssrc/relay/fs-handler-readdir-fallback.tssrc/relay/fs-handler.tssrc/shared/quick-open-filter.test.tssrc/shared/quick-open-filter.tssrc/shared/quick-open-readdir-walk.test.tssrc/shared/quick-open-readdir-walk.ts
|
Thanks for the review. Addressed:
Not changing:
|
nwparker
force-pushed
the
brennanb2025/issue-5939-monorepo-file-search
branch
from
d2124a5 to
c391e1f
Compare
When a workspace root is a monorepo parent containing nested git sub-projects, the file-explorer name search returned no matches for files inside those sub-projects. When ripgrep is unavailable, the git ls-files fallback does not recurse into nested git repositories, so their files were never listed. After the flat git ls-files pass, detect nested-repo entries (gitlink mode 160000, or untracked embedded repos with a trailing slash) and fill their subtrees with a bounded readdir walk, re-prefixed root-relative. Non-git roots now use a whole-root walk, and the fallback rejects on failure/timeout instead of silently returning an empty list. Applied to both the local main process and the SSH relay via a shared module. Detection reads git's -s mode output so ordinary files incur no extra lstat. The readdir walk shares one file/deadline budget across all nested subtrees. Co-authored-by: Orca <help@stably.ai>
Rebase workspace-root-relative exclude prefixes onto each nested repo before walking it, so excluded subtrees are skipped during traversal instead of being walked and filtered afterward. A large excluded nested subtree no longer burns the shared file/deadline budget. Co-authored-by: Orca <help@stably.ai>
The relay git fallback resolved on a clean close even when git exited non-zero, so it could expand a partial result set instead of surfacing the failure. Reject on non-zero exit to match the main-process fallback. Co-authored-by: Orca <help@stably.ai>
nwparker
force-pushed
the
brennanb2025/issue-5939-monorepo-file-search
branch
from
c391e1f to
65b7743
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 4cf9252e-fe5a-41bb-9d3a-1c48343e8107
📒 Files selected for processing (12)
src/main/ipc/filesystem-list-files-git-fallback-real.test.tssrc/main/ipc/filesystem-list-files-git-fallback.tssrc/main/ipc/filesystem-list-files.test.tssrc/main/ipc/filesystem-list-files.tssrc/relay/fs-handler-git-fallback.tssrc/relay/fs-handler-list-files-ignored.test.tssrc/relay/fs-handler-readdir-fallback.tssrc/relay/fs-handler.tssrc/shared/quick-open-filter.test.tssrc/shared/quick-open-filter.tssrc/shared/quick-open-readdir-walk.test.tssrc/shared/quick-open-readdir-walk.ts
🚧 Files skipped from review as they are similar to previous changes (9)
- src/relay/fs-handler.ts
- src/shared/quick-open-filter.test.ts
- src/relay/fs-handler-readdir-fallback.ts
- src/shared/quick-open-filter.ts
- src/shared/quick-open-readdir-walk.test.ts
- src/main/ipc/filesystem-list-files.ts
- src/relay/fs-handler-git-fallback.ts
- src/main/ipc/filesystem-list-files.test.ts
- src/relay/fs-handler-list-files-ignored.test.ts
| const nestedFiles = await listQuickOpenFilesWithReaddir(joinRootRel(opts.rootPath, relPath), { | ||
| // Why: exclude prefixes are workspace-root-relative; rebase them onto the | ||
| // nested repo so the walk prunes excluded subtrees during traversal | ||
| // instead of burning the shared budget and filtering them at the end. | ||
| excludePathPrefixes: rebaseExcludePrefixesForNestedRepo(excludePathPrefixes, relPath), | ||
| budget | ||
| }) |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Skip nested repos when the exclude covers their root.
rebaseExcludePrefixesForNestedRepo only prunes descendants like packages/app/src; excludes at or above the nested repo root, such as packages/app or packages, are dropped and the nested repo is still walked until final filtering. That can exhaust the shared budget on an excluded worktree.
Proposed fix
if (kind === 'drop-placeholder') {
continue
}
+ // Why: rebasing only handles excludes inside the nested repo; an exclude
+ // can also cover the nested repo root itself.
+ if (shouldExcludeQuickOpenRelPath(relPath, excludePathPrefixes)) {
+ continue
+ }
+
const nestedFiles = await listQuickOpenFilesWithReaddir(joinRootRel(opts.rootPath, relPath), {📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const nestedFiles = await listQuickOpenFilesWithReaddir(joinRootRel(opts.rootPath, relPath), { | |
| // Why: exclude prefixes are workspace-root-relative; rebase them onto the | |
| // nested repo so the walk prunes excluded subtrees during traversal | |
| // instead of burning the shared budget and filtering them at the end. | |
| excludePathPrefixes: rebaseExcludePrefixesForNestedRepo(excludePathPrefixes, relPath), | |
| budget | |
| }) | |
| if (kind === 'drop-placeholder') { | |
| continue | |
| } | |
| // Why: rebasing only handles excludes inside the nested repo; an exclude | |
| // can also cover the nested repo root itself. | |
| if (shouldExcludeQuickOpenRelPath(relPath, excludePathPrefixes)) { | |
| continue | |
| } | |
| const nestedFiles = await listQuickOpenFilesWithReaddir(joinRootRel(opts.rootPath, relPath), { | |
| // Why: exclude prefixes are workspace-root-relative; rebase them onto the | |
| // nested repo so the walk prunes excluded subtrees during traversal | |
| // instead of burning the shared budget and filtering them at the end. | |
| excludePathPrefixes: rebaseExcludePrefixesForNestedRepo(excludePathPrefixes, relPath), | |
| budget | |
| }) |
2 participants
Fixes #5939
What changes
File-explorer name search now lists and matches files that live inside nested git sub-projects when the workspace root is a monorepo parent. Previously, typing any query at a monorepo-parent root returned no results for files inside the sub-projects.
What does not change: the normal (no-query) file tree, the ripgrep code path, and text/content search are untouched. This only fixes the file listing that feeds name search when the ripgrep-less git fallback is in use.
Root cause
Name search lists files via
git ls-fileswhen ripgrep is unavailable (Orca bundles no ripgrep, so this is the common path on a fresh macOS install).git ls-filesdoes not recurse into nested git repositories: a committed gitlink surfaces as a single bare entry, an untracked embedded repo surfaces as a single directory entry, and a non-git root yields nothing. So files inside nested sub-projects were never enumerated and name search could not match them. The normal tree was unaffected because it lists directories withreaddir. Opening the workspace directly on a sub-project worked because that sub-project is itself a git repo.Approach
After the flat
git ls-filespass, detect nested-repo entries and fill their subtrees with a boundedreaddirwalk, re-prefixed root-relative:-smode output, so a gitlink is identified by mode160000and an untracked embedded repo by its trailing slash. Ordinary files are classified from the same output and incur no extralstat.readdirwalk shares one file-count + deadline budget across all nested subtrees, prunesnode_modules/blocklisted dirs, does not follow symlinks, and rejects on cap/deadline so a partial scan never reads as an empty result.Relationship to the open folder-workspace PR
This is a different layer than the open PR
#6170(renderer-only: makes the file-list request fire for folder workspaces and recomputes SSHconnectionId). That PR makes the listing request happen; this PR fixes what the backend listing returns. For a monorepo parent added as a git repo, the request already fires today, yet nested files are dropped — confirmed live before this fix. The two are complementary, not overlapping.Design review
Design was reviewed through Brennan's PR process. The review redirected the original per-repo
git ls-filesrecursion to reuse the relay's existing boundedreaddirwalk, fixed an aggregate-budget bug (per-call caps would not bound N subtrees), and required reject-on-failure semantics and main/relay parity.Implementation and deviations
Implemented as designed. One refinement during the performance pass: the first cut
lstat-ed every git entry to detect nested repos (an O(N) stat storm on large repos); switched detection to git's-smode output, which a committed gitlink (160000) vs. an ordinary file can be told apart textually with zero stats for regular files. A committed gitlink has no trailing slash, so a slash-only gate would have missed it — the mode check is required.Completeness verification
Read-only verification confirmed all design requirements and edge cases are implemented: detection variants (committed gitlink, untracked embedded, extensionless tracked file, un-checked-out gitlink), shared-budget bounding, final-path exclude/blocklist filtering, reject-vs-empty semantics, non-git-root walk on both paths, and SSH relay parity.
Headline behavior status
Implemented in production code (not scaffolding). Verified in the running app: at a monorepo-parent workspace, searching
mainshowspackages/app/src/main.ts(a committed-gitlink sub-project) and searchinglibshowspackages/lib/src/lib.ts+package.json(an untracked embedded sub-project). The backend listing went from 5 entries (nested files dropped) to 7 (nested files present).Performance
Audited before code review. Touched hot path: the ripgrep-less name-search file listing (fires once per search session, not per keystroke; never at startup). The only real concern — an
lstatper git entry — was eliminated by reading-smode output (zero stats for ordinary files; a deterministic test asserts nolstatfor a regular-file list). The shared budget bounds aggregate work across all subtrees; no leaks, no busy loops, one cheap addedgit rev-parseper listing to gate the non-git-root walk.Code review
Iterative two-reviewer loop ran to clean (2 rounds). Round 1 fixed a
max-linesoverflow (split the git fallback into a focused module rather than disabling the rule), two lint findings, and brought the relay's budget-error message into parity with the existing install-ripgrep guidance. Round 2: both reviewers clean.Merge-confidence validation
Verdict: land. Function-level proof against a real git monorepo parent confirmed local and relay
listFilesWithGitreturn identical results including nested files for both gitlink and untracked-embedded sub-projects; non-git root returns a whole-root walk. SSH reasoning: the relay change adds more entries to the samestring[]with no transport/serialization change. Cross-platform:git ls-filesemits forward slashes; the-smode regex handles SHA-1 and SHA-256; Windows separators are normalized.Accepted gaps: no live SSH-transport pass (transport layer unchanged; the changed logic is covered by the real-git function test and the full relay suite). Follow-up note: the local path could also wrap
readdirbudget-cap errors in the install-ripgrep guidance the relay now uses — narrow edge, falls back gracefully today.Checks run
pnpm exec vitest runon the shared, main-IPC, and relay file-listing suites — green (shared/main scoped 59; relay 564; main IPC filesystem 207).pnpm run typecheck:node— clean.pnpm exec oxlinton all touched files — clean.Made with Orca 🐋