Skip to content

fix: handle read_file on directories gracefully (EISDIR fallback)#392

Open
wonderwhy-er wants to merge 1 commit intomainfrom
fix/read-file-directory-fallback
Open

fix: handle read_file on directories gracefully (EISDIR fallback)#392
wonderwhy-er wants to merge 1 commit intomainfrom
fix/read-file-directory-fallback

Conversation

@wonderwhy-er
Copy link
Copy Markdown
Owner

@wonderwhy-er wonderwhy-er commented Mar 24, 2026

User description

Problem

When an AI agent calls read_file on a directory path, it gets a cryptic EISDIR: illegal operation on a directory, read error. This happens fairly often as agents sometimes pass directory paths to read_file instead of using list_directory.

Fix

In readFileFromDisk() (src/tools/filesystem.ts), added an isDirectory() check right after the existing fs.stat() call. When a directory is detected:

  1. Falls back to listDirectory() internally — so the agent still gets useful directory listing results immediately (no wasted round-trip)
  2. Returns a hint message telling the agent to use list_directory next time

What changed

  • src/tools/filesystem.ts: Added directory detection in readFileFromDisk() before the file read operation

Testing

  • All 33 existing tests pass
  • Build compiles cleanly
  • The fix also benefits readMultipleFiles since it calls readFileFromDisk under the hood

Summary by CodeRabbit

  • New Features

    • Directory requests now return a plain-text tree listing with a short instruction and are marked as directories so previews render as text.
  • Bug Fixes

    • Directory listings respect a timeout and will surface a clear timeout error if listing fails.
    • Errors tied to directory-listing no longer silently fall back to normal file reads; directory results force text previews.

CodeAnt-AI Description

Show directory results as a browsable tree instead of a file error

What Changed

  • read_file now returns directory contents when a folder path is used, instead of showing an illegal file-read error
  • Directory results open in a tree view with expand/collapse, file opening, and an option to open the folder in the system file browser
  • Users can drill into subfolders from the preview and go back to the parent directory
  • Directory previews and directory listings are marked clearly as folders so they render in the right view

Impact

✅ Fewer read-file errors on folders
✅ Faster directory browsing
✅ Clearer folder previews

💡 Usage Guide

Checking Your Pull Request

Every time you make a pull request, our system automatically looks through it. We check for security issues, mistakes in how you're setting up your infrastructure, and common code problems. We do this to make sure your changes are solid and won't cause any trouble later.

Talking to CodeAnt AI

Got a question or need a hand with something in your pull request? You can easily get in touch with CodeAnt AI right here. Just type the following in a comment on your pull request, and replace "Your question here" with whatever you want to ask:

@codeant-ai ask: Your question here

This lets you have a chat with CodeAnt AI about your pull request, making it easier to understand and improve your code.

Example

@codeant-ai ask: Can you suggest a safer alternative to storing this secret?

Preserve Org Learnings with CodeAnt

You can record team preferences so CodeAnt AI applies them in future reviews. Reply directly to the specific CodeAnt AI suggestion (in the same thread) and replace "Your feedback here" with your input:

@codeant-ai: Your feedback here

This helps CodeAnt AI learn and adapt to your team's coding style and standards.

Example

@codeant-ai: Do not flag unused imports.

Retrigger review

Ask CodeAnt AI to review the PR again, by typing:

@codeant-ai: review

Check Your Repository Health

To analyze the health of your code repository, visit our dashboard at https://app.codeant.ai. This tool helps you identify potential issues and areas for improvement in your codebase, ensuring your repository maintains high standards of code health.

@codeant-ai
Copy link
Copy Markdown
Contributor

codeant-ai bot commented Mar 24, 2026

CodeAnt AI is reviewing your PR.


Thanks for using CodeAnt! 🎉

We're free for open-source projects. if you're enjoying it, help us grow by sharing.

Share on X ·
Reddit ·
LinkedIn

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai bot commented Mar 24, 2026

Note

Reviews paused

It 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 reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

