Bill/obsidian-mcp-server
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

fix: use fileManager.trashFile instead of vault.delete

Browse Source 
Replace vault.delete() with fileManager.trashFile() to respect user's
trash preferences configured in Obsidian settings. This ensures deleted
files go to the user's configured trash location instead of being
permanently deleted without respecting system preferences.

Changes:
- src/tools/note-tools.ts: Replace vault.delete with fileManager.trashFile
  in createNote (overwrite conflict) and deleteNote (permanent delete)
- tests/note-tools.test.ts: Update test expectations to check for
  fileManager.trashFile calls instead of vault.delete

Addresses ObsidianReviewBot required issue #3.
This commit is contained in:
Bill Ballou
2025-10-28 19:49:04 -04:00
parent 862ad9d122
commit c9d7aeb0c3
2 changed files with 7 additions and 7 deletions
Download Patch File Download Diff File Expand all files Collapse all files

6
src/tools/note-tools.ts
View File

@@ -159,7 +159,7 @@ export class NoteTools {
const existingFile = PathUtils.resolveFile(this.app, normalizedPath);
/* istanbul ignore next */
if (existingFile) {
await this.vault.delete(existingFile);
await this.fileManager.trashFile(existingFile);
}
} else if (onConflict === 'rename') {
// Generate a unique name
@@ -542,8 +542,8 @@ export class NoteTools {
await this.vault.trash(file, true);
destination = `.trash/${file.name}`;
} else {
// Permanent deletion
await this.vault.delete(file);
// Delete using user's preferred trash settings (system trash or .trash/ folder)
await this.fileManager.trashFile(file);
}
const result: DeleteNoteResult = {

8
tests/note-tools.test.ts
View File

@@ -137,7 +137,7 @@ describe('NoteTools', () => {
(PathUtils.fileExists as jest.Mock).mockReturnValue(true);
(PathUtils.resolveFile as jest.Mock).mockReturnValue(mockFile);
mockVault.delete = jest.fn().mockResolvedValue(undefined);
mockFileManager.trashFile = jest.fn().mockResolvedValue(undefined);
mockVault.create = jest.fn().mockResolvedValue(mockFile);
(PathUtils.folderExists as jest.Mock).mockReturnValue(false);
(PathUtils.getParentPath as jest.Mock).mockReturnValue('');
@@ -145,7 +145,7 @@ describe('NoteTools', () => {
const result = await noteTools.createNote('test.md', 'content', false, 'overwrite');
expect(result.isError).toBeUndefined();
expect(mockVault.delete).toHaveBeenCalledWith(mockFile);
expect(mockFileManager.trashFile).toHaveBeenCalledWith(mockFile);
expect(mockVault.create).toHaveBeenCalled();
});
@@ -344,12 +344,12 @@ describe('NoteTools', () => {
const mockFile = createMockTFile('test.md');
(PathUtils.resolveFile as jest.Mock).mockReturnValue(mockFile);
mockVault.delete = jest.fn().mockResolvedValue(undefined);
mockFileManager.trashFile = jest.fn().mockResolvedValue(undefined);
const result = await noteTools.deleteNote('test.md', false, false);
expect(result.isError).toBeUndefined();
expect(mockVault.delete).toHaveBeenCalledWith(mockFile);
expect(mockFileManager.trashFile).toHaveBeenCalledWith(mockFile);
const parsed = JSON.parse(result.content[0].text);
expect(parsed.deleted).toBe(true);
expect(parsed.soft).toBe(false);