fix: resolve symlinks in Files panel and Auto Run folder tree#831
fix: resolve symlinks in Files panel and Auto Run folder tree#831pedramamini merged 5 commits intomainfrom
Conversation
Dirent.isDirectory()/isFile() return false for symlinks since they inspect the link itself, not the target. This caused symlinked subdirectories to be invisible in the Files panel and Auto Run document tree. Local: after readdir, check isSymbolicLink() and follow with fs.stat() to determine the actual target type. Remote: augment the ls -1AF command with a follow-up that identifies symlinks whose targets are directories, so readDirRemote now sets isDirectory=true for symlinked dirs. Also remove overly-strict !entry.isSymlink guards in autorun.ts that excluded symlinked .md files and prevented recursion into symlinked directories for backup cleanup. Closes #829
Update mock Dirent objects in filesystem and autorun tests to include isSymbolicLink() method, matching the new symlink resolution logic. Also fix countRecursive in fs:countItems to resolve symlinks via fs.stat(), consistent with the fs:readDir fix.
📝 WalkthroughWalkthroughResolve and follow symlinks when classifying directory entries for local and remote reads; recurse into symlinked directories, treat broken symlinks as files, and add utilities/tests to resolve Dirent target types. Changes
Sequence DiagramsequenceDiagram
participant Handler as IPC Handler / Renderer Caller
participant Readdir as readdir()/readDirRemote
participant Stat as fs.stat()/remote ls -ld
participant Resolver as resolveDirentType / utils
Handler->>Readdir: request directory entries
Readdir-->>Handler: return Dirent-like entries (may flag symlink)
alt entry.isSymbolicLink
Handler->>Stat: stat(fullPath) to follow symlink
Stat-->>Resolver: provide target info or error
Resolver-->>Handler: resolved {isDirectory,isFile,isBrokenSymlink}
else not symlink
Readdir-->>Handler: use entry.isDirectory / entry.isFile directly
end
Handler->>Handler: include files / recurse into directories based on resolved flags
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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 |
Greptile SummaryThis PR fixes symlink handling in the Files panel and Auto Run folder tree by introducing a Confidence Score: 5/5Safe to merge — the only remaining finding is a P2 style concern about circular-symlink cycle detection. All changes are correct and well-tested. The circular-symlink concern is bounded by the OS ELOOP limit (~40 levels) and is a known limitation of any recursive symlink-following traversal, so it does not block merge. src/main/utils/dirent-utils.ts — consider adding cycle detection via a visited-realpath Set if circular symlinks in user projects are a concern. Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[fs.readdir withFileTypes] --> B{entry.isSymbolicLink?}
B -- No --> C[Use isDirectory / isFile directly]
B -- Yes --> D[fs.stat fullPath]
D -- success --> E[Use stat.isDirectory / stat.isFile]
D -- ENOENT / ELOOP / error --> F[isBrokenSymlink = true]
C --> G[Caller decides: recurse dir / include file / skip]
E --> G
F --> H{Context}
H -- scanDirectory --> I[Filter out null — skip broken link]
H -- checkForMarkdownFiles --> J[isDirectory=false, isFile=false — skip silently]
H -- fs:readDir --> K[isFile = true — show in file browser]
H -- countRecursive --> L[fileCount++ — count as file]
Reviews (2): Last reviewed commit: "test: patch fs/promises mocks exposed by..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/__tests__/main/ipc/handlers/autorun.test.ts (1)
234-245: Consider adding tests for actual symlink entries.The mocks consistently return
isSymbolicLink: () => false, which tests the non-symlink path. Consider adding test cases whereisSymbolicLink: () => trueto verify:
- Symlinked directories are recursed into after
fs.stat()resolves the target- Symlinked
.mdfiles are included in listings- Broken symlinks (where
fs.stat()throws) are handled gracefully🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/__tests__/main/ipc/handlers/autorun.test.ts` around lines 234 - 245, Add tests in src/__tests__/main/ipc/handlers/autorun.test.ts that include mocked Dirent-like entries with isSymbolicLink: () => true and exercise the code paths that call fs.stat; specifically add: (1) a symlinked directory case where fs.stat resolves to a directory and verify recursion into it, (2) a symlinked .md file case where fs.stat resolves to a regular file and verify it appears in listings, and (3) a broken symlink case where fs.stat rejects and verify the handler swallows/logs the error and continues. Use the same mock setup as the existing entries but set isSymbolicLink to true and stub fs.stat (or fs.promises.stat) to resolve or reject accordingly so you validate the symlink handling logic invoked by the autorun/handler under test.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/__tests__/main/ipc/handlers/autorun.test.ts`:
- Around line 234-245: Add tests in
src/__tests__/main/ipc/handlers/autorun.test.ts that include mocked Dirent-like
entries with isSymbolicLink: () => true and exercise the code paths that call
fs.stat; specifically add: (1) a symlinked directory case where fs.stat resolves
to a directory and verify recursion into it, (2) a symlinked .md file case where
fs.stat resolves to a regular file and verify it appears in listings, and (3) a
broken symlink case where fs.stat rejects and verify the handler swallows/logs
the error and continues. Use the same mock setup as the existing entries but set
isSymbolicLink to true and stub fs.stat (or fs.promises.stat) to resolve or
reject accordingly so you validate the symlink handling logic invoked by the
autorun/handler under test.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 0f846ff5-3cf2-4904-b30f-df200eca25f3
📒 Files selected for processing (5)
src/__tests__/main/ipc/handlers/autorun.test.tssrc/__tests__/main/ipc/handlers/filesystem.test.tssrc/main/ipc/handlers/autorun.tssrc/main/ipc/handlers/filesystem.tssrc/main/utils/remote-fs.ts
Pulls the duplicated Dirent symlink-resolution logic out of the Files panel, countItems, and Auto Run scanners into a single helper (src/main/utils/dirent-utils.ts). Each caller now decides how to present broken symlinks (shown as files in fs:readDir, counted as files in countItems, skipped during Auto Run's .md walk) while sharing one implementation. Also tightens the remote-fs symlink-dir discovery glob to explicitly exclude "." and ".." via /.[!.]* and /..?* patterns, and adds test coverage for symlink-to-dir, symlink-to-file, and broken-symlink cases in the new helper plus the Files panel, countItems, and Auto Run handlers.
Two pre-existing mocks surfaced real failures once resolveDirentType is on the hot path: - fs:countItems "count items in local directory" was missing isFile() on its Dirent mocks. The old countRecursive only looked at isDirectory/isSymbolicLink, but the new helper also reads isFile() to classify non-symlink entries. - The new dirent-utils test needs readdir mocked alongside stat for readDirWithResolvedTypes; swap the stray vi.spyOn for vi.mocked on the module-level mock so the fn is discoverable.
|
@greptile re-analyze please |
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/main/ipc/handlers/filesystem.ts (1)
407-422:⚠️ Potential issue | 🔴 CriticalSymlink cycles will cause unbounded recursion in
fs:countItems.With
resolveDirentTypenow returningisDirectory: truefor symlinks that point at directories,countRecursivewill happily follow a cycle (e.g./a/link -> /a) forever — there is no depth cap and no visited-path tracking. Previouslyentry.isDirectory()wasfalsefor symlinks so this was impossible. This can stack-overflow the main process or wedge the IPC call indefinitely on benign user filesystems (node_modules hoisting, system symlinks, etc.).At minimum, add a depth cap (as
fs:directorySizealready does). Ideally, track visitedfs.realpathtargets to detect cycles precisely.🛡️ Suggested guard
- // Local: standard fs count - let fileCount = 0; - let folderCount = 0; - - const countRecursive = async (dir: string) => { - const entries = await fs.readdir(dir, { withFileTypes: true }); - for (const entry of entries) { - const fullPath = path.join(dir, entry.name); - const resolved = await resolveDirentType(entry, fullPath); - if (resolved.isDirectory) { - folderCount++; - await countRecursive(fullPath); - } else { - // Files, symlinks-to-files, and broken symlinks all count as files - fileCount++; - } - } - }; - - await countRecursive(dirPath); + // Local: standard fs count + let fileCount = 0; + let folderCount = 0; + const visited = new Set<string>(); + const MAX_DEPTH = 20; + + const countRecursive = async (dir: string, depth: number) => { + if (depth >= MAX_DEPTH) return; + let realDir: string; + try { + realDir = await fs.realpath(dir); + } catch { + return; + } + if (visited.has(realDir)) return; + visited.add(realDir); + + const entries = await fs.readdir(dir, { withFileTypes: true }); + for (const entry of entries) { + const fullPath = path.join(dir, entry.name); + const resolved = await resolveDirentType(entry, fullPath); + if (resolved.isDirectory) { + folderCount++; + await countRecursive(fullPath, depth + 1); + } else { + fileCount++; + } + } + }; + + await countRecursive(dirPath, 0);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/ipc/handlers/filesystem.ts` around lines 407 - 422, The recursive walker countRecursive in fs:countItems can follow symlinked directories indefinitely now that resolveDirentType treats symlinks-to-dirs as directories; add a recursion safety mechanism by (1) adding a maxDepth parameter (reusing the same cap pattern as fs:directorySize) passed into countRecursive and bailing out once reached, and (2) track visited real targets using fs.realpath (store resolved paths in a Set) and skip traversing if realpath is already seen; update the logic that increments folderCount/fileCount in countRecursive to still count the entry but not recurse into visited or over-depth directories so cycles and deep trees are guarded against.src/main/ipc/handlers/autorun.ts (1)
79-130:⚠️ Potential issue | 🔴 CriticalSymlink cycles will cause infinite recursion in
scanDirectory.Before this PR,
entry.isDirectory()returnedfalsefor symlinks, so cycles were impossible. NowresolveDirentTypeclassifiessymlink → directoryas a directory and the recursion at line 109 will follow it, including links that point at an ancestor (/folder/a/link -> /folder). There is no depth cap or visited-path set, so the main process can stack-overflow on perfectly reasonable user folders (e.g. anode_modulessymlink farm, a link to the project root,.configpointing at$HOME).Same issue applies to
checkForMarkdownFilesbelow (line 226) and the SSH counterpartsscanDirectoryRemote/deleteBackupsRemoteRecursive, which now also recurse through symlinked dirs per the updatedreadDirRemoteclassification.Recommend a shared guard that (a) caps recursion depth and (b) deduplicates by
fs.realpathfor the local paths; for remote, rely on a depth cap since resolving real paths over SSH is expensive.🛡️ Sketch of a guarded scanDirectory
-async function scanDirectory(dirPath: string, relativePath: string = ''): Promise<TreeNode[]> { - const entries = await fs.readdir(dirPath, { withFileTypes: true }); +const MAX_SCAN_DEPTH = 20; +async function scanDirectory( + dirPath: string, + relativePath: string = '', + depth: number = 0, + visited: Set<string> = new Set() +): Promise<TreeNode[]> { + if (depth >= MAX_SCAN_DEPTH) return []; + let realDir: string; + try { + realDir = await fs.realpath(dirPath); + } catch { + return []; + } + if (visited.has(realDir)) return []; + visited.add(realDir); + const entries = await fs.readdir(dirPath, { withFileTypes: true }); const nodes: TreeNode[] = []; // ... for (const entry of sortedEntries) { // ... if (entry.isDir) { - const children = await scanDirectory(path.join(dirPath, entry.name), entryRelativePath); + const children = await scanDirectory( + path.join(dirPath, entry.name), + entryRelativePath, + depth + 1, + visited + );Analogous change for
checkForMarkdownFiles,scanDirectoryRemote, anddeleteBackupsRemoteRecursive(the remote ones only need the depth cap).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/ipc/handlers/autorun.ts` around lines 79 - 130, scanDirectory currently recurses into symlinked directories (via resolveDirentType) and can infinitely loop; fix by adding a recursion guard: add an optional maxDepth parameter (e.g. default 50) and a visited RealPath Set to scanDirectory signature, call fs.realpath on each directory before recursing and skip recursion if depth exceeded or realpath already in visited; pass an incremented depth and a cloned/updated visited set into recursive calls; apply the same pattern to checkForMarkdownFiles, while for scanDirectoryRemote and deleteBackupsRemoteRecursive implement the maxDepth guard (remote versions should not rely on realpath resolution due to SSH cost) and skip recursion when depth exceeds the cap.
♻️ Duplicate comments (1)
src/main/ipc/handlers/autorun.ts (1)
226-253:⚠️ Potential issue | 🔴 CriticalSame cycle concern as
scanDirectory— infinite recursion on symlink loops.
checkForMarkdownFilesis called byautorun:hasDocuments, which runs any time the wizard / autorun UI checks a folder. A symlink cycle will now wedge this handler; please apply the same depth + visited-realpath guard suggested forscanDirectory.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/ipc/handlers/autorun.ts` around lines 226 - 253, checkForMarkdownFiles can infinite-recurse on symlink cycles; update it to accept and propagate a recursion guard (e.g., a maxDepth numeric parameter and a visited Set of realpaths) similar to scanDirectory: at start resolve the directory's realpath, if already in visited return false, add it to visited, bail out if currentDepth exceeds a sensible MAX_DEPTH, and pass incremented depth and the same visited Set into recursive calls; reference function name checkForMarkdownFiles and the autorun:hasDocuments caller so you ensure the new optional parameters are threaded through where it’s invoked.
🧹 Nitpick comments (2)
src/main/ipc/handlers/filesystem.ts (1)
265-287: Drive-by:fs:directorySizestill usesentry.isDirectory()and will misclassify symlinked dirs.Not a regression introduced by this PR, and the depth cap at line 262 prevents runaway recursion, but the result of this handler is now inconsistent with
fs:readDir/fs:countItems: a symlinked directory is counted as a file here but as a folder elsewhere, and its contents aren't summed intototalSize. If you're normalizing symlink handling across the file, consider migrating this block toresolveDirentTypeas well (while keeping the depth limit).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/ipc/handlers/filesystem.ts` around lines 265 - 287, The directorySize traversal misclassifies symlinked directories because it uses entry.isDirectory(); update the loop in calculateSize to call the existing resolveDirentType(entry, fullPath) (same helper used by fs:readDir / fs:countItems) and switch on that returned type so symlinked directories are treated like directories (recursing and adding their contents to totalSize) while preserving the depth cap and the ignore patterns (node_modules, __pycache__) and keeping the existing try/catch around fs.stat for files.src/main/utils/dirent-utils.ts (1)
50-63: Consider distinguishing broken-symlink errors from unexpected ones.The bare
catch {}collapses every possiblefs.statfailure — including EACCES, EIO, or programmer errors — intoisBrokenSymlink: truewith no visibility. Per coding guidelines, unexpected errors should be surfaced for Sentry.Minimum fix: log unknown error codes so broken-link suppression doesn't mask real failures.
🔎 Proposed refinement
- try { - const targetStat = await fs.stat(fullPath); - return { - isDirectory: targetStat.isDirectory(), - isFile: targetStat.isFile(), - isBrokenSymlink: false, - }; - } catch { - return { - isDirectory: false, - isFile: false, - isBrokenSymlink: true, - }; - } + try { + const targetStat = await fs.stat(fullPath); + return { + isDirectory: targetStat.isDirectory(), + isFile: targetStat.isFile(), + isBrokenSymlink: false, + }; + } catch (err: unknown) { + const code = (err as NodeJS.ErrnoException | undefined)?.code; + // ENOENT / ELOOP / EACCES are expected for broken or inaccessible symlinks. + // Anything else is unexpected and worth reporting for diagnostics. + if (code && !['ENOENT', 'ELOOP', 'EACCES', 'ENOTDIR'].includes(code)) { + // e.g. logger.warn / captureException from src/utils/sentry.ts + } + return { isDirectory: false, isFile: false, isBrokenSymlink: true }; + }As per coding guidelines: "Do not silently swallow errors. Let unhandled exceptions bubble up to Sentry for error tracking in production."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/utils/dirent-utils.ts` around lines 50 - 63, The catch block around fs.stat(fullPath) is treating every error as a broken symlink; change it to catch (err) and inspect err.code — return isBrokenSymlink: true only for known "broken link" codes (e.g. 'ENOENT', 'ENOTDIR', 'ELOOP'), and for any other/unknown error either rethrow the error or log it and rethrow so it surfaces to Sentry; keep the returned shape (isDirectory/isFile/isBrokenSymlink) when handling known broken-link codes and use fullPath and fs.stat as the reference points to locate the change.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/main/ipc/handlers/filesystem.ts`:
- Around line 124-136: scanMarkdownFiles' recursive helper scanDir can recurse
indefinitely through symlink cycles; add a numeric depth parameter (e.g., depth
or remainingDepth) to scanDir and enforce a maxDepth (use the same maxDepth=10
convention as fileExplorer.ts) so that when depth <= 0 the function returns
without recursing further. Thread this parameter through all recursive calls
from scanDir (decrementing on each recursion) and ensure the top-level
scanMarkdownFiles invocation starts with the maxDepth value; keep the existing
behavior of listing files otherwise.
---
Outside diff comments:
In `@src/main/ipc/handlers/autorun.ts`:
- Around line 79-130: scanDirectory currently recurses into symlinked
directories (via resolveDirentType) and can infinitely loop; fix by adding a
recursion guard: add an optional maxDepth parameter (e.g. default 50) and a
visited RealPath Set to scanDirectory signature, call fs.realpath on each
directory before recursing and skip recursion if depth exceeded or realpath
already in visited; pass an incremented depth and a cloned/updated visited set
into recursive calls; apply the same pattern to checkForMarkdownFiles, while for
scanDirectoryRemote and deleteBackupsRemoteRecursive implement the maxDepth
guard (remote versions should not rely on realpath resolution due to SSH cost)
and skip recursion when depth exceeds the cap.
In `@src/main/ipc/handlers/filesystem.ts`:
- Around line 407-422: The recursive walker countRecursive in fs:countItems can
follow symlinked directories indefinitely now that resolveDirentType treats
symlinks-to-dirs as directories; add a recursion safety mechanism by (1) adding
a maxDepth parameter (reusing the same cap pattern as fs:directorySize) passed
into countRecursive and bailing out once reached, and (2) track visited real
targets using fs.realpath (store resolved paths in a Set) and skip traversing if
realpath is already seen; update the logic that increments folderCount/fileCount
in countRecursive to still count the entry but not recurse into visited or
over-depth directories so cycles and deep trees are guarded against.
---
Duplicate comments:
In `@src/main/ipc/handlers/autorun.ts`:
- Around line 226-253: checkForMarkdownFiles can infinite-recurse on symlink
cycles; update it to accept and propagate a recursion guard (e.g., a maxDepth
numeric parameter and a visited Set of realpaths) similar to scanDirectory: at
start resolve the directory's realpath, if already in visited return false, add
it to visited, bail out if currentDepth exceeds a sensible MAX_DEPTH, and pass
incremented depth and the same visited Set into recursive calls; reference
function name checkForMarkdownFiles and the autorun:hasDocuments caller so you
ensure the new optional parameters are threaded through where it’s invoked.
---
Nitpick comments:
In `@src/main/ipc/handlers/filesystem.ts`:
- Around line 265-287: The directorySize traversal misclassifies symlinked
directories because it uses entry.isDirectory(); update the loop in
calculateSize to call the existing resolveDirentType(entry, fullPath) (same
helper used by fs:readDir / fs:countItems) and switch on that returned type so
symlinked directories are treated like directories (recursing and adding their
contents to totalSize) while preserving the depth cap and the ignore patterns
(node_modules, __pycache__) and keeping the existing try/catch around fs.stat
for files.
In `@src/main/utils/dirent-utils.ts`:
- Around line 50-63: The catch block around fs.stat(fullPath) is treating every
error as a broken symlink; change it to catch (err) and inspect err.code —
return isBrokenSymlink: true only for known "broken link" codes (e.g. 'ENOENT',
'ENOTDIR', 'ELOOP'), and for any other/unknown error either rethrow the error or
log it and rethrow so it surfaces to Sentry; keep the returned shape
(isDirectory/isFile/isBrokenSymlink) when handling known broken-link codes and
use fullPath and fs.stat as the reference points to locate the change.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 6db624ec-2fff-41a3-ba78-a926ac7d31da
📒 Files selected for processing (7)
src/__tests__/main/ipc/handlers/autorun.test.tssrc/__tests__/main/ipc/handlers/filesystem.test.tssrc/__tests__/main/utils/dirent-utils.test.tssrc/main/ipc/handlers/autorun.tssrc/main/ipc/handlers/filesystem.tssrc/main/utils/dirent-utils.tssrc/main/utils/remote-fs.ts
CodeRabbit flagged that the graph builder's scanMarkdownFiles walks directories without a depth bound. Now that fs:readDir resolves symlinked directories to isDirectory=true, a cycle like a/link -> a would recurse indefinitely. loadFileTree in fileExplorer.ts already caps at depth 10 — match that convention here and bail out with a warning when the cap is hit. Adds a regression test that stubs readDir to return a self-referencing dir and asserts the scan terminates with a bounded number of calls.
There was a problem hiding this comment.
🧹 Nitpick comments (3)
src/__tests__/renderer/components/DocumentGraph/graphDataBuilder.test.ts (1)
242-278: Good regression coverage for the depth cap.Cleanly simulates a self-referencing directory cycle and verifies bounded termination plus a non-empty result. The
≤ 12slack over the actual cap of 10 is reasonable headroom. Optional: also assertconsole.warnwas emitted so the guard's logging contract is pinned down — otherwise a future refactor could silently drop the cap without failing this test.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/__tests__/renderer/components/DocumentGraph/graphDataBuilder.test.ts` around lines 242 - 278, Add an assertion that the depth-cap warning was emitted: spy on console.warn (or the logger used) before calling buildGraphData in this test ("should terminate directory scan when symlink cycle would recurse forever"), run the scan, and assert console.warn was called (optionally with a message matching the depth-cap or recursion guard). Keep the existing cyclicReadDir spy and existing assertions (result.nodes and cyclicReadDir.mock.calls length) and restore/clear the console.warn spy after the test.src/renderer/components/DocumentGraph/graphDataBuilder.ts (2)
320-353: Depth cap looks good; consider sharing the constant withfileExplorer.The depth guard correctly bounds recursion and matches
fileExplorer's cap of 10, which is exactly what prevents the symlink-cycle regression. One small suggestion: since the JSDoc explicitly ties this toloadFileTree's cap, consider hoistingSCAN_MAX_DEPTHinto a shared constants module (or re-exporting fromfileExplorer) so the two limits can't silently drift apart if one side is tuned later. Non-blocking.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/renderer/components/DocumentGraph/graphDataBuilder.ts` around lines 320 - 353, The SCAN_MAX_DEPTH constant used by scanMarkdownFiles should be shared with fileExplorer.loadFileTree to prevent drift; move SCAN_MAX_DEPTH into a shared constants module (e.g., export a MAX_SCAN_DEPTH) or re-export the existing constant from fileExplorer, then import and use that shared symbol in scanMarkdownFiles instead of the local SCAN_MAX_DEPTH so both scanMarkdownFiles and loadFileTree reference the same single source of truth.
347-353: Potentially noisy warning on wide cyclic trees.In a real symlink cycle this
console.warnwill fire once per branch that reaches depth 10, which can be many times per scan (andscanMarkdownFilesis also called a second time bystartBacklinkScan). Consider logging only once perscanMarkdownFilesinvocation (e.g., a localwarnedflag) to avoid log spam in the renderer console. Optional.♻️ Suggested refactor
async function scanMarkdownFiles( rootPath: string, onProgress?: ProgressCallback, sshRemoteId?: string ): Promise<string[]> { const markdownFiles: string[] = []; let directoriesScanned = 0; let isRootDirectory = true; + let depthWarned = false; async function scanDir(currentPath: string, relativePath: string, depth: number): Promise<void> { const isRoot = isRootDirectory; isRootDirectory = false; if (depth >= SCAN_MAX_DEPTH) { - // Bail out rather than risk an infinite loop through a symlink cycle. - console.warn( - `scanMarkdownFiles: reached max depth ${SCAN_MAX_DEPTH} at ${currentPath}; stopping recursion` - ); + if (!depthWarned) { + console.warn( + `scanMarkdownFiles: reached max depth ${SCAN_MAX_DEPTH} (first hit at ${currentPath}); stopping further recursion` + ); + depthWarned = true; + } return; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/renderer/components/DocumentGraph/graphDataBuilder.ts` around lines 347 - 353, The console.warn inside scanMarkdownFiles fires repeatedly when many branches hit SCAN_MAX_DEPTH causing log spam; add a per-invocation boolean (e.g., local warned) in scanMarkdownFiles so the warning is emitted only once per call (set warned when you log and check it before logging), or alternatively pass a warnOnce flag from startBacklinkScan into scanMarkdownFiles to ensure the message about SCAN_MAX_DEPTH is logged a single time per scan invocation.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/__tests__/renderer/components/DocumentGraph/graphDataBuilder.test.ts`:
- Around line 242-278: Add an assertion that the depth-cap warning was emitted:
spy on console.warn (or the logger used) before calling buildGraphData in this
test ("should terminate directory scan when symlink cycle would recurse
forever"), run the scan, and assert console.warn was called (optionally with a
message matching the depth-cap or recursion guard). Keep the existing
cyclicReadDir spy and existing assertions (result.nodes and
cyclicReadDir.mock.calls length) and restore/clear the console.warn spy after
the test.
In `@src/renderer/components/DocumentGraph/graphDataBuilder.ts`:
- Around line 320-353: The SCAN_MAX_DEPTH constant used by scanMarkdownFiles
should be shared with fileExplorer.loadFileTree to prevent drift; move
SCAN_MAX_DEPTH into a shared constants module (e.g., export a MAX_SCAN_DEPTH) or
re-export the existing constant from fileExplorer, then import and use that
shared symbol in scanMarkdownFiles instead of the local SCAN_MAX_DEPTH so both
scanMarkdownFiles and loadFileTree reference the same single source of truth.
- Around line 347-353: The console.warn inside scanMarkdownFiles fires
repeatedly when many branches hit SCAN_MAX_DEPTH causing log spam; add a
per-invocation boolean (e.g., local warned) in scanMarkdownFiles so the warning
is emitted only once per call (set warned when you log and check it before
logging), or alternatively pass a warnOnce flag from startBacklinkScan into
scanMarkdownFiles to ensure the message about SCAN_MAX_DEPTH is logged a single
time per scan invocation.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: fdbb9140-541e-4589-8465-108d08bf68fc
📒 Files selected for processing (2)
src/__tests__/renderer/components/DocumentGraph/graphDataBuilder.test.tssrc/renderer/components/DocumentGraph/graphDataBuilder.ts
1 participant
Summary
Dirent.isDirectory()/isFile()returnfalsefor symlinks — we now checkisSymbolicLink()and follow withfs.stat()to determine the actual target typels -1AFcommand inreadDirRemoteto also identify symlinks whose targets are directories, so they can be recursed into!entry.isSymlinkguards in Auto Run that excluded symlinked.mdfiles and prevented recursion into symlinked directoriesfs:countItemsto also follow symlinks when counting items recursivelyCloses #829
Test plan
ln -s /some/path ./project/linked-dir)linked-dirappears and is expandable.mdfiles inside the symlinked directorySummary by CodeRabbit
Bug Fixes
Tests