readFileFromDisk now checks fs.stat early; if the path is a directory it returns a plaintext FileResult with metadata.isDirectory: true and a newline tree produced by listDirectory() (called under a timeout). Non-directory flows use the existing timed file-read path.

Changes

Cohort / File(s) Summary
Filesystem read logic
src/tools/filesystem.ts
Added early fs.stat() check in readFileFromDisk; when stats.isDirectory() is true, run listDirectory() under withTimeout(FILE_READ, 'Directory listing fallback'), format entries into a tree, and return a plaintext FileResult with metadata.isDirectory: true. Listing timeouts/errors are rethrown; existing file-read logic retained for non-directories or stat failures.
Preview handling
src/handlers/filesystem-handlers.ts
When a read returns metadata?.isDirectory, structuredContent.fileType is forced to 'text' instead of using resolvePreviewFileType(...).
File metadata type
src/utils/files/base.ts
Extended FileMetadata with optional isDirectory?: boolean to mark directory results from reads.

Sequence Diagram(s)

sequenceDiagram
  participant Caller
  participant Reader as readFileFromDisk
  participant Stat as fs.stat()
  participant Timeout as withTimeout()
  participant List as listDirectory()
  participant Result as FileResult

  Caller->>Reader: request(path)
  Reader->>Stat: stat(path)
  alt stats.isDirectory()
    Reader->>Timeout: listDirectory(path) with timeout
    Timeout->>List: perform listing
    List-->>Timeout: entries or throw
    Timeout-->>Reader: entries (or throw)
    Reader->>Result: build plaintext result (isDirectory: true + tree)
    Result-->>Caller: return directory FileResult
  else not a directory / stat/listing non-fatal
    Reader->>Reader: proceed with existing timed file-read flow
    Reader-->>Caller: return file read result
  end
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

Suggested reviewers

  • serg33v
  • dmitry-ottic-ai

Poem

🐰 I nudged a path and found a tree,
Lines like twigs, listed for me,
"Use list_directory" — a plaintext cheer,
I hop through names both far and near,
A tiny rabbit, cataloguing with glee. 🥕

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and specifically describes the main change: handling read_file calls on directories gracefully with an EISDIR fallback mechanism.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/read-file-directory-fallback

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@codeant-ai codeant-ai bot added the size:S This PR changes 10-29 lines, ignoring generated files label Mar 24, 2026
@codeant-ai
Copy link
Copy Markdown
Contributor

codeant-ai bot commented Mar 24, 2026

Sequence Diagram

This PR changes read_file so directory paths no longer fail with a low level error. The tool now detects directories, returns a directory listing with guidance to use list_directory, while keeping normal file reads unchanged.

sequenceDiagram
    participant Agent
    participant FileTool
    participant Disk

    Agent->>FileTool: Call read_file with path
    FileTool->>FileTool: Validate path and check file stats

    alt Path is directory
        FileTool->>Disk: List directory entries
        Disk-->>FileTool: Directory names
        FileTool-->>Agent: Return listing and list_directory hint
    else Path is file
        FileTool->>Disk: Read file content
        Disk-->>FileTool: File content
        FileTool-->>Agent: Return file content
    end
Loading

Generated by CodeAnt AI

@codeant-ai
Copy link
Copy Markdown
Contributor

codeant-ai bot commented Mar 24, 2026

Nitpicks 🔍

