From 4d8edc8b96a89c6be0b3feffa8673ef66e3f02dc Mon Sep 17 00:00:00 2001 From: Andrew Beal Date: Mon, 3 Nov 2025 21:37:47 +0000 Subject: [PATCH] Update delete functionality to support folders and use trash - Enhance DeleteVaultFiles to handle both files and folders - Change VaultService.delete to use vault.trash instead of permanent deletion - Remove TFile type check to allow folder deletion - Simplify delete method signature by removing unused force parameter - Increase default snippet size limit from 150 to 300 characters - Fix IPrompt to use SettingsService instead of direct plugin access - Update all tests to reflect trash behavior and folder support --- .../Functions/DeleteVaultFiles.ts | 6 +++--- AIClasses/IPrompt.ts | 5 ++++- Services/FileSystemService.ts | 4 ++-- Services/SettingsService.ts | 2 +- Services/VaultService.ts | 4 ++-- __tests__/Services/FileSystemService.test.ts | 11 +++++----- __tests__/Services/SettingsService.test.ts | 18 ++++++++-------- __tests__/Services/VaultService.test.ts | 21 ++++++++++--------- 8 files changed, 38 insertions(+), 33 deletions(-) diff --git a/AIClasses/FunctionDefinitions/Functions/DeleteVaultFiles.ts b/AIClasses/FunctionDefinitions/Functions/DeleteVaultFiles.ts index 53d1697..90a5acb 100644 --- a/AIClasses/FunctionDefinitions/Functions/DeleteVaultFiles.ts +++ b/AIClasses/FunctionDefinitions/Functions/DeleteVaultFiles.ts @@ -3,8 +3,8 @@ import type { IAIFunctionDefinition } from "../IAIFunctionDefinition"; export const DeleteVaultFiles: IAIFunctionDefinition = { name: AIFunction.DeleteVaultFiles, - description: `Permanently removes files from the vault. Use this when the user explicitly - requests to delete file(s), when a file is no longer needed, or when removing outdated + description: `Permanently removes files and folders from the vault. Use this when the user explicitly + requests to delete file(s) or folder(s), when a file or folder is no longer needed, or when removing outdated content. IMPORTANT: This action is irreversible - always confirm the exact file path(s) before deletion. Prefer archiving or moving files to a trash folder over permanent deletion when uncertain. Only call this after verifying the file(s) exist and confirming @@ -17,7 +17,7 @@ export const DeleteVaultFiles: IAIFunctionDefinition = { items: { type: "string" }, - description: "An array of file paths to be deleted (e.g., ['folder/note.md', 'note2.md']). Must be an exact match to existing files." + description: "An array of file paths to be deleted (e.g., ['folder/note.md', 'note2.md', 'folder/subfolder']). Must be an exact match to existing files or folders." }, user_message: { type: "string", diff --git a/AIClasses/IPrompt.ts b/AIClasses/IPrompt.ts index 8cee851..5fc8cc1 100644 --- a/AIClasses/IPrompt.ts +++ b/AIClasses/IPrompt.ts @@ -3,6 +3,7 @@ import { Resolve } from "Services/DependencyService"; import { Services } from "Services/Services"; import { SystemInstruction } from "./SystemPrompt"; import type { FileSystemService } from "Services/FileSystemService"; +import type { SettingsService } from "Services/SettingsService"; export interface IPrompt { systemInstruction(): string; @@ -12,10 +13,12 @@ export interface IPrompt { export class AIPrompt implements IPrompt { private readonly plugin: AIAgentPlugin; + private readonly settingsService: SettingsService; private readonly fileSystemService: FileSystemService; public constructor() { this.plugin = Resolve(Services.AIAgentPlugin); + this.settingsService = Resolve(Services.SettingsService); this.fileSystemService = Resolve(Services.FileSystemService); } @@ -24,7 +27,7 @@ export class AIPrompt implements IPrompt { } public async userInstruction(): Promise { - const userInstruction: string | null = await this.fileSystemService.readFile(this.plugin.settings.userInstruction, true); + const userInstruction: string | null = await this.fileSystemService.readFile(this.settingsService.settings.userInstruction, true); return userInstruction ?? ""; } } \ No newline at end of file diff --git a/Services/FileSystemService.ts b/Services/FileSystemService.ts index f708441..b7bc9df 100644 --- a/Services/FileSystemService.ts +++ b/Services/FileSystemService.ts @@ -48,11 +48,11 @@ export class FileSystemService { public async deleteFile(filePath: string, allowAccessToPluginRoot: boolean = false): Promise<{ success: true } | { success: false, error: string }> { const file: TAbstractFile | null = this.vaultService.getAbstractFileByPath(filePath, allowAccessToPluginRoot); - if (!file || !(file instanceof TFile)) { + if (!file) { return { success: false, error: "File not found" }; } - return await this.vaultService.delete(file, undefined, allowAccessToPluginRoot); + return await this.vaultService.delete(file, allowAccessToPluginRoot); } public async moveFile(sourcePath: string, destinationPath: string, allowAccessToPluginRoot: boolean = false): Promise<{ success: true } | { success: false, error: string }> { diff --git a/Services/SettingsService.ts b/Services/SettingsService.ts index 96972ad..622ada3 100644 --- a/Services/SettingsService.ts +++ b/Services/SettingsService.ts @@ -16,7 +16,7 @@ const DEFAULT_SETTINGS: IAIAgentSettings = { exclusions: [], searchResultsLimit: 15, - snippetSizeLimit: 150 + snippetSizeLimit: 300 } export interface IAIAgentSettings { diff --git a/Services/VaultService.ts b/Services/VaultService.ts index e4eb7ca..a3bf734 100644 --- a/Services/VaultService.ts +++ b/Services/VaultService.ts @@ -91,13 +91,13 @@ export class VaultService { await this.vault.process(file, () => content); } - public async delete(file: TAbstractFile, force?: boolean, allowAccessToPluginRoot: boolean = false): Promise<{ success: true } | { success: false, error: string }> { + public async delete(file: TAbstractFile, allowAccessToPluginRoot: boolean = false): Promise<{ success: true } | { success: false, error: string }> { if (this.isExclusion(file.path, allowAccessToPluginRoot)) { console.error(`Plugin attempted to delete a file that is in the exclusions list: ${file.path}`) return { success: false, error: "File is in exclusion list" }; } try { - await this.vault.delete(file, force); + await this.vault.trash(file, true); return { success: true }; } catch (error) { console.error(`Error deleting file ${file.path}:`, error); diff --git a/__tests__/Services/FileSystemService.test.ts b/__tests__/Services/FileSystemService.test.ts index a39dda2..e07c92b 100644 --- a/__tests__/Services/FileSystemService.test.ts +++ b/__tests__/Services/FileSystemService.test.ts @@ -233,7 +233,7 @@ describe('FileSystemService', () => { const result = await fileSystemService.deleteFile('delete-me.md'); expect(result).toEqual({ success: true }); - expect(mockVaultService.delete).toHaveBeenCalledWith(mockFile, undefined, false); + expect(mockVaultService.delete).toHaveBeenCalledWith(mockFile, false); }); it('should return error when file does not exist', async () => { @@ -254,18 +254,19 @@ describe('FileSystemService', () => { await fileSystemService.deleteFile('plugin/temp.json', true); expect(mockVaultService.getAbstractFileByPath).toHaveBeenCalledWith('plugin/temp.json', true); - expect(mockVaultService.delete).toHaveBeenCalledWith(mockFile, undefined, true); + expect(mockVaultService.delete).toHaveBeenCalledWith(mockFile, true); }); - it('should return error when path is not a file', async () => { + it('should delete folder successfully (supports both files and folders)', async () => { const mockFolder = createMockFolder('folder'); mockVaultService.getAbstractFileByPath = vi.fn().mockReturnValue(mockFolder); + mockVaultService.delete = vi.fn().mockResolvedValue({ success: true }); const result = await fileSystemService.deleteFile('folder'); - expect(result).toEqual({ success: false, error: 'File not found' }); - expect(mockVaultService.delete).not.toHaveBeenCalled(); + expect(result).toEqual({ success: true }); + expect(mockVaultService.delete).toHaveBeenCalledWith(mockFolder, false); }); }); diff --git a/__tests__/Services/SettingsService.test.ts b/__tests__/Services/SettingsService.test.ts index be91b84..5454e2d 100644 --- a/__tests__/Services/SettingsService.test.ts +++ b/__tests__/Services/SettingsService.test.ts @@ -35,7 +35,7 @@ describe('SettingsService', () => { expect(settingsService.settings.exclusions).toEqual([]); expect(settingsService.settings.userInstruction).toBe(''); expect(settingsService.settings.searchResultsLimit).toBe(15); - expect(settingsService.settings.snippetSizeLimit).toBe(150); + expect(settingsService.settings.snippetSizeLimit).toBe(300); }); it('should merge loaded settings with defaults', () => { @@ -80,7 +80,7 @@ describe('SettingsService', () => { expect(settingsService.settings.exclusions).toEqual([]); // Default expect(settingsService.settings.userInstruction).toBe(''); // Default expect(settingsService.settings.searchResultsLimit).toBe(15); // Default - expect(settingsService.settings.snippetSizeLimit).toBe(150); // Default + expect(settingsService.settings.snippetSizeLimit).toBe(300); // Default }); }); @@ -97,7 +97,7 @@ describe('SettingsService', () => { exclusions: [], userInstruction: '', searchResultsLimit: 15, - snippetSizeLimit: 150 + snippetSizeLimit: 300 }; settingsService = new SettingsService(loadedSettings); }); @@ -137,7 +137,7 @@ describe('SettingsService', () => { exclusions: [], userInstruction: '', searchResultsLimit: 15, - snippetSizeLimit: 150 + snippetSizeLimit: 300 }; settingsService = new SettingsService(loadedSettings); @@ -157,7 +157,7 @@ describe('SettingsService', () => { exclusions: [], userInstruction: '', searchResultsLimit: 15, - snippetSizeLimit: 150 + snippetSizeLimit: 300 }; settingsService = new SettingsService(loadedSettings); @@ -177,7 +177,7 @@ describe('SettingsService', () => { exclusions: [], userInstruction: '', searchResultsLimit: 15, - snippetSizeLimit: 150 + snippetSizeLimit: 300 }; settingsService = new SettingsService(loadedSettings); @@ -222,7 +222,7 @@ describe('SettingsService', () => { exclusions: [], userInstruction: '', searchResultsLimit: 15, - snippetSizeLimit: 150 + snippetSizeLimit: 300 }; settingsService = new SettingsService(loadedSettings); }); @@ -276,7 +276,7 @@ describe('SettingsService', () => { exclusions: ['node_modules'], userInstruction: 'Be helpful', searchResultsLimit: 15, - snippetSizeLimit: 150 + snippetSizeLimit: 300 }; settingsService = new SettingsService(loadedSettings); }); @@ -403,7 +403,7 @@ describe('SettingsService', () => { it('should use default snippetSizeLimit when not specified', () => { settingsService = new SettingsService({}); - expect(settingsService.settings.snippetSizeLimit).toBe(150); + expect(settingsService.settings.snippetSizeLimit).toBe(300); }); it('should allow custom searchResultsLimit values', () => { diff --git a/__tests__/Services/VaultService.test.ts b/__tests__/Services/VaultService.test.ts index 53e2891..0300583 100644 --- a/__tests__/Services/VaultService.test.ts +++ b/__tests__/Services/VaultService.test.ts @@ -26,6 +26,7 @@ const mockVault = { create: vi.fn(), process: vi.fn(), delete: vi.fn(), + trash: vi.fn(), createFolder: vi.fn(), getFiles: vi.fn(), getAllFolders: vi.fn(), @@ -53,7 +54,7 @@ const mockSettings: IAIAgentSettings = { exclusions: [], userInstruction: '', searchResultsLimit: 15, - snippetSizeLimit: 150 + snippetSizeLimit: 300 }; const mockPlugin = { @@ -104,7 +105,7 @@ describe('VaultService - Integration Tests', () => { // Reset settings to defaults mockSettings.exclusions = []; mockSettings.searchResultsLimit = 15; - mockSettings.snippetSizeLimit = 150; + mockSettings.snippetSizeLimit = 300; // Set default mock for adapter.exists (can be overridden in individual tests) mockVault.adapter.exists.mockResolvedValue(false); @@ -399,36 +400,36 @@ describe('VaultService - Integration Tests', () => { describe('delete', () => { it('should delete file successfully when not excluded', async () => { const mockFile = createMockFile('note.md'); - mockVault.delete.mockResolvedValue(undefined); + mockVault.trash.mockResolvedValue(undefined); const result = await vaultService.delete(mockFile); expect(result).toEqual({ success: true }); - expect(mockVault.delete).toHaveBeenCalledWith(mockFile, undefined); + expect(mockVault.trash).toHaveBeenCalledWith(mockFile, true); }); it('should not delete file and return error when excluded', async () => { const mockFile = createMockFile('AI Agent/test.md'); - const result = await vaultService.delete(mockFile, false, false); + const result = await vaultService.delete(mockFile, false); expect(result).toEqual({ success: false, error: 'File is in exclusion list' }); - expect(mockVault.delete).not.toHaveBeenCalled(); + expect(mockVault.trash).not.toHaveBeenCalled(); expect(consoleErrorSpy).toHaveBeenCalled(); }); it('should pass force parameter to vault.delete', async () => { const mockFile = createMockFile('note.md'); - mockVault.delete.mockResolvedValue(undefined); + mockVault.trash.mockResolvedValue(undefined); await vaultService.delete(mockFile, true); - expect(mockVault.delete).toHaveBeenCalledWith(mockFile, true); + expect(mockVault.trash).toHaveBeenCalledWith(mockFile, true); }); it('should return error when deletion fails', async () => { const mockFile = createMockFile('note.md'); - mockVault.delete.mockRejectedValue(new Error('Deletion failed')); + mockVault.trash.mockRejectedValue(new Error('Deletion failed')); const result = await vaultService.delete(mockFile); @@ -438,7 +439,7 @@ describe('VaultService - Integration Tests', () => { it('should handle non-Error objects in catch block', async () => { const mockFile = createMockFile('note.md'); - mockVault.delete.mockRejectedValue('string error'); + mockVault.trash.mockRejectedValue('string error'); const result = await vaultService.delete(mockFile);