Skip to content

filesystem: recursive search can hang on macOS CloudStorage / lazy provider paths #4162

Description

@alemuenchen

Hi — thanks for maintaining the filesystem server.

I'm filing this against @modelcontextprotocol/server-filesystem after several weeks of production use on macOS with provider-backed directories in allowedDirectories,
especially:

  • ~/Library/CloudStorage/GoogleDrive-.../...
  • ~/Library/Mobile Documents/com~apple~CloudDocs/...

Symptom

When allowedDirectories includes macOS CloudStorage / lazy provider paths, search_files and other operations that call validatePath() can intermittently hang for minutes.
Claude Desktop eventually reports the MCP server as unresponsive after roughly 4 minutes. The subprocess remains alive, but subsequent filesystem calls may continue to hang until
the server is restarted.

This may overlap with reports like #3281, but I'm filing separately because this issue points to specific filesystem code paths.

Code paths

In src/filesystem/lib.ts, validatePath() calls:

const realPath = await fs.realpath(absolute);

and, in the ENOENT branch:

const realParentPath = await fs.realpath(parentDir);

These calls have no timeout. On macOS provider-backed paths that are not locally materialized, realpath can block while the provider fetches metadata.

searchFilesWithValidation() then recursively does:

const entries = await fs.readdir(currentPath, { withFileTypes: true });
...
await validatePath(fullPath);
...
if (entry.isDirectory()) {
  await search(fullPath);
}

As far as I can tell, recursive search currently has no upper bound on visited entries, no timeout around readdir, and no abort path. A large or lazy-materialized tree can
therefore keep the server busy for a long time.

Why this matters

This is especially visible with CloudStorage / FileProvider-backed folders such as Google Drive and iCloud Drive, where metadata for subdirectories may be fetched lazily. It can
also affect very large local trees.

The libuv threadpool angle may make the symptom worse: filesystem operations use libuv's threadpool, whose default size is 4. A small number of slow filesystem calls can
therefore degrade unrelated filesystem operations in the same process. I mention this as a contributing factor, not necessarily as the main fix.

Suggested mitigations

The two minimal mitigations I'd suggest are:

  1. Add a timeout wrapper around fs.realpath in validatePath() and around fs.readdir in recursive search.

  2. Add a configurable maximum visited-entry count for search_files, returning a clear error such as:

    Search aborted after visiting N entries. Please refine the pattern or narrow the search path.

Optional follow-ups:

  • Document how to exclude / avoid recursive search over CloudStorage-style folders.
  • Consider an opt-in exclude-prefix configuration for recursive search.
  • Document UV_THREADPOOL_SIZE tuning for users who intentionally expose large / slow provider-backed trees.

I have tested equivalent mitigations locally in a patched installed build for several weeks with Google Drive and iCloud Drive paths in allowedDirectories. I'm not opening a PR
because the exact configuration surface — env vars vs config object, defaults, and docs placement — seems like a maintainer policy decision, but I'd be happy to test any
proposed fix.