🔒 No security issues identified
⚡ Recommended areas for review

  • Fallback Failure
    The directory branch is inside the same try block as fs.stat(), so if listDirectory() throws, execution falls through to the normal file-read path and can still surface the original EISDIR behavior. This fallback should be isolated and return a safe response even when directory listing fails.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 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/tools/filesystem.ts`:
- Around line 376-385: The directory handling branch using stats.isDirectory()
currently calls listDirectory(validPath) inside the outer try-catch so any error
there falls through to the readOperation and triggers EISDIR; modify the block
around listDirectory(validPath) (inside the stats.isDirectory() branch) to have
its own try-catch that either returns a proper directory-response (with the
listing) on success or returns/throws a clear directory-error on failure so
execution never falls through to readOperation; target functions/variables to
change: stats.isDirectory, listDirectory(validPath), filePath, readOperation to
ensure directory errors are handled separately from fs.stat() errors.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 721d8343-4c40-41bf-b097-abd2435177c1

📥 Commits

Reviewing files that changed from the base of the PR and between d854870 and 50ce4d7.

📒 Files selected for processing (4)
  • package.json
  • server.json
  • src/tools/filesystem.ts
  • src/version.ts

Comment on lines +378 to +379
const entries = await listDirectory(validPath);
const listing = entries.join('\n');
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggestion: The directory fallback runs before the existing file-read timeout wrapper, so large or slow directory traversals can block much longer than the intended read timeout. Wrap the listDirectory call with the same timeout policy used for file reads to avoid hangs and keep behavior consistent. [logic error]

Severity Level: Major ⚠️
- ⚠️ read_file directory fallback ignores FILE_READ 30s timeout.
- ⚠️ read_multiple_files may stall on slow directory traversal.
Suggested change
const entries = await listDirectory(validPath);
const listing = entries.join('\n');
const entries = await withTimeout(
listDirectory(validPath),
FILE_OPERATION_TIMEOUTS.FILE_READ,
`List directory operation for ${filePath}`,
null
);
const listing = (entries ?? []).join('\n');
Steps of Reproduction ✅
1. Call MCP tool `read_file` (registered at `src/server.ts:281`) with a directory path.

2. Request enters `handleReadFile()` at `src/handlers/filesystem-handlers.ts:72`, then
calls `readFile(parsed.path, options)` at `src/handlers/filesystem-handlers.ts:108`.

3. `readFile()` routes to `readFileFromDisk()` at `src/tools/filesystem.ts:457-464`;
directory branch triggers at `src/tools/filesystem.ts:377-379` and directly awaits
`listDirectory(validPath)`.

4. `listDirectory()` performs recursive `fs.readdir` traversal at
`src/tools/filesystem.ts:597-609` without `withTimeout`, so this path skips the file-read
timeout wrapper at `src/tools/filesystem.ts:97-102`.

5. Observe timeout inconsistency: `read_file` is only bounded by handler timeout 60s
(`src/handlers/filesystem-handlers.ts:187-192`), and `read_multiple_files`
(`src/handlers/filesystem-handlers.ts:203-205`) has no handler timeout at all, so
directory fallback can wait significantly longer than `FILE_READ` policy.
Prompt for AI Agent 🤖
This is a comment left during a code review.

**Path:** src/tools/filesystem.ts
**Line:** 378:379
**Comment:**
	*Logic Error: The directory fallback runs before the existing file-read timeout wrapper, so large or slow directory traversals can block much longer than the intended read timeout. Wrap the `listDirectory` call with the same timeout policy used for file reads to avoid hangs and keep behavior consistent.

Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
👍 | 👎

@codeant-ai
Copy link
Copy Markdown
Contributor

codeant-ai bot commented Mar 24, 2026

CodeAnt AI finished reviewing your PR.

@wonderwhy-er wonderwhy-er force-pushed the fix/read-file-directory-fallback branch 2 times, most recently from 17b3273 to 7f75d63 Compare March 24, 2026 17:31
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

♻️ Duplicate comments (1)
src/tools/filesystem.ts (1)

376-385: ⚠️ Potential issue | 🟠 Major

listDirectory error still falls through to file-read path.

The concern raised in the previous review remains unaddressed: listDirectory(validPath) at line 378 is still inside the outer try-catch (lines 373-399). If listDirectory throws (e.g., permission denied on the directory), the catch block logs and continues to readOperation, which will attempt to read the directory as a file—re-triggering the EISDIR error this PR aims to fix.

Please wrap the directory-handling logic in its own try-catch as suggested in the prior review, so errors from listDirectory either return a clear error message or re-throw, preventing fallthrough.

,

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/tools/filesystem.ts` around lines 376 - 385, The directory-handling
branch currently calls listDirectory(validPath) inside the outer try-catch, so
if listDirectory throws you fall through to the readOperation path; wrap the
entire stats.isDirectory() block (including the call to listDirectory and the
return of the directory listing object) in its own try-catch so
directory-listing errors do not continue to the file-read logic; on catch,
either return a clear error result (e.g., content explaining permission/ listing
failure for filePath and mimeType 'text/plain') or re-throw the error to surface
it, ensuring functions like listDirectory and the readOperation/readFile logic
are never both attempted for the same request.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Duplicate comments:
In `@src/tools/filesystem.ts`:
- Around line 376-385: The directory-handling branch currently calls
listDirectory(validPath) inside the outer try-catch, so if listDirectory throws
you fall through to the readOperation path; wrap the entire stats.isDirectory()
block (including the call to listDirectory and the return of the directory
listing object) in its own try-catch so directory-listing errors do not continue
to the file-read logic; on catch, either return a clear error result (e.g.,
content explaining permission/ listing failure for filePath and mimeType
'text/plain') or re-throw the error to surface it, ensuring functions like
listDirectory and the readOperation/readFile logic are never both attempted for
the same request.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 7e7fbb38-f5f0-4135-89df-fd2427d70e85

