diff --git a/src/tools/filesystem.ts b/src/tools/filesystem.ts index 7aa40719..44202aa5 100644 --- a/src/tools/filesystem.ts +++ b/src/tools/filesystem.ts @@ -676,6 +676,32 @@ export async function listDirectory(dirPath: string, depth: number = 2): Promise const results: string[] = []; const MAX_NESTED_ITEMS = 100; // Maximum items to show per nested directory + const isAccessDeniedError = (err: NodeJS.ErrnoException) => + err.code === 'EPERM' || err.code === 'EACCES' || err.code === 'ETIMEDOUT'; + + const getDisplayPath = (targetPath: string): string => path.basename(targetPath) || targetPath; + + function addDeniedEntry(displayPath: string): void { + // Keep the payload path-only so the UI can parse denied entries reliably. + results.push(`[DENIED] ${displayPath}`); + } + + try { + const stats = await fs.stat(validPath); + if (!stats.isDirectory()) { + throw new Error(`Directory not found: ${dirPath}`); + } + } catch (error) { + const err = error as NodeJS.ErrnoException; + if (err.code === 'ENOENT' || err.code === 'ENOTDIR') { + throw new Error(`Directory not found: ${dirPath}`); + } + if (isAccessDeniedError(err)) { + addDeniedEntry(getDisplayPath(validPath)); + return results; + } + throw error; + } async function listRecursive(currentPath: string, currentDepth: number, relativePath: string = '', isTopLevel: boolean = true): Promise { if (currentDepth <= 0) return; @@ -685,14 +711,11 @@ export async function listDirectory(dirPath: string, depth: number = 2): Promise entries = await fs.readdir(currentPath, { withFileTypes: true }); } catch (error) { const err = error as NodeJS.ErrnoException; - const displayPath = relativePath || path.basename(currentPath); - // Keep [DENIED] prefix so UI parser regex still matches. - // Append a hint for permission/timeout errors so user gets context. - if (err.code === 'EPERM' || err.code === 'EACCES' || err.code === 'ETIMEDOUT') { - results.push(`[DENIED] ${displayPath} — not accessible (permission denied, cloud-only file, or Full Disk Access not granted)`); - } else { - results.push(`[DENIED] ${displayPath}`); + if (isTopLevel && (err.code === 'ENOENT' || err.code === 'ENOTDIR')) { + throw new Error(`Directory not found: ${dirPath}`); } + const displayPath = relativePath || getDisplayPath(currentPath); + addDeniedEntry(displayPath); return; } @@ -1010,4 +1033,4 @@ export async function writePdf( } else { throw new Error('Invalid content type for writePdf. Expected string (markdown) or array of operations.'); } -} \ No newline at end of file +} diff --git a/test/test-list-directory-errors.js b/test/test-list-directory-errors.js new file mode 100644 index 00000000..b6b972f8 --- /dev/null +++ b/test/test-list-directory-errors.js @@ -0,0 +1,106 @@ +import assert from 'assert'; +import fs from 'fs/promises'; +import os from 'os'; +import path from 'path'; +import { pathToFileURL } from 'url'; + +import { configManager } from '../dist/config-manager.js'; +import { listDirectory } from '../dist/tools/filesystem.js'; + +const TEST_ROOT_PREFIX = path.join(os.tmpdir(), 'desktop-commander-list-directory-errors-'); + +async function setup() { + const originalConfig = await configManager.getConfig(); + const testRoot = await fs.realpath(await fs.mkdtemp(TEST_ROOT_PREFIX)); + await configManager.setValue('allowedDirectories', [testRoot]); + return { originalConfig, testRoot }; +} + +async function teardown(originalConfig, testRoot) { + await configManager.updateConfig(originalConfig); + await fs.rm(testRoot, { recursive: true, force: true }); +} + +async function testMissingDirectoryReturnsNotFound(testRoot) { + const missingDir = path.join(testRoot, 'missing-dir'); + + await assert.rejects( + listDirectory(missingDir, 1), + (error) => { + assert(error instanceof Error); + assert(error.message.includes(`Directory not found: ${missingDir}`)); + assert(!error.message.includes('[DENIED]')); + return true; + }, + 'Missing directories should raise a not-found error instead of returning [DENIED]', + ); + + console.log('✓ missing directory returns not-found error'); +} + +async function testTopLevelStatAccessDeniedReturnsDeniedEntry(testRoot) { + const deniedDir = path.join(testRoot, 'stat-denied'); + await fs.mkdir(deniedDir); + + const originalStat = fs.stat; + fs.stat = async (target, ...args) => { + if (path.resolve(String(target)) === deniedDir) { + const error = new Error(`EACCES: permission denied, stat '${deniedDir}'`); + error.code = 'EACCES'; + throw error; + } + return originalStat.call(fs, target, ...args); + }; + + try { + const entries = await listDirectory(deniedDir, 1); + assert.deepStrictEqual(entries, ['[DENIED] stat-denied']); + console.log('✓ top-level stat access error returns denied entry'); + } finally { + fs.stat = originalStat; + } +} + +async function testFilePathReturnsNotFound(testRoot) { + const filePath = path.join(testRoot, 'not-a-directory.txt'); + await fs.writeFile(filePath, 'not a directory'); + + await assert.rejects( + listDirectory(filePath, 1), + (error) => { + assert(error instanceof Error); + assert(error.message.includes(`Directory not found: ${filePath}`)); + assert(!error.message.includes('Path is not a directory')); + return true; + }, + 'File paths should use the same not-found contract as missing directories', + ); + + console.log('✓ file path returns not-found error'); +} + +export default async function runTests() { + let originalConfig; + let testRoot; + try { + ({ originalConfig, testRoot } = await setup()); + await testMissingDirectoryReturnsNotFound(testRoot); + await testTopLevelStatAccessDeniedReturnsDeniedEntry(testRoot); + await testFilePathReturnsNotFound(testRoot); + console.log('\n✅ list_directory error tests passed!'); + return true; + } catch (error) { + console.error('❌ list_directory error test failed:', error instanceof Error ? error.message : String(error)); + return false; + } finally { + if (originalConfig && testRoot) { + await teardown(originalConfig, testRoot); + } + } +} + +if (process.argv[1] && import.meta.url === pathToFileURL(process.argv[1]).href) { + runTests().then((ok) => { + process.exit(ok ? 0 : 1); + }); +}