Merge pull request #622 from yamadashy/fix/is-not-a-dir

Improve user authentication flow

Author: yamadashy

src/core/file/fileSearch.ts

@@ -1,3 +1,4 @@
+import type { Stats } from 'node:fs';
 import fs from 'node:fs/promises';
 import path from 'node:path';
 import { globby } from 'globby';
@@ -92,7 +93,42 @@ export const normalizeGlobPattern = (pattern: string): string => {
 
 // Get all file paths considering the config
 export const searchFiles = async (rootDir: string, config: RepomixConfigMerged): Promise<FileSearchResult> => {
-  // First check directory permissions
+  // Check if the path exists and get its type
+  let pathStats: Stats;
+  try {
+    pathStats = await fs.stat(rootDir);
+  } catch (error) {
+    if (error instanceof Error && 'code' in error) {
+      const errorCode = (error as NodeJS.ErrnoException).code;
+      if (errorCode === 'ENOENT') {
+        throw new RepomixError(`Target path does not exist: ${rootDir}`);
+      }
+      if (errorCode === 'EPERM' || errorCode === 'EACCES') {
+        throw new PermissionError(
+          `Permission denied while accessing path. Please check folder access permissions for your terminal app. path: ${rootDir}`,
+          rootDir,
+          errorCode,
+        );
+      }
+      // Handle other specific error codes with more context
+      throw new RepomixError(`Failed to access path: ${rootDir}. Error code: ${errorCode}. ${error.message}`);
+    }
+    // Preserve original error stack trace for debugging
+    const repomixError = new RepomixError(
+      `Failed to access path: ${rootDir}. Reason: ${error instanceof Error ? error.message : JSON.stringify(error)}`,
+    );
+    repomixError.cause = error;
+    throw repomixError;
+  }
+
+  // Check if the path is a directory
+  if (!pathStats.isDirectory()) {
+    throw new RepomixError(
+      `Target path is not a directory: ${rootDir}. Please specify a directory path, not a file path.`,
+    );
+  }
+
+  // Now check directory permissions
   const permissionCheck = await checkDirectoryPermissions(rootDir);
 
   if (permissionCheck.details?.read !== true) {

tests/core/file/fileSearch.test.ts

@@ -13,14 +13,41 @@ import {
   parseIgnoreContent,
   searchFiles,
 } from '../../../src/core/file/fileSearch.js';
+import { PermissionError } from '../../../src/core/file/permissionCheck.js';
+import { RepomixError } from '../../../src/shared/errorHandle.js';
 import { createMockConfig, isWindows } from '../../testing/testUtils.js';
 
+import { checkDirectoryPermissions } from '../../../src/core/file/permissionCheck.js';
+
 vi.mock('fs/promises');
 vi.mock('globby');
+vi.mock('../../../src/core/file/permissionCheck.js', () => ({
+  checkDirectoryPermissions: vi.fn(),
+  PermissionError: class extends Error {
+    constructor(
+      message: string,
+      public readonly path: string,
+      public readonly code?: string,
+    ) {
+      super(message);
+      this.name = 'PermissionError';
+    }
+  },
+}));
 
 describe('fileSearch', () => {
   beforeEach(() => {
     vi.resetAllMocks();
+    // Default mock for fs.stat to assume directory exists and is a directory
+    vi.mocked(fs.stat).mockResolvedValue({
+      isDirectory: () => true,
+      isFile: () => false,
+    } as Stats);
+    // Default mock for checkDirectoryPermissions
+    vi.mocked(checkDirectoryPermissions).mockResolvedValue({
+      hasAllPermission: true,
+      details: { read: true, write: true, execute: true },
+    });
   });
 
   describe('getIgnoreFilePaths', () => {
@@ -204,6 +231,15 @@ node_modules
   describe('filterFiles', () => {
     beforeEach(() => {
       vi.resetAllMocks();
+      // Re-establish default mocks after reset
+      vi.mocked(fs.stat).mockResolvedValue({
+        isDirectory: () => true,
+        isFile: () => false,
+      } as Stats);
+      vi.mocked(checkDirectoryPermissions).mockResolvedValue({
+        hasAllPermission: true,
+        details: { read: true, write: true, execute: true },
+      });
     });
 
     test('should call globby with correct parameters', async () => {
@@ -311,12 +347,24 @@ node_modules
       // Mock .git file content for worktree
       const gitWorktreeContent = 'gitdir: /path/to/main/repo/.git/worktrees/feature-branch';
 
-      // Mock fs.stat and fs.readFile for .git file
-      vi.mocked(fs.stat).mockResolvedValue({
-        isFile: () => true,
-      } as Stats);
+      // Mock fs.stat - first call for rootDir, subsequent calls for .git file
+      vi.mocked(fs.stat)
+        .mockResolvedValueOnce({
+          isDirectory: () => true,
+          isFile: () => false,
+        } as Stats)
+        .mockResolvedValue({
+          isFile: () => true,
+          isDirectory: () => false,
+        } as Stats);
       vi.mocked(fs.readFile).mockResolvedValue(gitWorktreeContent);
 
+      // Override checkDirectoryPermissions mock for this test
+      vi.mocked(checkDirectoryPermissions).mockResolvedValue({
+        hasAllPermission: true,
+        details: { read: true, write: true, execute: true },
+      });
+
       // Mock globby to return some test files
       vi.mocked(globby).mockResolvedValue(['file1.js', 'file2.js']);
 
@@ -345,9 +393,21 @@ node_modules
 
     test('should handle regular git repository correctly', async () => {
       // Mock .git as a directory
-      vi.mocked(fs.stat).mockResolvedValue({
-        isFile: () => false,
-      } as Stats);
+      vi.mocked(fs.stat)
+        .mockResolvedValueOnce({
+          isDirectory: () => true,
+          isFile: () => false,
+        } as Stats)
+        .mockResolvedValue({
+          isFile: () => false,
+          isDirectory: () => true,
+        } as Stats);
+
+      // Override checkDirectoryPermissions mock for this test
+      vi.mocked(checkDirectoryPermissions).mockResolvedValue({
+        hasAllPermission: true,
+        details: { read: true, write: true, execute: true },
+      });
 
       // Mock globby to return some test files
       vi.mocked(globby).mockResolvedValue(['file1.js', 'file2.js']);
@@ -453,4 +513,54 @@ node_modules
       expect(normalizeGlobPattern('**/folder/**/*')).toBe('**/folder/**/*');
     });
   });
+
+  describe('searchFiles path validation', () => {
+    test('should throw error when target path does not exist', async () => {
+      const error = new Error('ENOENT') as Error & { code: string };
+      error.code = 'ENOENT';
+      vi.mocked(fs.stat).mockRejectedValue(error);
+
+      const mockConfig = createMockConfig();
+
+      await expect(searchFiles('/nonexistent/path', mockConfig)).rejects.toThrow(RepomixError);
+      await expect(searchFiles('/nonexistent/path', mockConfig)).rejects.toThrow(
+        'Target path does not exist: /nonexistent/path',
+      );
+    });
+
+    test('should throw PermissionError when access is denied', async () => {
+      const error = new Error('EPERM') as Error & { code: string };
+      error.code = 'EPERM';
+      vi.mocked(fs.stat).mockRejectedValue(error);
+
+      const mockConfig = createMockConfig();
+
+      await expect(searchFiles('/forbidden/path', mockConfig)).rejects.toThrow(PermissionError);
+    });
+
+    test('should throw error when target path is a file, not a directory', async () => {
+      vi.mocked(fs.stat).mockResolvedValue({
+        isDirectory: () => false,
+        isFile: () => true,
+      } as Stats);
+
+      const mockConfig = createMockConfig();
+
+      await expect(searchFiles('/path/to/file.txt', mockConfig)).rejects.toThrow(RepomixError);
+      await expect(searchFiles('/path/to/file.txt', mockConfig)).rejects.toThrow(
+        'Target path is not a directory: /path/to/file.txt. Please specify a directory path, not a file path.',
+      );
+    });
+
+    test('should succeed when target path is a valid directory', async () => {
+      vi.mocked(globby).mockResolvedValue(['test.js']);
+
+      const mockConfig = createMockConfig();
+
+      const result = await searchFiles('/valid/directory', mockConfig);
+
+      expect(result.filePaths).toEqual(['test.js']);
+      expect(result.emptyDirPaths).toEqual([]);
+    });
+  });
 });