📥 Commits

Reviewing files that changed from the base of the PR and between 50ce4d7 and 17b3273.

📒 Files selected for processing (1)
  • src/tools/filesystem.ts

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

♻️ Duplicate comments (1)
src/tools/filesystem.ts (1)

396-403: ⚠️ Potential issue | 🟡 Minor

Don't rely on error text to prevent fallthrough.

Only errors whose message happens to contain Directory listing or list_directory are rethrown here. Since listDirectory() does its own validatePath() call, other directory-branch failures can still slip into the normal file-read path and reintroduce the misleading EISDIR/file-read behavior.

💡 Suggested change
     try {
         const stats = await fs.stat(validPath);
         if (stats.isDirectory()) {
-            const dirResult = await withTimeout(
-                dirListOp(),
-                FILE_OPERATION_TIMEOUTS.FILE_READ,
-                'Directory listing fallback',
-                null
-            );
-            if (dirResult === null) {
-                throw new Error(`Directory listing timed out for: ${filePath}`);
-            }
-            return dirResult;
+            try {
+                const dirResult = await withTimeout(
+                    dirListOp(),
+                    FILE_OPERATION_TIMEOUTS.FILE_READ,
+                    'Directory listing fallback',
+                    null
+                );
+                if (dirResult === null) {
+                    throw new Error(`Directory listing timed out for: ${filePath}`);
+                }
+                return dirResult;
+            } catch (error) {
+                throw new Error(`Failed to list directory for ${filePath}: ${error instanceof Error ? error.message : String(error)}`);
+            }
         }
-    } catch (error) {
-        // If stat itself failed, fall through to the read path which will produce a proper error.
-        // But if this was a directory-listing error, re-throw — don't let it fall into the file-read path.
-        const err = error as NodeJS.ErrnoException;
-        if (err.message?.includes('Directory listing') || err.message?.includes('list_directory')) {
-            throw error;
-        }
-        // stat() failed (e.g. ENOENT) — fall through to the read path below
+    } catch {
+        // stat() failed (e.g. ENOENT) — fall through to the read path below
     }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/tools/filesystem.ts` around lines 396 - 403, The current catch block
relies on checking error.message text to detect directory-listing failures;
instead, update the logic to detect directory errors by inspecting a stable
error indicator: check err.code for directory-related values (e.g., 'EISDIR' or
other OS-specific directory errno values) or, if listDirectory() throws a custom
error, check a dedicated property (e.g., err.isDirectoryError or err.type ===
'DirectoryListingError') rather than matching substrings; keep the rest of the
flow (rethrow directory errors, fallthrough for stat ENOENT) and reference the
existing stat() call and listDirectory()/validatePath() behavior when making the
change.
🧹 Nitpick comments (1)
src/tools/filesystem.ts (1)

376-380: Keep this fallback shallow and capped.

This recovery path currently inherits the full listDirectory() behavior, which means a second path validation plus recursive traversal/output building. For an accidental read_file on a large folder, that's a lot more work than needed just to steer the agent toward list_directory. A capped top-level preview would keep the fallback much cheaper and more predictable.

💡 Suggested change
         if (stats.isDirectory()) {
             const dirListOp = async () => {
-                const entries = await listDirectory(validPath);
-                const listing = entries.join('\n');
+                const maxEntries = 100;
+                const entries = await fs.readdir(validPath, { withFileTypes: true });
+                const visibleEntries = entries.slice(0, maxEntries).map((entry) =>
+                    `${entry.isDirectory() ? '[DIR]' : '[FILE]'} ${entry.name}`
+                );
+                const hiddenCount = entries.length - visibleEntries.length;
+                const listing = [
+                    ...visibleEntries,
+                    ...(hiddenCount > 0 ? [`[WARNING] ${hiddenCount} items hidden`] : [])
+                ].join('\n');
                 return {
                     content: `This is a directory, not a file. Use the list_directory tool instead of read_file for directories.\n\nDirectory listing for ${filePath}:\n${listing}`,
                     mimeType: 'text/plain',
                     metadata: { isImage: false }
                 } as FileResult;
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/tools/filesystem.ts` around lines 376 - 380, The fallback in dirListOp
currently calls listDirectory with its full recursive traversal and output;
change it to produce a shallow, capped preview instead: use the
already-validated validPath (avoid re-validating filePath), retrieve only the
top-level entries (no recursion) and limit the number of items (e.g., first N
entries) and total characters, then build a short message pointing the caller to
use the list_directory tool and noting when the listing was truncated; update
the message keys referenced (dirListOp, listDirectory, filePath, validPath,
read_file, list_directory) accordingly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Duplicate comments:
In `@src/tools/filesystem.ts`:
- Around line 396-403: The current catch block relies on checking error.message
text to detect directory-listing failures; instead, update the logic to detect
directory errors by inspecting a stable error indicator: check err.code for
directory-related values (e.g., 'EISDIR' or other OS-specific directory errno
values) or, if listDirectory() throws a custom error, check a dedicated property
(e.g., err.isDirectoryError or err.type === 'DirectoryListingError') rather than
matching substrings; keep the rest of the flow (rethrow directory errors,
fallthrough for stat ENOENT) and reference the existing stat() call and
listDirectory()/validatePath() behavior when making the change.