Activity

  1. alemuenchen commented on May 17, 2026

    @alemuenchen
    Author

    Thanks @Sean-Kenneth-Doherty for picking this up.

    Being able to exclude a known-slow folder before the server tries to touch it is genuinely useful — on my setup the slow paths are always the same Google Drive and iCloud Drive folders, so even this first step would help in practice.

    And thanks for being upfront that it doesn't cover everything: the timeout, the cap on how many entries get visited, the abort path, and directory_tree are still where the real hangs come from for me. For several weeks I've been running a patched build with timeout + max-entry mitigations and Google Drive / iCloud paths in allowedDirectories, and it's been stable. Happy to share what I did if it helps a follow-up, and happy to test #4181 on my CloudStorage setup.

  2. JonoGitty commented on May 18, 2026

    @JonoGitty

    Hi @alemuenchen, @Sean-Kenneth-Doherty — I can reproduce this reliably on macOS with both ~/Library/CloudStorage/OneDrive-... and ~/Library/Mobile Documents/com~apple~CloudDocs/... in allowedDirectories. search_files hangs for minutes on lazy-materialized subtrees; directory_tree is worse.

    #4181 (excludePatterns before validatePath) is a useful first cut but, as @alemuenchen noted, doesn't address the underlying timeout / abort / max-entry gaps. To avoid stepping on that PR I'd like to scope a follow-up to the parts not covered:

    1. Timeout wrapper around fs.realpath in validatePath() (configurable via env, sane default e.g. 5s) returning a structured error rather than hanging.
    2. Timeout wrapper around fs.readdir in searchFilesWithValidation() and directory_tree.
    3. Configurable max-visited-entries cap for search_files and directory_tree, returning a clear "search aborted after N entries — narrow the path or pattern" error.
    4. Tests using a mocked slow realpath / readdir so CI doesn't depend on a real CloudStorage path.

    Happy to drop any of these from scope if maintainers prefer them split into separate PRs, or if there's existing design preference on config surface (env vars vs constructor options). Will hold off coding until either a maintainer or @Sean-Kenneth-Doherty signals this scope is fine.

  3. alemuenchen commented on May 18, 2026

    @alemuenchen
    Author

    Thanks @JonoGitty — your four-point scope matches exactly where the real hangs come from on my setup, so from the reporter's side this is the right follow-up to #4181.

    Two things that might save you time:

    • I've been running a patched build with mitigations 1–3 (realpath/readdir timeouts + a max-visited-entries cap) for several weeks, with Google Drive and iCloud Drive paths in allowedDirectories, and it's been stable. Happy to share the diff so you can compare design choices — I went with env-var config and conservative defaults, which matches your instinct.
    • On point 4: a mocked slow realpath/readdir is the way to go. The hang is 100% reproducible with a real CloudStorage path, but that's obviously unusable in CI.

    I'm just the reporter, not a maintainer, so I can't green-light the scope — but for what it's worth, consider this a +1 that splitting this out from #4181 is the right call. Happy to test your branch on my CloudStorage setup once it's up.

  4. JonoGitty commented on May 18, 2026

    @JonoGitty

    Thanks @alemuenchen — yes please, I'd love to see the diff. Saves me re-deriving design decisions you've already validated in real use. Happy to credit your work in the PR description if it ends up materially shaping the patch.

    Agreed the scope split makes sense regardless of which way maintainers go on config surface. I'll hold off coding until a maintainer chimes in, but the diff will let me have a draft ready to put up the moment they do — and your offer to test on a live CloudStorage setup is exactly the validation this fix needs.

  5. alemuenchen commented on May 18, 2026

    @alemuenchen
    Author

    @JonoGitty — here's the diff, against current main (src/filesystem/lib.ts). Treat it as a reference, not a PR: take whatever design decisions are useful and drop the rest.

    What it does (your scope points 1–3, search side):

    • withFsTimeout() wraps every fs.realpath in validatePath() and every fs.readdir in searchFilesWithValidation(). On timeout it rejects with a typed FsTimeoutError.
    • FS_SEARCH_MAX_VISITED caps recursive search; hitting the cap aborts with a clear "narrow the path / refine the pattern" error.
    • FS_SEARCH_EXCLUDE_PREFIXES — opt-in, empty by default — lets a user refuse recursive search on known-slow roots outright. Containment reuses isPathWithinAllowedDirectories, so it's boundary-aware (a prefix can't match an unrelated sibling that just shares a string head).
    • Config is env-var driven, defaults conservative (15s timeout, 50k entries). Invalid numeric values are ignored with a stderr warning rather than silently disabling a guard.

    Design notes / things I deliberately left for your PR:

    • FsTimeoutError propagates from any depth — the per-entry catch in search re-throws timeouts instead of swallowing them, so a search never returns silently-partial results. Tradeoff: one genuinely-hung subtree aborts the whole search. If you'd rather collect skipped paths and return partial results + a warning, that needs an API change (search_files currently returns string[]) — your call.
    • directory_tree is not touched here (it lives in index.ts, not lib.ts). Your point 2 includes it; the same withFsTimeout + cap pattern applies.
    • The cap bounds recursion breadth/depth, not a single fs.readdir: one directory with millions of entries is still read in a single call (the timeout bounds the wait, not the allocation). Switching to fs.opendir streaming would fix that but is a larger structural change — flagging it as a known limitation.
    • The libuv threadpool angle from the issue (slow calls still stuck in the pool after a timeout) isn't addressed — a concurrency limiter is a separate mitigation.
    • No tests — your point 4 (mocked slow realpath / readdir) is the right approach; I left it to the PR.

    Env-var names and defaults (FS_OP_TIMEOUT_MS, FS_SEARCH_MAX_VISITED, FS_SEARCH_EXCLUDE_PREFIXES) are just my picks — rename freely to whatever config surface the maintainers settle on.

    This diff was independently code-reviewed before posting. Happy to test your branch on a live Google Drive / iCloud setup once it's up.

    --- a/src/filesystem/lib.ts
    +++ b/src/filesystem/lib.ts
    @@ -38,6 +38,86 @@
     export interface SearchResult {
       path: string;
       isDirectory: boolean;
    +}
    +
    +// ---------------------------------------------------------------------------
    +// Mitigations for issue #4162: recursive search / validatePath can hang for
    +// minutes on macOS CloudStorage / lazy provider-backed paths, because
    +// fs.realpath and fs.readdir have no timeout and recursive search has no
    +// upper bound. All knobs below are env-var configurable and default to
    +// behavior that is safe for ordinary local trees.
    +//
    +//   FS_OP_TIMEOUT_MS            timeout (ms) per fs.realpath / fs.readdir
    +//                               call. Default 15000. Positive integer.
    +//   FS_SEARCH_MAX_VISITED       max entries a single recursive search may
    +//                               visit before aborting. Default 50000.
    +//                               Positive integer.
    +//   FS_SEARCH_EXCLUDE_PREFIXES  comma-separated path prefixes that recursive
    +//                               search refuses outright. Empty by default
    +//                               (no behavior change). '~' is expanded.
    +//
    +// Invalid numeric values are ignored (a warning is logged) and the default is
    +// used, so a typo cannot silently disable a guard.
    +// ---------------------------------------------------------------------------
    +
    +// Raised when a wrapped filesystem call exceeds FS_OP_TIMEOUT_MS. Distinct
    +// from ordinary fs errors so callers can tell a hang apart from e.g. EACCES
    +// and avoid silently swallowing it.
    +class FsTimeoutError extends Error {
    +  constructor(message: string) {
    +    super(message);
    +    this.name = 'FsTimeoutError';
    +  }
    +}
    +
    +// Reads a positive-integer env var, falling back to `defaultValue` (with a
    +// warning) when unset or invalid. Uses Number() rather than parseInt() so that
    +// partially-numeric garbage such as "15s" is rejected instead of truncated.
    +function readPositiveIntEnv(name: string, defaultValue: number): number {
    +  const raw = process.env[name];
    +  if (raw === undefined || raw.trim() === '') return defaultValue;
    +  const parsed = Number(raw);
    +  if (!Number.isInteger(parsed) || parsed < 1) {
    +    console.error(`[filesystem] Ignoring invalid ${name}="${raw}"; using default ${defaultValue}.`);
    +    return defaultValue;
    +  }
    +  return parsed;
    +}
    +
    +// Timeout (ms) applied to individual fs.realpath / fs.readdir calls.
    +const FS_OP_TIMEOUT_MS = readPositiveIntEnv('FS_OP_TIMEOUT_MS', 15000);
    +
    +// Hard cap on entries visited by a single recursive search before it aborts.
    +const FS_SEARCH_MAX_VISITED = readPositiveIntEnv('FS_SEARCH_MAX_VISITED', 50000);
    +
    +// Opt-in: comma-separated path prefixes that recursive search refuses outright.
    +// Empty by default -- no behavior change unless the user sets it. Intended for
    +// known-slow provider-backed roots (e.g. a CloudStorage folder). Each entry is
    +// expanded ('~') and normalized so the containment check below is boundary-
    +// aware: "/data/foo" must not match an unrelated "/data/foobar" root.
    +const FS_SEARCH_EXCLUDE_PREFIXES = (process.env.FS_SEARCH_EXCLUDE_PREFIXES ?? '')
    +  .split(',')
    +  .map(prefix => prefix.trim())
    +  .filter(prefix => prefix.length > 0)
    +  .map(prefix => normalizePath(path.resolve(expandHome(prefix))));
    +
    +// Races a filesystem promise against a timeout. On timeout it rejects with an
    +// FsTimeoutError instead of letting the caller hang. Note: the underlying fs
    +// operation cannot be cancelled and may still settle in the background; the
    +// timeout only unblocks the caller.
    +async function withFsTimeout<T>(operation: Promise<T>, label: string): Promise<T> {
    +  let timer: ReturnType<typeof setTimeout>;
    +  const timeout = new Promise<never>((_, reject) => {
    +    timer = setTimeout(
    +      () => reject(new FsTimeoutError(`Filesystem operation timed out after ${FS_OP_TIMEOUT_MS}ms: ${label}`)),
    +      FS_OP_TIMEOUT_MS,
    +    );
    +  });
    +  try {
    +    return await Promise.race([operation, timeout]);
    +  } finally {
    +    clearTimeout(timer!);
    +  }
     }
     
     // Pure Utility Functions
    @@ -112,8 +192,10 @@
     
       // Security: Handle symlinks by checking their real path to prevent symlink attacks
       // This prevents attackers from creating symlinks that point outside allowed directories
    +  // #4162: fs.realpath is wrapped with a timeout so a lazy provider-backed path
    +  // cannot block validation indefinitely.
       try {
    -    const realPath = await fs.realpath(absolute);
    +    const realPath = await withFsTimeout(fs.realpath(absolute), `realpath ${absolute}`);
         const normalizedReal = normalizePath(realPath);
         if (!isPathWithinAllowedDirectories(normalizedReal, allowedDirectories)) {
           throw new Error(`Access denied - symlink target outside allowed directories: ${realPath} not in ${allowedDirectories.join(', ')}`);
    @@ -124,16 +206,21 @@
         // This ensures we can't create files in unauthorized locations
         if ((error as NodeJS.ErrnoException).code === 'ENOENT') {
           const parentDir = path.dirname(absolute);
    +      // #4162: only the realpath call is inside this try, so a timeout
    +      // surfaces as FsTimeoutError and the access-denied check below is not
    +      // masked into a misleading "Parent directory does not exist".
    +      let realParentPath: string;
           try {
    -        const realParentPath = await fs.realpath(parentDir);
    -        const normalizedParent = normalizePath(realParentPath);
    -        if (!isPathWithinAllowedDirectories(normalizedParent, allowedDirectories)) {
    -          throw new Error(`Access denied - parent directory outside allowed directories: ${realParentPath} not in ${allowedDirectories.join(', ')}`);
    -        }
    -        return absolute;
    -      } catch {
    +        realParentPath = await withFsTimeout(fs.realpath(parentDir), `realpath ${parentDir}`);
    +      } catch (parentError) {
    +        if (parentError instanceof FsTimeoutError) throw parentError;
             throw new Error(`Parent directory does not exist: ${parentDir}`);
           }
    +      const normalizedParent = normalizePath(realParentPath);
    +      if (!isPathWithinAllowedDirectories(normalizedParent, allowedDirectories)) {
    +        throw new Error(`Access denied - parent directory outside allowed directories: ${realParentPath} not in ${allowedDirectories.join(', ')}`);
    +      }
    +      return absolute;
         }
         throw error;
       }
    @@ -377,13 +464,47 @@
       allowedDirectories: string[],
       options: SearchOptions = {}
     ): Promise<string[]> {
    +  // #4162: refuse recursive search outright on user-configured slow roots.
    +  // Containment reuses isPathWithinAllowedDirectories so it is boundary-aware:
    +  // a prefix cannot match an unrelated sibling that merely shares a string head.
    +  if (FS_SEARCH_EXCLUDE_PREFIXES.length > 0) {
    +    const normalizedRoot = normalizePath(path.resolve(rootPath));
    +    if (isPathWithinAllowedDirectories(normalizedRoot, FS_SEARCH_EXCLUDE_PREFIXES)) {
    +      throw new Error(
    +        `Recursive search is disabled for this path by FS_SEARCH_EXCLUDE_PREFIXES: ${rootPath}. ` +
    +        `Recursive search over lazy provider-backed trees (macOS CloudStorage / FileProvider) ` +
    +        `is unreliable; use list_directory on specific subfolders, or narrow the search root.`
    +      );
    +    }
    +  }
    +
       const { excludePatterns = [] } = options;
       const results: string[] = [];
    +  // #4162: bound the recursion so a large / lazy-materialized tree cannot keep
    +  // the server busy indefinitely. The search aborts once FS_SEARCH_MAX_VISITED
    +  // entries have been visited; `aborted` is checked at every re-entry.
    +  let visited = 0;
    +  let aborted = false;
     
       async function search(currentPath: string) {
    -    const entries = await fs.readdir(currentPath, { withFileTypes: true });
    +    if (aborted) return;
    +    // #4162: fs.readdir is wrapped with a timeout. A timeout (FsTimeoutError)
    +    // propagates to the caller from any depth: the per-entry catch below
    +    // deliberately re-throws timeouts instead of swallowing them, so a search
    +    // never returns silently-partial results.
    +    const entries = await withFsTimeout(
    +      fs.readdir(currentPath, { withFileTypes: true }),
    +      `readdir ${currentPath}`,
    +    );
     
         for (const entry of entries) {
    +      if (aborted) return;
    +      if (visited >= FS_SEARCH_MAX_VISITED) {
    +        aborted = true;
    +        return;
    +      }
    +      visited++;
    +
           const fullPath = path.join(currentPath, entry.name);
     
           try {
    @@ -403,13 +524,24 @@
     
             if (entry.isDirectory()) {
               await search(fullPath);
    +          if (aborted) return;
             }
    -      } catch {
    +      } catch (error) {
    +        // #4162: a timeout is a real hang, not an inaccessible entry -- surface
    +        // it instead of continuing with partial results.
    +        if (error instanceof FsTimeoutError) throw error;
             continue;
           }
         }
       }
     
       await search(rootPath);
    +  if (aborted) {
    +    throw new Error(
    +      `Search aborted after visiting ${visited} entries ` +
    +      `(cap FS_SEARCH_MAX_VISITED=${FS_SEARCH_MAX_VISITED}). ` +
    +      `Refine the pattern (e.g. '**/name') or narrow the search path.`
    +    );
    +  }
       return results;
     }
  6. JonoGitty commented on May 18, 2026

    @JonoGitty

    Thank you @alemuenchen — this is a serious gift, well beyond what I expected. The design choices are all reasonable to me and several are sharper than what I'd have arrived at independently: re-throwing FsTimeoutError from the per-entry catch (so a search never silently returns partial results), the boundary-aware containment for FS_SEARCH_EXCLUDE_PREFIXES via isPathWithinAllowedDirectories, and warn-and-default on bad env values are all the right calls.

    I also ran the diff through a cross-vendor code-review pass (GPT-5-mini) as a sanity check; it concurred on the design and didn't surface anything beyond what's already mitigated, with one note worth carrying into the test plan: defaults that work well for local trees and CloudStorage may be tight on NFS / SMB / other remote mounts where individual readdir calls legitimately take seconds. Worth documenting in the README alongside the env-var knobs.

    What I'll be picking up from here, assuming maintainers green-light the direction:

    1. Vitest tests with mocked slow fs.realpath / fs.readdir — cap fires, timeout fires, ENOENT path preserved, env-var validation behaviour, exclude-prefix boundary cases.
    2. The same withFsTimeout + cap pattern applied to directory_tree (in index.ts, or lib.ts if refactor(filesystem): extract buildDirectoryTree to lib.ts #4055 lands first).
    3. README updates documenting the three env vars + the remote-mount note.
    4. PR description crediting the diff + your weeks of real-world testing.

    I'll flag the limitations you noted (libuv threadpool, single-directory allocation under fs.readdir, search aborting on first hang vs collecting partial results) in the PR description as deliberately out-of-scope — worth their own follow-ups but conflating them muddies this one.

    Still holding off on opening the PR until a maintainer signals on scope and config-surface preferences. Will ping you to test on a live CloudStorage branch the moment one's up.

  7. alemuenchen commented on May 18, 2026

    @alemuenchen
    Author

    Thanks @JonoGitty — glad the diff lands well and that the design holds up under the second-opinion review. The NFS/SMB caveat is a fair catch; agreed it belongs in the README next to the env-var knobs, and the out-of-scope split for the libuv / partial-results items sounds right to me.

    No rush on the PR from my side — happy to wait for a maintainer to weigh in on scope and config surface. Whenever there's a live branch, I'm set up to test it against real macOS CloudStorage paths, so just ping me.

    Thanks again for picking this up.

  8. JonoGitty commented on May 19, 2026

    @JonoGitty

    Quick note for @alemuenchen — the branch is up at https://github.com/JonoGitty/servers/tree/fix/4162-filesystem-cloudstorage-hangs if you want to pull it for that CloudStorage test you offered. Test suite is green (157/157, incl. 11 new in __tests__/4162-timeouts-and-caps.test.ts), lib.ts coverage 82%. Still holding off opening the PR until a maintainer weighs in on scope.

  9. alemuenchen commented on May 19, 2026

    @alemuenchen
    Author

    @JonoGitty — pulled fix/4162-filesystem-cloudstorage-hangs and tested it on my macOS setup.

    Build & tests: clean build, your 4162-timeouts-and-caps.test.ts is 11/11 green, full suite 157/157.

    Real Google Drive path (~900 dehydrated entries): search_files returns correct results, no regression on pattern matching, and — worth noting — the 15s default timeout does not misfire on a warm provider path.

    FS_SEARCH_MAX_VISITED: aborts deterministically at the cap with a clear error. FS_SEARCH_EXCLUDE_PREFIXES: refuses recursive search as documented; boundary-aware containment confirmed (a /x/foo prefix does not match /x/foobar).

    What I could not reproduce: the multi-minute hang itself. My provider metadata cache is currently warm across the whole tree — I probed ~6000 directories under Google Drive + iCloud Drive and every readdir was <500ms. The hang needs cold provider metadata; "free up space" evicts file content but not directory listings, so it doesn't recreate the cold state on demand. So the timeout's real-world firing on a slow fs call is still only exercised by your mocked unit test — which is the right approach for CI anyway; just being transparent that my live test couldn't add empirical coverage there.

    One real issue (low severity): the README states FS_SEARCH_EXCLUDE_PREFIXES applies to both search_files and directory_tree, but the exclude-prefix check is only implemented in searchFilesWithValidation. directory_tree gets the timeout + visited-cap but not the exclude-prefix — docs and code disagree. Either wire the check into buildTree too, or narrow the README.

    Net: from the reporter's side this looks solid. Thanks for the careful scoping.

  10. JonoGitty commented on May 19, 2026

    @JonoGitty

    Thanks @alemuenchen — sharp catch. Fixed across three commits on the same branch:

    1. 16c959e — extracted containment into a shared assertSearchPathNotExcluded(rootPath, opName) helper used by both searchFilesWithValidation and directory_tree. directory_tree now produces a directory_tree is disabled... error from the same template.
    2. 7409bfd — expandHome applied to rootPath (prior asymmetry meant a ~/foo root wouldn't match a ~/foo prefix). Two extra helper tests: descendant-of-prefix, sibling-with-shared-string-head. 160/160 locally.
    3. b871b4f — README now documents the exclude as a fast lexical prefilter, not a canonical-path exclusion: a symlink whose target lives under an excluded prefix is not blocked and the request would still hit the slow path via validatePath's realpath. Inherited from the existing search check, not regressed by this PR.

    Open design question for maintainers: the symlink bypass is a real performance hole, not just a security one. Hardening options I considered:

    • realpath-before-compare — risky, that's the very call this PR is bounding with timeouts because it can hang
    • Two-stage (lexical first → bounded canonical second) — viable but a real design decision with implications for search_files too

    I'd rather raise this than silently pick a direction. Happy to do the hardening as a follow-up if you'd like a particular shape — or leave it as documented prefilter behaviour, your call.

  11. alemuenchen commented on May 19, 2026

    @alemuenchen
    Author

    @JonoGitty thanks — re-verified the three follow-up commits (16c959e, 7409bfd, b871b4f) on fix/4162-filesystem-cloudstorage-hangs:

    • Build clean, 160/160 tests across 8 files locally.
    • No dependency changes — package.json untouched, so no supply-chain surface to review.
    • The shared assertSearchPathNotExcluded helper reuses isPathWithinAllowedDirectories, so containment stays boundary-aware, and the expandHome-on-rootPath symmetry fix only ever tightens the guard. validatePath and its realpath-based symlink check are untouched.

    On the open design question — my vote is to leave it as the documented lexical prefilter for this PR. The symlink-into-an-excluded-prefix bypass is pre-existing behaviour, not a regression introduced here, and validatePath's realpath still enforces the actual allowed_directories boundary. The only cost of such a symlink is hitting the slow provider path — which is exactly what this PR's timeout + visited-cap now bound. realpath-before-compare would reintroduce the very hang this PR exists to fix; the two-stage (lexical → bounded canonical) approach is the right long-term shape but it's a genuine design decision that also touches search_files, so it belongs in a separate follow-up rather than as scope creep here. I'd suggest opening that follow-up issue so it isn't lost, and otherwise this looks good to me from the reporter side.

  12. JonoGitty commented on May 19, 2026

    @JonoGitty

    Thanks @alemuenchen — appreciate the re-verification and the substantive read on the symlink question. That matches my understanding too: this branch stays focused on the timeout/cap/prefilter fix, and the canonical-exclude question is its own design conversation. Opened the follow-up: #4208.

  13. Kakarottoooo commented on Jun 5, 2026

    @Kakarottoooo

    I can investigate a narrow fix for this if it is not already covered by the fork branch mentioned in #4208.

    To keep the first PR reviewable, I’d avoid adding a broad configuration system. I’m thinking about:

    • adding bounded traversal behavior for search_files;
    • preventing one slow subtree from making the entire server unresponsive;
    • preserving current behavior for normal local paths;
    • adding tests with mocked slow filesystem operations or a bounded traversal fixture.

    Before I start, would maintainers prefer:

    1. returning partial results with warnings, or
    2. returning a bounded error when traversal exceeds the limit/deadline?

    I can start with the least invasive option.

  14. JonoGitty commented on Jun 6, 2026

    @JonoGitty

    @Kakarottoooo thanks for looking into this. The fork branch mentioned in #4208 is now PR #4212, opened 2026-05-20.

    It may cover the same ground: bounded traversal via FS_SEARCH_MAX_VISITED, per-call timeouts via FS_OP_TIMEOUT_MS, defaults intended to preserve current behavior for normal local trees, and tests using mocked slow fs operations.

    On the partial-results-vs-bounded-error question, #4212 currently uses option 2: a bounded error raised as FsSearchTruncatedError, carrying visited and maxVisited. That came out of discussion with @temurkhan13 on the PR. #4208 is tracking the separate symlink-aware-exclude design question.

    If you're interested, review or feedback on #4212 would be very welcome.

  15. Kakarottoooo commented on Jun 10, 2026

    @Kakarottoooo

    Thanks for the pointer — that makes sense. I’ll avoid duplicating #4212.

    I’ll review #4212 locally instead and focus on whether the timeout / max-visited behavior is observable, deterministic in tests, and clear for MCP clients. If I find anything concrete, I’ll leave focused feedback on the PR rather than opening overlapping work.

    If #4212 lands cleanly, I may look at one of the explicitly out-of-scope follow-ups, such as symlink-aware excludes, fs.opendir-based streaming for huge single directories, or a small docs/test improvement.

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

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions