From a62bc81616dac51df6fd71425803237e1c871bf2 Mon Sep 17 00:00:00 2001 From: showiix <2138757206@qq.com> Date: Wed, 29 Apr 2026 09:34:05 +0800 Subject: [PATCH 1/4] fix: return not-found for missing directories --- src/tools/filesystem.ts | 18 ++++++++- test/test-list-directory-errors.js | 61 ++++++++++++++++++++++++++++++ 2 files changed, 78 insertions(+), 1 deletion(-) create mode 100644 test/test-list-directory-errors.js diff --git a/src/tools/filesystem.ts b/src/tools/filesystem.ts index 7aa40719..719ccd1e 100644 --- a/src/tools/filesystem.ts +++ b/src/tools/filesystem.ts @@ -677,6 +677,19 @@ export async function listDirectory(dirPath: string, depth: number = 2): Promise const MAX_NESTED_ITEMS = 100; // Maximum items to show per nested directory + try { + const stats = await fs.stat(validPath); + if (!stats.isDirectory()) { + throw new Error(`Path is not a directory: ${dirPath}`); + } + } catch (error) { + const err = error as NodeJS.ErrnoException; + if (err.code === 'ENOENT' || err.code === 'ENOTDIR') { + throw new Error(`Directory not found: ${dirPath}`); + } + throw error; + } + async function listRecursive(currentPath: string, currentDepth: number, relativePath: string = '', isTopLevel: boolean = true): Promise { if (currentDepth <= 0) return; @@ -685,6 +698,9 @@ 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; + if (isTopLevel && (err.code === 'ENOENT' || err.code === 'ENOTDIR')) { + throw new Error(`Directory not found: ${dirPath}`); + } 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. @@ -1010,4 +1026,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..61f2b7ae --- /dev/null +++ b/test/test-list-directory-errors.js @@ -0,0 +1,61 @@ +import assert from 'assert'; +import fs from 'fs/promises'; +import os from 'os'; +import path from 'path'; + +import { configManager } from '../dist/config-manager.js'; +import { listDirectory } from '../dist/tools/filesystem.js'; + +const TEST_ROOT = path.join(os.tmpdir(), 'desktop-commander-list-directory-errors'); + +async function setup() { + const originalConfig = await configManager.getConfig(); + await fs.mkdir(TEST_ROOT, { recursive: true }); + await configManager.setValue('allowedDirectories', [TEST_ROOT]); + return originalConfig; +} + +async function teardown(originalConfig) { + await configManager.updateConfig(originalConfig); + await fs.rm(TEST_ROOT, { recursive: true, force: true }); +} + +async function testMissingDirectoryReturnsNotFound() { + const missingDir = path.join(TEST_ROOT, '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'); +} + +export default async function runTests() { + let originalConfig; + try { + originalConfig = await setup(); + await testMissingDirectoryReturnsNotFound(); + 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) { + await teardown(originalConfig); + } + } +} + +if (import.meta.url === `file://${process.argv[1]}`) { + runTests().then((ok) => { + process.exit(ok ? 0 : 1); + }); +} From 56f6b1d147890da2989321aa7b40cfed5dcdfab4 Mon Sep 17 00:00:00 2001 From: showiix <2138757206@qq.com> Date: Wed, 29 Apr 2026 09:44:30 +0800 Subject: [PATCH 2/4] test: isolate list directory temp path --- test/test-list-directory-errors.js | 25 +++++++++++++------------ 1 file changed, 13 insertions(+), 12 deletions(-) diff --git a/test/test-list-directory-errors.js b/test/test-list-directory-errors.js index 61f2b7ae..e171a578 100644 --- a/test/test-list-directory-errors.js +++ b/test/test-list-directory-errors.js @@ -6,22 +6,22 @@ import path from 'path'; import { configManager } from '../dist/config-manager.js'; import { listDirectory } from '../dist/tools/filesystem.js'; -const TEST_ROOT = path.join(os.tmpdir(), 'desktop-commander-list-directory-errors'); +const TEST_ROOT_PREFIX = path.join(os.tmpdir(), 'desktop-commander-list-directory-errors-'); async function setup() { const originalConfig = await configManager.getConfig(); - await fs.mkdir(TEST_ROOT, { recursive: true }); - await configManager.setValue('allowedDirectories', [TEST_ROOT]); - return originalConfig; + const testRoot = await fs.mkdtemp(TEST_ROOT_PREFIX); + await configManager.setValue('allowedDirectories', [testRoot]); + return { originalConfig, testRoot }; } -async function teardown(originalConfig) { +async function teardown(originalConfig, testRoot) { await configManager.updateConfig(originalConfig); - await fs.rm(TEST_ROOT, { recursive: true, force: true }); + await fs.rm(testRoot, { recursive: true, force: true }); } -async function testMissingDirectoryReturnsNotFound() { - const missingDir = path.join(TEST_ROOT, 'missing-dir'); +async function testMissingDirectoryReturnsNotFound(testRoot) { + const missingDir = path.join(testRoot, 'missing-dir'); await assert.rejects( listDirectory(missingDir, 1), @@ -39,17 +39,18 @@ async function testMissingDirectoryReturnsNotFound() { export default async function runTests() { let originalConfig; + let testRoot; try { - originalConfig = await setup(); - await testMissingDirectoryReturnsNotFound(); + ({ originalConfig, testRoot } = await setup()); + await testMissingDirectoryReturnsNotFound(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) { - await teardown(originalConfig); + if (originalConfig && testRoot) { + await teardown(originalConfig, testRoot); } } } From 3f1346723e4d14b947858704c3f6757362234b09 Mon Sep 17 00:00:00 2001 From: showiix <2138757206@qq.com> Date: Wed, 29 Apr 2026 09:50:29 +0800 Subject: [PATCH 3/4] fix: preserve denied output for stat access errors --- src/tools/filesystem.ts | 24 +++++++++++++++++------- test/test-list-directory-errors.js | 28 +++++++++++++++++++++++++++- 2 files changed, 44 insertions(+), 8 deletions(-) diff --git a/src/tools/filesystem.ts b/src/tools/filesystem.ts index 719ccd1e..8b7e1031 100644 --- a/src/tools/filesystem.ts +++ b/src/tools/filesystem.ts @@ -676,6 +676,18 @@ 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'; + + function addDeniedEntry(displayPath: string, err: NodeJS.ErrnoException): void { + // Keep [DENIED] prefix so UI parser regex still matches. + // Append a hint for permission/timeout errors so user gets context. + if (isAccessDeniedError(err)) { + results.push(`[DENIED] ${displayPath} — not accessible (permission denied, cloud-only file, or Full Disk Access not granted)`); + } else { + results.push(`[DENIED] ${displayPath}`); + } + } try { const stats = await fs.stat(validPath); @@ -687,6 +699,10 @@ export async function listDirectory(dirPath: string, depth: number = 2): Promise if (err.code === 'ENOENT' || err.code === 'ENOTDIR') { throw new Error(`Directory not found: ${dirPath}`); } + if (isAccessDeniedError(err)) { + addDeniedEntry(path.basename(validPath), err); + return results; + } throw error; } @@ -702,13 +718,7 @@ export async function listDirectory(dirPath: string, depth: number = 2): Promise throw new Error(`Directory not found: ${dirPath}`); } 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}`); - } + addDeniedEntry(displayPath, err); return; } diff --git a/test/test-list-directory-errors.js b/test/test-list-directory-errors.js index e171a578..773dcd1a 100644 --- a/test/test-list-directory-errors.js +++ b/test/test-list-directory-errors.js @@ -10,7 +10,7 @@ const TEST_ROOT_PREFIX = path.join(os.tmpdir(), 'desktop-commander-list-director async function setup() { const originalConfig = await configManager.getConfig(); - const testRoot = await fs.mkdtemp(TEST_ROOT_PREFIX); + const testRoot = await fs.realpath(await fs.mkdtemp(TEST_ROOT_PREFIX)); await configManager.setValue('allowedDirectories', [testRoot]); return { originalConfig, testRoot }; } @@ -37,12 +37,38 @@ async function testMissingDirectoryReturnsNotFound(testRoot) { 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.strictEqual(entries.length, 1); + assert(entries[0].startsWith('[DENIED] stat-denied'), 'Top-level stat access errors should keep [DENIED] output'); + assert(entries[0].includes('not accessible'), 'Permission-like stat errors should include the access hint'); + console.log('✓ top-level stat access error returns denied entry'); + } finally { + fs.stat = originalStat; + } +} + export default async function runTests() { let originalConfig; let testRoot; try { ({ originalConfig, testRoot } = await setup()); await testMissingDirectoryReturnsNotFound(testRoot); + await testTopLevelStatAccessDeniedReturnsDeniedEntry(testRoot); console.log('\n✅ list_directory error tests passed!'); return true; } catch (error) { From 7ee513d9fb97358055444434c4b5b05695dbce1e Mon Sep 17 00:00:00 2001 From: showiix <2138757206@qq.com> Date: Wed, 29 Apr 2026 10:02:11 +0800 Subject: [PATCH 4/4] fix: keep denied directory entries parseable --- src/tools/filesystem.ts | 21 +++++++++------------ test/test-list-directory-errors.js | 26 ++++++++++++++++++++++---- 2 files changed, 31 insertions(+), 16 deletions(-) diff --git a/src/tools/filesystem.ts b/src/tools/filesystem.ts index 8b7e1031..44202aa5 100644 --- a/src/tools/filesystem.ts +++ b/src/tools/filesystem.ts @@ -679,20 +679,17 @@ export async function listDirectory(dirPath: string, depth: number = 2): Promise const isAccessDeniedError = (err: NodeJS.ErrnoException) => err.code === 'EPERM' || err.code === 'EACCES' || err.code === 'ETIMEDOUT'; - function addDeniedEntry(displayPath: string, err: NodeJS.ErrnoException): void { - // Keep [DENIED] prefix so UI parser regex still matches. - // Append a hint for permission/timeout errors so user gets context. - if (isAccessDeniedError(err)) { - results.push(`[DENIED] ${displayPath} — not accessible (permission denied, cloud-only file, or Full Disk Access not granted)`); - } else { - results.push(`[DENIED] ${displayPath}`); - } + 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(`Path is not a directory: ${dirPath}`); + throw new Error(`Directory not found: ${dirPath}`); } } catch (error) { const err = error as NodeJS.ErrnoException; @@ -700,7 +697,7 @@ export async function listDirectory(dirPath: string, depth: number = 2): Promise throw new Error(`Directory not found: ${dirPath}`); } if (isAccessDeniedError(err)) { - addDeniedEntry(path.basename(validPath), err); + addDeniedEntry(getDisplayPath(validPath)); return results; } throw error; @@ -717,8 +714,8 @@ export async function listDirectory(dirPath: string, depth: number = 2): Promise if (isTopLevel && (err.code === 'ENOENT' || err.code === 'ENOTDIR')) { throw new Error(`Directory not found: ${dirPath}`); } - const displayPath = relativePath || path.basename(currentPath); - addDeniedEntry(displayPath, err); + const displayPath = relativePath || getDisplayPath(currentPath); + addDeniedEntry(displayPath); return; } diff --git a/test/test-list-directory-errors.js b/test/test-list-directory-errors.js index 773dcd1a..b6b972f8 100644 --- a/test/test-list-directory-errors.js +++ b/test/test-list-directory-errors.js @@ -2,6 +2,7 @@ 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'; @@ -53,15 +54,31 @@ async function testTopLevelStatAccessDeniedReturnsDeniedEntry(testRoot) { try { const entries = await listDirectory(deniedDir, 1); - assert.strictEqual(entries.length, 1); - assert(entries[0].startsWith('[DENIED] stat-denied'), 'Top-level stat access errors should keep [DENIED] output'); - assert(entries[0].includes('not accessible'), 'Permission-like stat errors should include the access hint'); + 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; @@ -69,6 +86,7 @@ export default async function runTests() { ({ 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) { @@ -81,7 +99,7 @@ export default async function runTests() { } } -if (import.meta.url === `file://${process.argv[1]}`) { +if (process.argv[1] && import.meta.url === pathToFileURL(process.argv[1]).href) { runTests().then((ok) => { process.exit(ok ? 0 : 1); });