---

Nitpick comments:
In `@src/tools/filesystem.ts`:
- Around line 376-380: The fallback in dirListOp currently calls listDirectory
with its full recursive traversal and output; change it to produce a shallow,
capped preview instead: use the already-validated validPath (avoid re-validating
filePath), retrieve only the top-level entries (no recursion) and limit the
number of items (e.g., first N entries) and total characters, then build a short
message pointing the caller to use the list_directory tool and noting when the
listing was truncated; update the message keys referenced (dirListOp,
listDirectory, filePath, validPath, read_file, list_directory) accordingly.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 79ea9507-039c-4381-8305-0677660be767

📥 Commits

Reviewing files that changed from the base of the PR and between 17b3273 and 7f75d63.

📒 Files selected for processing (1)
  • src/tools/filesystem.ts

@wonderwhy-er wonderwhy-er force-pushed the fix/read-file-directory-fallback branch from 7f75d63 to a65d930 Compare March 25, 2026 10:45
@codeant-ai
Copy link
Copy Markdown
Contributor

codeant-ai bot commented Mar 25, 2026

CodeAnt AI is running Incremental review


Thanks for using CodeAnt! 🎉

We're free for open-source projects. if you're enjoying it, help us grow by sharing.

Share on X ·
Reddit ·
LinkedIn

