refactor(filesystem): extract buildDirectoryTree to lib.ts

FazeelUsmani · FazeelUsmani · commit acf3c6d2f16a · 2026-04-27T18:26:53.000+05:30
Move the buildTree closure out of the directory_tree handler and
export it as buildDirectoryTree. Replaces the duplicated
buildTreeForTesting in the test suite so tests now exercise the
production function directly.

No behavior change.
diff --git a/src/filesystem/__tests__/directory-tree.test.ts b/src/filesystem/__tests__/directory-tree.test.ts
@@ -2,65 +2,24 @@ import { describe, it, expect, beforeEach, afterEach } from 'vitest';
 import * as fs from 'fs/promises';
 import * as path from 'path';
 import * as os from 'os';
+import { buildDirectoryTree, setAllowedDirectories } from '../lib.js';
 
-// We need to test the buildTree function, but it's defined inside the request handler
-// So we'll extract the core logic into a testable function
-import { minimatch } from 'minimatch';
-
-interface TreeEntry {
-    name: string;
-    type: 'file' | 'directory';
-    children?: TreeEntry[];
-}
-
-async function buildTreeForTesting(currentPath: string, rootPath: string, excludePatterns: string[] = []): Promise<TreeEntry[]> {
-    const entries = await fs.readdir(currentPath, {withFileTypes: true});
-    const result: TreeEntry[] = [];
-
-    for (const entry of entries) {
-        const relativePath = path.relative(rootPath, path.join(currentPath, entry.name));
-        const shouldExclude = excludePatterns.some(pattern => {
-            if (pattern.includes('*')) {
-                return minimatch(relativePath, pattern, {dot: true});
-            }
-            // For files: match exact name or as part of path
-            // For directories: match as directory path
-            return minimatch(relativePath, pattern, {dot: true}) ||
-                   minimatch(relativePath, `**/${pattern}`, {dot: true}) ||
-                   minimatch(relativePath, `**/${pattern}/**`, {dot: true});
-        });
-        if (shouldExclude)
-            continue;
-
-        const entryData: TreeEntry = {
-            name: entry.name,
-            type: entry.isDirectory() ? 'directory' : 'file'
-        };
-
-        if (entry.isDirectory()) {
-            const subPath = path.join(currentPath, entry.name);
-            entryData.children = await buildTreeForTesting(subPath, rootPath, excludePatterns);
-        }
-
-        result.push(entryData);
-    }
-
-    return result;
-}
-
-describe('buildTree exclude patterns', () => {
+describe('buildDirectoryTree exclude patterns', () => {
     let testDir: string;
 
     beforeEach(async () => {
-        testDir = await fs.mkdtemp(path.join(os.tmpdir(), 'filesystem-test-'));
-        
-        // Create test directory structure
+        // On macOS, os.tmpdir() returns /var/folders/... which symlinks to
+        // /private/var/folders/..., and validatePath compares against the
+        // resolved real path. Resolve once up front so allowedDirectories
+        // matches what the recursion sees.
+        testDir = await fs.realpath(await fs.mkdtemp(path.join(os.tmpdir(), 'filesystem-test-')));
+        setAllowedDirectories([testDir]);
+
         await fs.mkdir(path.join(testDir, 'src'));
         await fs.mkdir(path.join(testDir, 'node_modules'));
         await fs.mkdir(path.join(testDir, '.git'));
         await fs.mkdir(path.join(testDir, 'nested', 'node_modules'), { recursive: true });
-        
-        // Create test files
+
         await fs.writeFile(path.join(testDir, '.env'), 'SECRET=value');
         await fs.writeFile(path.join(testDir, '.env.local'), 'LOCAL_SECRET=value');
         await fs.writeFile(path.join(testDir, 'src', 'index.js'), 'console.log("hello");');
@@ -71,62 +30,60 @@ describe('buildTree exclude patterns', () => {
 
     afterEach(async () => {
         await fs.rm(testDir, { recursive: true, force: true });
+        setAllowedDirectories([]);
     });
 
     it('should exclude files matching simple patterns', async () => {
-        // Test the current implementation - this will fail until the bug is fixed
-        const tree = await buildTreeForTesting(testDir, testDir, ['.env']);
+        const tree = await buildDirectoryTree(testDir, ['.env']);
         const fileNames = tree.map(entry => entry.name);
-        
+
         expect(fileNames).not.toContain('.env');
-        expect(fileNames).toContain('.env.local'); // Should not exclude this
+        expect(fileNames).toContain('.env.local');
         expect(fileNames).toContain('src');
         expect(fileNames).toContain('package.json');
     });
 
     it('should exclude directories matching simple patterns', async () => {
-        const tree = await buildTreeForTesting(testDir, testDir, ['node_modules']);
+        const tree = await buildDirectoryTree(testDir, ['node_modules']);
         const dirNames = tree.map(entry => entry.name);
-        
+
         expect(dirNames).not.toContain('node_modules');
         expect(dirNames).toContain('src');
         expect(dirNames).toContain('.git');
     });
 
     it('should exclude nested directories with same pattern', async () => {
-        const tree = await buildTreeForTesting(testDir, testDir, ['node_modules']);
-        
-        // Find the nested directory
+        const tree = await buildDirectoryTree(testDir, ['node_modules']);
+
         const nestedDir = tree.find(entry => entry.name === 'nested');
         expect(nestedDir).toBeDefined();
         expect(nestedDir!.children).toBeDefined();
-        
-        // The nested/node_modules should also be excluded
+
         const nestedChildren = nestedDir!.children!.map(child => child.name);
         expect(nestedChildren).not.toContain('node_modules');
     });
 
     it('should handle glob patterns correctly', async () => {
-        const tree = await buildTreeForTesting(testDir, testDir, ['*.env']);
+        const tree = await buildDirectoryTree(testDir, ['*.env']);
         const fileNames = tree.map(entry => entry.name);
-        
+
         expect(fileNames).not.toContain('.env');
-        expect(fileNames).toContain('.env.local'); // *.env should not match .env.local
+        expect(fileNames).toContain('.env.local');
         expect(fileNames).toContain('src');
     });
 
     it('should handle dot files correctly', async () => {
-        const tree = await buildTreeForTesting(testDir, testDir, ['.git']);
+        const tree = await buildDirectoryTree(testDir, ['.git']);
         const dirNames = tree.map(entry => entry.name);
-        
+
         expect(dirNames).not.toContain('.git');
-        expect(dirNames).toContain('.env'); // Should not exclude this
+        expect(dirNames).toContain('.env');
     });
 
     it('should work with multiple exclude patterns', async () => {
-        const tree = await buildTreeForTesting(testDir, testDir, ['node_modules', '.env', '.git']);
+        const tree = await buildDirectoryTree(testDir, ['node_modules', '.env', '.git']);
         const entryNames = tree.map(entry => entry.name);
-        
+
         expect(entryNames).not.toContain('node_modules');
         expect(entryNames).not.toContain('.env');
         expect(entryNames).not.toContain('.git');
@@ -135,13 +92,12 @@ describe('buildTree exclude patterns', () => {
     });
 
     it('should handle empty exclude patterns', async () => {
-        const tree = await buildTreeForTesting(testDir, testDir, []);
+        const tree = await buildDirectoryTree(testDir, []);
         const entryNames = tree.map(entry => entry.name);
-        
-        // All entries should be included
+
         expect(entryNames).toContain('node_modules');
         expect(entryNames).toContain('.env');
         expect(entryNames).toContain('.git');
         expect(entryNames).toContain('src');
     });
-});
+});
diff --git a/src/filesystem/index.ts b/src/filesystem/index.ts
@@ -11,7 +11,6 @@ import fs from "fs/promises";
 import { createReadStream } from "fs";
 import path from "path";
 import { z } from "zod";
-import { minimatch } from "minimatch";
 import { normalizePath, expandHome } from './path-utils.js';
 import { getValidRootDirectories } from './roots-utils.js';
 import {
@@ -26,6 +25,7 @@ import {
   tailFile,
   headFile,
   setAllowedDirectories,
+  buildDirectoryTree,
 } from './lib.js';
 
 // Command line argument parsing
@@ -541,50 +541,7 @@ server.registerTool(
     annotations: { readOnlyHint: true }
   },
   async (args: z.infer<typeof DirectoryTreeArgsSchema>) => {
-    interface TreeEntry {
-      name: string;
-      type: 'file' | 'directory';
-      children?: TreeEntry[];
-    }
-    const rootPath = args.path;
-
-    async function buildTree(currentPath: string, excludePatterns: string[] = []): Promise<TreeEntry[]> {
-      const validPath = await validatePath(currentPath);
-      const entries = await fs.readdir(validPath, { withFileTypes: true });
-      const result: TreeEntry[] = [];
-
-      for (const entry of entries) {
-        const relativePath = path.relative(rootPath, path.join(currentPath, entry.name));
-        const shouldExclude = excludePatterns.some(pattern => {
-          if (pattern.includes('*')) {
-            return minimatch(relativePath, pattern, { dot: true });
-          }
-          // For files: match exact name or as part of path
-          // For directories: match as directory path
-          return minimatch(relativePath, pattern, { dot: true }) ||
-            minimatch(relativePath, `**/${pattern}`, { dot: true }) ||
-            minimatch(relativePath, `**/${pattern}/**`, { dot: true });
-        });
-        if (shouldExclude)
-          continue;
-
-        const entryData: TreeEntry = {
-          name: entry.name,
-          type: entry.isDirectory() ? 'directory' : 'file'
-        };
-
-        if (entry.isDirectory()) {
-          const subPath = path.join(currentPath, entry.name);
-          entryData.children = await buildTree(subPath, excludePatterns);
-        }
-
-        result.push(entryData);
-      }
-
-      return result;
-    }
-
-    const treeData = await buildTree(rootPath, args.excludePatterns);
+    const treeData = await buildDirectoryTree(args.path, args.excludePatterns);
     const text = JSON.stringify(treeData, null, 2);
     const contentBlock = { type: "text" as const, text };
     return {
diff --git a/src/filesystem/lib.ts b/src/filesystem/lib.ts
@@ -413,3 +413,50 @@ export async function searchFilesWithValidation(
   await search(rootPath);
   return results;
 }
+
+interface DirectoryTreeEntry {
+  name: string;
+  type: 'file' | 'directory';
+  children?: DirectoryTreeEntry[];
+}
+
+export async function buildDirectoryTree(
+  rootPath: string,
+  excludePatterns: string[] = []
+): Promise<DirectoryTreeEntry[]> {
+  async function walk(currentPath: string): Promise<DirectoryTreeEntry[]> {
+    const validPath = await validatePath(currentPath);
+    const entries = await fs.readdir(validPath, { withFileTypes: true });
+    const result: DirectoryTreeEntry[] = [];
+
+    for (const entry of entries) {
+      const relativePath = path.relative(rootPath, path.join(currentPath, entry.name));
+      const shouldExclude = excludePatterns.some(pattern => {
+        if (pattern.includes('*')) {
+          return minimatch(relativePath, pattern, { dot: true });
+        }
+        // For files: match exact name or as part of path
+        // For directories: match as directory path
+        return minimatch(relativePath, pattern, { dot: true }) ||
+          minimatch(relativePath, `**/${pattern}`, { dot: true }) ||
+          minimatch(relativePath, `**/${pattern}/**`, { dot: true });
+      });
+      if (shouldExclude) continue;
+
+      const entryData: DirectoryTreeEntry = {
+        name: entry.name,
+        type: entry.isDirectory() ? 'directory' : 'file',
+      };
+
+      if (entry.isDirectory()) {
+        entryData.children = await walk(path.join(currentPath, entry.name));
+      }
+
+      result.push(entryData);
+    }
+
+    return result;
+  }
+
+  return walk(rootPath);
+}
