From 7d7366c954c4e07dbb546dac4f077f4989763462 Mon Sep 17 00:00:00 2001 From: Yohanes Date: Fri, 28 Aug 2026 16:19:09 +0800 Subject: [PATCH 1/2] fix(filesystem): reject existing move destinations --- .../__tests__/structured-content.test.ts | 14 ++++++++++++++ src/filesystem/index.ts | 11 +++++++++++ 2 files changed, 25 insertions(+) diff --git a/src/filesystem/__tests__/structured-content.test.ts b/src/filesystem/__tests__/structured-content.test.ts index 67da16f901..c86729be63 100644 --- a/src/filesystem/__tests__/structured-content.test.ts +++ b/src/filesystem/__tests__/structured-content.test.ts @@ -124,6 +124,20 @@ describe('structuredContent schema compliance', () => { // The content should contain success message expect(structuredContent.content).toContain('Successfully moved'); }); + + it('should reject an existing destination without overwriting it', async () => { + const sourcePath = path.join(testDir, 'test.txt'); + const destPath = path.join(testDir, 'existing.txt'); + await fs.writeFile(destPath, 'keep this content'); + + await expect(client.callTool({ + name: 'move_file', + arguments: { source: sourcePath, destination: destPath } + })).rejects.toThrow('Destination already exists'); + + await expect(fs.readFile(sourcePath, 'utf-8')).resolves.toBe('test content'); + await expect(fs.readFile(destPath, 'utf-8')).resolves.toBe('keep this content'); + }); }); describe('list_directory (control - already working)', () => { diff --git a/src/filesystem/index.ts b/src/filesystem/index.ts index 234605bb13..19c73346c4 100644 --- a/src/filesystem/index.ts +++ b/src/filesystem/index.ts @@ -631,6 +631,17 @@ server.registerTool( async (args: z.infer) => { const validSourcePath = await validatePath(args.source); const validDestPath = await validatePath(args.destination); + // fs.rename replaces an existing destination on POSIX (and has platform- + // dependent overwrite semantics elsewhere). The tool contract promises a + // failed move instead, so make the check explicit to prevent data loss. + try { + await fs.lstat(validDestPath); + throw new Error(`Destination already exists: ${args.destination}`); + } catch (error) { + if ((error as NodeJS.ErrnoException).code !== 'ENOENT') { + throw error; + } + } await fs.rename(validSourcePath, validDestPath); const text = `Successfully moved ${args.source} to ${args.destination}`; const contentBlock = { type: "text" as const, text }; From 8592fa2fed029d8b498f2781f5ba171ba495974e Mon Sep 17 00:00:00 2001 From: Yohanes Date: Fri, 28 Aug 2026 16:25:46 +0800 Subject: [PATCH 2/2] test(filesystem): assert encoded move errors --- src/filesystem/__tests__/structured-content.test.ts | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/src/filesystem/__tests__/structured-content.test.ts b/src/filesystem/__tests__/structured-content.test.ts index c86729be63..a4635f0716 100644 --- a/src/filesystem/__tests__/structured-content.test.ts +++ b/src/filesystem/__tests__/structured-content.test.ts @@ -130,10 +130,15 @@ describe('structuredContent schema compliance', () => { const destPath = path.join(testDir, 'existing.txt'); await fs.writeFile(destPath, 'keep this content'); - await expect(client.callTool({ + const result = await client.callTool({ name: 'move_file', arguments: { source: sourcePath, destination: destPath } - })).rejects.toThrow('Destination already exists'); + }); + expect(result.isError).toBe(true); + expect(result.content[0]).toMatchObject({ + type: 'text', + text: expect.stringContaining('Destination already exists'), + }); await expect(fs.readFile(sourcePath, 'utf-8')).resolves.toBe('test content'); await expect(fs.readFile(destPath, 'utf-8')).resolves.toBe('keep this content');