@codeant-ai codeant-ai bot added size:M This PR changes 30-99 lines, ignoring generated files and removed size:S This PR changes 10-29 lines, ignoring generated files labels Mar 25, 2026
@codeant-ai
Copy link
Copy Markdown
Contributor

codeant-ai bot commented Mar 25, 2026

CodeAnt AI Incremental review completed.

@wonderwhy-er wonderwhy-er force-pushed the fix/read-file-directory-fallback branch from a65d930 to 4be21c9 Compare March 25, 2026 10:48
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

♻️ Duplicate comments (1)
src/tools/filesystem.ts (1)

396-404: ⚠️ Potential issue | 🟠 Major

Fragile error-filtering pattern may still allow EISDIR fallthrough.

The substring-based error filtering (err.message?.includes('Directory listing') || err.message?.includes('list_directory')) doesn't catch all directory-handling errors. If listDirectory throws an error from its internal validatePath call (e.g., "Path not allowed: ..."), the message won't match either pattern and execution falls through to the file-read path, re-triggering EISDIR.

A safer approach is to track that we're in the directory branch and always re-throw, or catch directory errors in a nested try-catch.

🐛 Proposed fix: use a flag or nested try-catch
     // Check if path is a directory — return listing instead of EISDIR error
-    try {
-        const stats = await fs.stat(validPath);
-        if (stats.isDirectory()) {
+    let isDirectory = false;
+    try {
+        const stats = await fs.stat(validPath);
+        isDirectory = stats.isDirectory();
+    } catch {
+        // stat() failed (e.g. ENOENT) — fall through to the read path below
+    }
+
+    if (isDirectory) {
+        try {
             const dirListOp = async () => {
                 const entries = await listDirectory(validPath);
                 const listing = entries.join('\n');
                 return {
                     content: `This is a directory, not a file. Use the list_directory tool instead of read_file for directories.\n\nDirectory listing for ${filePath}:\n${listing}`,
                     mimeType: 'text/plain',
                     metadata: { isImage: false, isDirectory: true }
                 } as FileResult;
             };
             const dirResult = await withTimeout(
                 dirListOp(),
                 FILE_OPERATION_TIMEOUTS.FILE_READ,
                 'Directory listing fallback',
                 null
             );
             if (dirResult === null) {
                 throw new Error(`Directory listing timed out for: ${filePath}`);
             }
             return dirResult;
+        } catch (dirError) {
+            const errorMessage = dirError instanceof Error ? dirError.message : String(dirError);
+            throw new Error(`Path is a directory but failed to list contents: ${errorMessage}`);
         }
-    } catch (error) {
-        // If stat itself failed, fall through to the read path which will produce a proper error.
-        // But if this was a directory-listing error, re-throw — don't let it fall into the file-read path.
-        const err = error as NodeJS.ErrnoException;
-        if (err.message?.includes('Directory listing') || err.message?.includes('list_directory')) {
-            throw error;
-        }
-        // stat() failed (e.g. ENOENT) — fall through to the read path below
     }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/tools/filesystem.ts` around lines 396 - 404, The current substring-based
filtering of the caught error in the stat branch (checking err.message for
'Directory listing' or 'list_directory') is fragile and can miss
directory-related errors (causing EISDIR to fall through); update the logic in
the stat + directory handling flow (where stat(), listDirectory(), and
validatePath() are used) to explicitly track the directory branch—either set a
boolean flag like isDirectoryAttempt before calling listDirectory() and re-throw
any error if that flag is set, or use a nested try-catch around the
listDirectory() call so directory-related errors are caught and re-thrown
immediately instead of falling through to the file-read path; ensure you
reference and adjust the same error variable (err) handling so EISDIR cannot be
produced by the fallthrough.
🧹 Nitpick comments (2)
src/tools/filesystem.ts (1)

372-408: Duplicate fs.stat calls for non-directory paths.

When the path is a file (not a directory), fs.stat(validPath) is called twice: once at line 374 for the directory check, and again at line 408 for telemetry. Consider reusing the stats object from the first call to avoid the redundant syscall.

♻️ Proposed optimization to reuse stats
     // Check if path is a directory — return listing instead of EISDIR error
+    let cachedStats: Awaited<ReturnType<typeof fs.stat>> | null = null;
     try {
-        const stats = await fs.stat(validPath);
-        if (stats.isDirectory()) {
+        cachedStats = await fs.stat(validPath);
+        if (cachedStats.isDirectory()) {
             // ... directory handling ...
         }
     } catch (error) {
         // ... error handling ...
     }

     // Check file size before attempting to read
     try {
-        const stats = await fs.stat(validPath);
+        const stats = cachedStats ?? await fs.stat(validPath);
         // ... telemetry ...
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/tools/filesystem.ts` around lines 372 - 408, The code calls
fs.stat(validPath) twice (once to check directory and again before reading)
causing a redundant syscall; modify the flow in the method that performs the
directory check and file-read telemetry so you capture the first fs.stat result
into a variable (e.g., stats) and reuse that same stats object for the later
size/telemetry checks instead of calling fs.stat(validPath) again, preserving
the current error-handling: if stats.isDirectory() return the directory-listing
path (using listDirectory, withTimeout, FILE_OPERATION_TIMEOUTS.FILE_READ,
etc.), and if the initial stat throws let it fall through to the read path
exactly as today.
src/handlers/filesystem-handlers.ts (1)

174-174: Consider a dedicated fileType for directories.

Setting fileType to 'text' works functionally, but based on the preview rendering logic in src/ui/file-preview/src/app.ts, this causes directories to:

  • Render as syntax-highlighted code (via renderCodeViewer)
  • Show canCopy: true and supportsPreview: true
  • Display a potentially misleading file type label based on path extension

For now this is acceptable since the primary goal is avoiding EISDIR errors, but a follow-up could introduce a 'directory' fileType with dedicated UI handling for clearer UX.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/handlers/filesystem-handlers.ts` at line 174, The code currently coerces
directories to 'text' via the expression fileResult.metadata?.isDirectory ?
'text' as const : resolvePreviewFileType(resolvedFilePath); which causes
directories to be treated as code previews; change this to emit a dedicated
'directory' fileType when fileResult.metadata?.isDirectory is true (e.g.,
fileType = 'directory'), and then update the associated type/unions and any
callers that consume fileType (search for resolvePreviewFileType, fileType
usage, and preview rendering code in src/ui/file-preview/src/app.ts) so the new
'directory' value is accepted and can be handled separately in the UI (or
fallback safely) to avoid being rendered as code.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Duplicate comments:
In `@src/tools/filesystem.ts`:
- Around line 396-404: The current substring-based filtering of the caught error
in the stat branch (checking err.message for 'Directory listing' or
'list_directory') is fragile and can miss directory-related errors (causing
EISDIR to fall through); update the logic in the stat + directory handling flow
(where stat(), listDirectory(), and validatePath() are used) to explicitly track
the directory branch—either set a boolean flag like isDirectoryAttempt before
calling listDirectory() and re-throw any error if that flag is set, or use a
nested try-catch around the listDirectory() call so directory-related errors are
caught and re-thrown immediately instead of falling through to the file-read
path; ensure you reference and adjust the same error variable (err) handling so
EISDIR cannot be produced by the fallthrough.

---

Nitpick comments:
In `@src/handlers/filesystem-handlers.ts`:
- Line 174: The code currently coerces directories to 'text' via the expression
fileResult.metadata?.isDirectory ? 'text' as const :
resolvePreviewFileType(resolvedFilePath); which causes directories to be treated
as code previews; change this to emit a dedicated 'directory' fileType when
fileResult.metadata?.isDirectory is true (e.g., fileType = 'directory'), and
then update the associated type/unions and any callers that consume fileType
(search for resolvePreviewFileType, fileType usage, and preview rendering code
in src/ui/file-preview/src/app.ts) so the new 'directory' value is accepted and
can be handled separately in the UI (or fallback safely) to avoid being rendered
as code.

In `@src/tools/filesystem.ts`:
- Around line 372-408: The code calls fs.stat(validPath) twice (once to check
directory and again before reading) causing a redundant syscall; modify the flow
in the method that performs the directory check and file-read telemetry so you
capture the first fs.stat result into a variable (e.g., stats) and reuse that
same stats object for the later size/telemetry checks instead of calling
fs.stat(validPath) again, preserving the current error-handling: if
stats.isDirectory() return the directory-listing path (using listDirectory,
withTimeout, FILE_OPERATION_TIMEOUTS.FILE_READ, etc.), and if the initial stat
throws let it fall through to the read path exactly as today.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: c72887d9-a2f0-4ac6-8110-13bccd02621e

📥 Commits

Reviewing files that changed from the base of the PR and between 7f75d63 and a65d930.

📒 Files selected for processing (3)
  • src/handlers/filesystem-handlers.ts
  • src/tools/filesystem.ts
  • src/utils/files/base.ts
✅ Files skipped from review due to trivial changes (1)
  • src/utils/files/base.ts

@wonderwhy-er wonderwhy-er force-pushed the fix/read-file-directory-fallback branch 7 times, most recently from c5e2524 to 8fe69ef Compare March 26, 2026 12:35
@codeant-ai
Copy link
Copy Markdown
Contributor

codeant-ai bot commented Mar 26, 2026

CodeAnt AI is running Incremental review


Thanks for using CodeAnt! 🎉

We're free for open-source projects. if you're enjoying it, help us grow by sharing.

Share on X ·
Reddit ·
LinkedIn

@codeant-ai codeant-ai bot added size:L This PR changes 100-499 lines, ignoring generated files and removed size:M This PR changes 30-99 lines, ignoring generated files labels Mar 26, 2026
@codeant-ai
Copy link
Copy Markdown
Contributor

codeant-ai bot commented Mar 26, 2026

CodeAnt AI Incremental review completed.

@wonderwhy-er wonderwhy-er force-pushed the fix/read-file-directory-fallback branch from 8fe69ef to e46fbb9 Compare March 26, 2026 14:20
// Build flat list
const flat: { name: string; fullPath: string; isDir: boolean; isDenied: boolean; depth: number }[] = [];
for (const line of entryLines) {
if (line.startsWith('[WARNING]')) continue;
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Very low risk, but, if there are 100 sub folders, we show warning that N folders are hidden, those would not appear in the tree.

@wonderwhy-er wonderwhy-er force-pushed the fix/read-file-directory-fallback branch from e46fbb9 to 154d015 Compare March 27, 2026 05:43
@codeant-ai
Copy link
Copy Markdown
Contributor

codeant-ai bot commented Mar 27, 2026

CodeAnt AI is running Incremental review


Thanks for using CodeAnt! 🎉

We're free for open-source projects. if you're enjoying it, help us grow by sharing.

Share on X ·
Reddit ·
LinkedIn

@codeant-ai codeant-ai bot added size:L This PR changes 100-499 lines, ignoring generated files and removed size:L This PR changes 100-499 lines, ignoring generated files labels Mar 27, 2026
@codeant-ai
Copy link
Copy Markdown
Contributor

codeant-ai bot commented Mar 27, 2026

CodeAnt AI Incremental review completed.

When an agent calls read_file on a directory path, instead of throwing
EISDIR error, detect the directory via stat() and fall back to
listDirectory() internally. Returns the directory listing along with
a hint to use list_directory tool next time.

This prevents confusing EISDIR errors and gives the agent useful
results immediately.
@wonderwhy-er wonderwhy-er force-pushed the fix/read-file-directory-fallback branch from 154d015 to 9700c4f Compare March 27, 2026 05:48
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unclear if the changes to the lockfile are necessary for this change since no dependencies are added/changed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:L This PR changes 100-499 lines, ignoring generated files

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants