diff --git a/AIClasses/FunctionDefinitions/AIFunctionDefinitions.ts b/AIClasses/FunctionDefinitions/AIFunctionDefinitions.ts index 742fe08..bf4c26c 100644 --- a/AIClasses/FunctionDefinitions/AIFunctionDefinitions.ts +++ b/AIClasses/FunctionDefinitions/AIFunctionDefinitions.ts @@ -5,6 +5,7 @@ import { WriteVaultFile } from "./Functions/WriteVaultFile"; import { DeleteVaultFiles } from "./Functions/DeleteVaultFiles"; import { MoveVaultFiles } from "./Functions/MoveVaultFiles"; import { ListVaultFiles } from "./Functions/ListVaultFiles"; +import { PatchVaultFile } from "./Functions/PatchVaultFile"; export class AIFunctionDefinitions { public getQueryActions(destructive: boolean): IAIFunctionDefinition[] { @@ -17,6 +18,7 @@ export class AIFunctionDefinitions { if (destructive) { actions = actions.concat([ WriteVaultFile, + PatchVaultFile, DeleteVaultFiles, MoveVaultFiles ]); diff --git a/AIClasses/FunctionDefinitions/Functions/PatchVaultFile.ts b/AIClasses/FunctionDefinitions/Functions/PatchVaultFile.ts new file mode 100644 index 0000000..4803be9 --- /dev/null +++ b/AIClasses/FunctionDefinitions/Functions/PatchVaultFile.ts @@ -0,0 +1,54 @@ +import { AIFunction } from "Enums/AIFunction"; +import type { IAIFunctionDefinition } from "../IAIFunctionDefinition"; + +export const PatchVaultFile: IAIFunctionDefinition = { + name: AIFunction.PatchVaultFile, + description: `Apply targeted changes to an existing file in the vault using a unified diff patch format. + + This tool efficiently modifies specific sections of a file without requiring the entire file content to be sent. The patch follows standard unified diff format (similar to git diff output). + + **When to use this tool:** + - Making small, targeted edits to large files (a few lines changed) + - Edits in the middle of a file where you have clear surrounding context + - Simple line additions, deletions, or replacements with minimal changes + + **CRITICAL:** The patch must match the existing file content EXACTLY - blank lines, spaces and exact line contents should all correspond to the current state of the file.`, + parameters: { + type: "object", + properties: { + file_path: { + type: "string", + description: "The full path to the file within the vault (e.g., 'folder/note.md')" + }, + patch: { + type: "string", + description: `A unified diff format patch string showing the changes to apply. + + CRITICAL FORMAT REQUIREMENTS: + - Must follow standard unified diff format with @@ line markers + - Line counts in @@ headers MUST be exact (e.g., @@ -10,5 +10,6 @@ means 5 old lines, 6 new lines) + - Include ALL blank/empty lines exactly as they appear in the source file + - Context lines (no prefix) must match the file content EXACTLY including whitespace + - Use '+' prefix for added lines and '-' prefix for removed lines (no space after prefix) + - Context lines should have a single space prefix + + Example (adding a line after line 10): + @@ -10,3 +10,4 @@ + existing line before + existing line where we insert + +new content to add + existing line after + + WARNING: The patch will fail if: + - Line counts don't match the actual number of lines shown + - Blank lines are missing or extra blank lines are included + - Context lines don't exactly match the file content` + }, + user_message: { + type: "string", + description: "A short message to be displayed to the user explaining what you're writing and why (e.g., 'Updating project plan with new tasks')" + } + }, + required: ["file_path", "patch", "user_message"] + } +} \ No newline at end of file diff --git a/AIClasses/FunctionDefinitions/Functions/WriteVaultFile.ts b/AIClasses/FunctionDefinitions/Functions/WriteVaultFile.ts index 982e30d..a7c21fe 100644 --- a/AIClasses/FunctionDefinitions/Functions/WriteVaultFile.ts +++ b/AIClasses/FunctionDefinitions/Functions/WriteVaultFile.ts @@ -4,9 +4,11 @@ import type { IAIFunctionDefinition } from "../IAIFunctionDefinition"; export const WriteVaultFile: IAIFunctionDefinition = { name: AIFunction.WriteVaultFile, description: `Writes content to a file, creating it if it doesn't exist or replacing its contents if it does. - Use this for creating new notes or completely updating existing ones when you have the full content ready. - IMPORTANT: This replaces the entire file content - always read the file first with ${AIFunction.ReadVaultFiles} if you need to preserve existing content and make partial changes. - For simple updates or additions, reading first ensures you don't lose data.`, + + **When to use this tool:** + - Creating new notes, documents, or files from scratch + - Completely rewriting a file's contents (when most/all content needs to change) + - Generating new files from templates or structured data`, parameters: { type: "object", properties: { @@ -20,7 +22,7 @@ export const WriteVaultFile: IAIFunctionDefinition = { }, user_message: { type: "string", - description: "A short message to be displayed to the user explaining what you're writing and why (e.g., 'Creating your daily note for today' or 'Updating project plan with new tasks')" + description: "A short message to be displayed to the user explaining what you're writing and why (e.g., 'Creating your daily note for today')" } }, required: ["file_path", "content", "user_message"] diff --git a/AIClasses/Schemas/AIFunctionSchemas.ts b/AIClasses/Schemas/AIFunctionSchemas.ts index 8026a84..91aa4cb 100644 --- a/AIClasses/Schemas/AIFunctionSchemas.ts +++ b/AIClasses/Schemas/AIFunctionSchemas.ts @@ -21,6 +21,12 @@ export const WriteVaultFileArgsSchema = z.object({ user_message: z.string() }); +export const PatchVaultFileArgsSchema = z.object({ + file_path: z.string(), + patch: z.string(), + user_message: z.string() +}); + export const DeleteVaultFilesArgsSchema = z.object({ file_paths: z.array(z.string()), user_message: z.string(), @@ -43,6 +49,7 @@ export const ListVaultFilesArgsSchema = z.object({ export type SearchVaultFilesArgs = z.infer; export type ReadVaultFilesArgs = z.infer; export type WriteVaultFileArgs = z.infer; +export type PatchVaultFileArgs = z.infer; export type DeleteVaultFilesArgs = z.infer; export type MoveVaultFilesArgs = z.infer; export type ListVaultFilesArgs = z.infer; diff --git a/Enums/AIFunction.ts b/Enums/AIFunction.ts index 9777c55..6d9edd3 100644 --- a/Enums/AIFunction.ts +++ b/Enums/AIFunction.ts @@ -4,6 +4,7 @@ export enum AIFunction { SearchVaultFiles = "search_vault_files", ReadVaultFiles = "read_vault_files", WriteVaultFile = "write_vault_file", + PatchVaultFile = "patch_vault_file", DeleteVaultFiles = "delete_vault_files", MoveVaultFiles = "move_vault_files", ListVaultFiles = "list_vault_files", diff --git a/Services/AIFunctionService.ts b/Services/AIFunctionService.ts index 1a71d95..c0878e0 100644 --- a/Services/AIFunctionService.ts +++ b/Services/AIFunctionService.ts @@ -13,7 +13,8 @@ import { WriteVaultFileArgsSchema, DeleteVaultFilesArgsSchema, MoveVaultFilesArgsSchema, - ListVaultFilesArgsSchema + ListVaultFilesArgsSchema, + PatchVaultFileArgsSchema } from "AIClasses/Schemas/AIFunctionSchemas"; export class AIFunctionService { @@ -64,6 +65,18 @@ export class AIFunctionService { } return new AIFunctionResponse(functionCall.name, await this.writeVaultFile(parseResult.data.file_path, parseResult.data.content), functionCall.toolId); } + + case AIFunction.PatchVaultFile: { + const parseResult = PatchVaultFileArgsSchema.safeParse(functionCall.arguments); + if (!parseResult.success) { + return new AIFunctionResponse( + functionCall.name, + { error: `Invalid arguments for PatchVaultFile: ${parseResult.error.message}` }, + functionCall.toolId + ); + } + return new AIFunctionResponse(functionCall.name, await this.patchVaultFile(parseResult.data.file_path, parseResult.data.patch), functionCall.toolId); + } case AIFunction.DeleteVaultFiles: { const parseResult = DeleteVaultFilesArgsSchema.safeParse(functionCall.arguments); @@ -159,6 +172,14 @@ export class AIFunctionService { return { success: true }; } + private async patchVaultFile(filePath: string, patch: string): Promise { + const result = await this.fileSystemService.patchFile(normalizePath(filePath), patch); + if (result instanceof Error) { + return { success: false, error: result.message }; + } + return { success: true }; + } + private async deleteVaultFiles(filePaths: string[], confirmation: boolean): Promise { if (!confirmation) { return { error: "Confirmation was false, no action taken" }; diff --git a/Services/ChatService.ts b/Services/ChatService.ts index 54f343a..6ddc012 100644 --- a/Services/ChatService.ts +++ b/Services/ChatService.ts @@ -108,8 +108,8 @@ export class ChatService { if (AbortService.isAbortError(error)) { callbacks.onCancel(); } + new Notice("Vaultkeeper AI encountered an error"); } finally { - // reset "Cancelling..." flag this needs to get to window (window may actually handle this itself) this.eventService.trigger(Event.DiffClosed); await this.saveConversation(conversation); if (this.semaphoreHeld) { diff --git a/Services/DiffService.ts b/Services/DiffService.ts index 4d57802..338c720 100644 --- a/Services/DiffService.ts +++ b/Services/DiffService.ts @@ -8,6 +8,7 @@ import type { Diff2HtmlUIConfig } from 'diff2html/lib/ui/js/diff2html-ui'; import { ColorSchemeType, OutputFormatType } from 'diff2html/lib/types'; import { Component } from 'obsidian'; import { AbortService } from './AbortService'; +import { Exception } from 'Helpers/Exception'; interface DiffResult { accepted: boolean; @@ -35,6 +36,14 @@ export class DiffService extends Component { })); } + public applyPatch(source: string, patch: string): string | false | Error { + try { + return Diff.applyPatch(source, patch); + } catch (error) { + return Exception.new(error); + } + } + public async requestDiff(oldFileName: string, newFileName: string, oldContent: string, newContent: string): Promise { const diffString = this.createDiffString(oldFileName, newFileName, oldContent, newContent); diff --git a/Services/FileSystemService.ts b/Services/FileSystemService.ts index 745b0e6..7a310e8 100644 --- a/Services/FileSystemService.ts +++ b/Services/FileSystemService.ts @@ -33,6 +33,24 @@ export class FileSystemService { return await this.vaultService.modify(file, content, allowAccessToPluginRoot, requiresConfirmation); } + public async patchFile(filePath: string, patch: string, allowAccessToPluginRoot: boolean = false, requiresConfirmation: boolean = true): Promise { + const file: TAbstractFile | null = this.vaultService.getAbstractFileByPath(filePath, allowAccessToPluginRoot); + + let fileToPatch: TFile; + if (file instanceof TFile) { + fileToPatch = file; + } else { + // if the file doesn't exist we may as well create it even though this is just a patch operation + const result = await this.writeFile(filePath, "", allowAccessToPluginRoot, requiresConfirmation); + if (result instanceof Error) { + return result; + } + fileToPatch = result; + } + + return await this.vaultService.patch(fileToPatch, patch, allowAccessToPluginRoot, requiresConfirmation); + } + public async deleteFile(filePath: string, allowAccessToPluginRoot: boolean = false, requiresConfirmation: boolean = true): Promise { const file: TAbstractFile | null = this.vaultService.getAbstractFileByPath(filePath, allowAccessToPluginRoot); diff --git a/Services/VaultService.ts b/Services/VaultService.ts index 52fc50a..7266793 100644 --- a/Services/VaultService.ts +++ b/Services/VaultService.ts @@ -106,12 +106,36 @@ export class VaultService { return Exception.new(`File does not exist: ${filePath}`); } - return this.proposeChange(file.name, file.name, await this.vault.read(file), content, requiresConfirmation, async () => { + return this.proposeChange(file.name, file.name, await this.read(file, allowAccessToPluginRoot), content, requiresConfirmation, async () => { await this.vault.process(file, () => content); return file; }); } + public async patch(file: TFile, patch: string, allowAccessToPluginRoot: boolean = false, requiresConfirmation: boolean = true): Promise { + const filePath = this.sanitiserService.sanitize(file.path); + if (this.isExclusion(file.path, allowAccessToPluginRoot)) { + Exception.log(`Plugin attempted to patch a file that is in the exclusion list: ${filePath}`); + return Exception.new(`File does not exist: ${filePath}`); + } + + const currentContent = await this.read(file, allowAccessToPluginRoot); + const patchedContent = this.diffService.applyPatch(currentContent, patch); + + if (patchedContent instanceof Error) { + return Exception.new(`Failed to apply patch - invalid patch format. ${patchedContent.message}`); + } + + if (patchedContent === false) { + return Exception.new("Failed to apply patch - the file may have been modified since the patch was created"); + } + + return this.proposeChange(file.name, file.name, currentContent, patchedContent, requiresConfirmation, async () => { + await this.vault.process(file, () => patchedContent); + return file; + }); + } + public async delete(file: TAbstractFile, allowAccessToPluginRoot: boolean = false, requiresConfirmation: boolean = true): Promise { const filePath = this.sanitiserService.sanitize(file.path); if (this.isExclusion(file.path, allowAccessToPluginRoot)) { @@ -121,7 +145,7 @@ export class VaultService { // handle file deletion if (file instanceof TFile) { - return this.proposeChange(file.name, file.name, await this.vault.read(file), "", requiresConfirmation, async () => { + return this.proposeChange(file.name, file.name, await this.read(file, allowAccessToPluginRoot), "", requiresConfirmation, async () => { await this.fileManager.trashFile(file); }); } diff --git a/Views/DiffView.ts b/Views/DiffView.ts index 1d1aa01..c4377d1 100644 --- a/Views/DiffView.ts +++ b/Views/DiffView.ts @@ -68,6 +68,12 @@ export class DiffView extends ItemView { const diff2htmlUi = new Diff2HtmlUI(this.diffContainer, this.diffString, this.config); diff2htmlUi.draw(); + diff2htmlUi.highlightCode(); + + requestAnimationFrame(() => { + const firstChange = this.diffContainer?.querySelector('.d2h-change, .d2h-ins, .d2h-del'); + firstChange?.scrollIntoView({ behavior: 'smooth', block: 'center' }); + }); } private resetContainer(): HTMLElement { diff --git a/__tests__/Services/AIFunctionService.test.ts b/__tests__/Services/AIFunctionService.test.ts index 382d5e2..221686d 100644 --- a/__tests__/Services/AIFunctionService.test.ts +++ b/__tests__/Services/AIFunctionService.test.ts @@ -17,6 +17,7 @@ import { AbortService } from '../../Services/AbortService'; * - SearchVaultFiles * - ReadVaultFiles * - WriteVaultFile + * - PatchVaultFile * - DeleteVaultFiles * - MoveVaultFiles * - RequestWebSearch (Gemini only) @@ -34,6 +35,7 @@ describe('AIFunctionService - Integration Tests', () => { listFilesInDirectory: vi.fn(), readFile: vi.fn(), writeFile: vi.fn(), + patchFile: vi.fn(), deleteFile: vi.fn(), moveFile: vi.fn() }; @@ -342,6 +344,230 @@ describe('AIFunctionService - Integration Tests', () => { }); }); + describe('performAIFunction - PatchVaultFile', () => { + it('should apply patch successfully', async () => { + mockFileSystemService.patchFile.mockResolvedValue(createMockFile('notes/test.md', 'test')); + + const patch = `@@ -1,3 +1,3 @@ + # Test Note +-old content ++new content + more content`; + + const result = await service.performAIFunction({ + name: AIFunction.PatchVaultFile, + arguments: { + file_path: 'notes/test.md', + patch: patch, + user_message: 'Updating test note' + }, + toolId: 'tool_patch_1' + } as any); + + expect(mockFileSystemService.patchFile).toHaveBeenCalledWith('notes/test.md', patch); + expect(result.response).toEqual({ success: true }); + }); + + it('should handle patch failure', async () => { + const error = new Error('Failed to apply patch - the file may have been modified since the patch was created'); + mockFileSystemService.patchFile.mockResolvedValue(error); + + const patch = `@@ -1,3 +1,3 @@ + # Test +-old ++new`; + + const result = await service.performAIFunction({ + name: AIFunction.PatchVaultFile, + arguments: { + file_path: 'notes/test.md', + patch: patch, + user_message: 'Updating note' + }, + toolId: 'tool_patch_2' + } as any); + + expect(result.response.success).toBe(false); + expect(result.response.error).toBe('Failed to apply patch - the file may have been modified since the patch was created'); + }); + + it('should normalize file path', async () => { + mockFileSystemService.patchFile.mockResolvedValue(createMockFile('folder/file.md', 'file')); + + const patch = `@@ -1,1 +1,1 @@ +-old ++new`; + + await service.performAIFunction({ + name: AIFunction.PatchVaultFile, + arguments: { + file_path: 'folder\\subfolder\\file.md', + patch: patch, + user_message: 'Patching file' + }, + toolId: 'tool_patch_3' + } as any); + + // normalizePath should convert backslashes to forward slashes + expect(mockFileSystemService.patchFile).toHaveBeenCalledWith( + expect.stringContaining('/'), + patch + ); + }); + + it('should handle file not found error', async () => { + const error = new Error('File does not exist: missing.md'); + mockFileSystemService.patchFile.mockResolvedValue(error); + + const patch = `@@ -1,1 +1,1 @@ +-old ++new`; + + const result = await service.performAIFunction({ + name: AIFunction.PatchVaultFile, + arguments: { + file_path: 'missing.md', + patch: patch, + user_message: 'Patching missing file' + }, + toolId: 'tool_patch_4' + } as any); + + expect(result.response.success).toBe(false); + expect(result.response.error).toContain('File does not exist'); + }); + + it('should handle complex multi-hunk patch', async () => { + mockFileSystemService.patchFile.mockResolvedValue(createMockFile('complex.md', 'complex')); + + const patch = `@@ -1,3 +1,3 @@ + # Title +-Old line 1 ++New line 1 + Context line +@@ -10,3 +10,3 @@ + Another context +-Old line 2 ++New line 2 + Final context`; + + const result = await service.performAIFunction({ + name: AIFunction.PatchVaultFile, + arguments: { + file_path: 'complex.md', + patch: patch, + user_message: 'Applying complex patch' + }, + toolId: 'tool_patch_5' + } as any); + + expect(mockFileSystemService.patchFile).toHaveBeenCalledWith('complex.md', patch); + expect(result.response.success).toBe(true); + }); + + it('should handle patch with additions only', async () => { + mockFileSystemService.patchFile.mockResolvedValue(createMockFile('additions.md', 'additions')); + + const patch = `@@ -1,2 +1,4 @@ + # Title + Existing content ++New line 1 ++New line 2`; + + const result = await service.performAIFunction({ + name: AIFunction.PatchVaultFile, + arguments: { + file_path: 'additions.md', + patch: patch, + user_message: 'Adding new lines' + }, + toolId: 'tool_patch_6' + } as any); + + expect(result.response.success).toBe(true); + }); + + it('should handle patch with deletions only', async () => { + mockFileSystemService.patchFile.mockResolvedValue(createMockFile('deletions.md', 'deletions')); + + const patch = `@@ -1,4 +1,2 @@ + # Title +-Line to remove 1 +-Line to remove 2 + Remaining content`; + + const result = await service.performAIFunction({ + name: AIFunction.PatchVaultFile, + arguments: { + file_path: 'deletions.md', + patch: patch, + user_message: 'Removing lines' + }, + toolId: 'tool_patch_7' + } as any); + + expect(result.response.success).toBe(true); + }); + + it('should handle permission denied error', async () => { + const error = new Error('Permission denied'); + mockFileSystemService.patchFile.mockResolvedValue(error); + + const patch = `@@ -1,1 +1,1 @@ +-old ++new`; + + const result = await service.performAIFunction({ + name: AIFunction.PatchVaultFile, + arguments: { + file_path: 'protected.md', + patch: patch, + user_message: 'Patching protected file' + }, + toolId: 'tool_patch_8' + } as any); + + expect(result.response.success).toBe(false); + expect(result.response.error).toBe('Permission denied'); + }); + + it('should return correct toolId in response', async () => { + mockFileSystemService.patchFile.mockResolvedValue(createMockFile('test.md', 'test')); + + const patch = `@@ -1,1 +1,1 @@ +-old ++new`; + + const result = await service.performAIFunction({ + name: AIFunction.PatchVaultFile, + arguments: { + file_path: 'test.md', + patch: patch, + user_message: 'Test patch' + }, + toolId: 'unique_tool_id_123' + } as any); + + expect(result.toolId).toBe('unique_tool_id_123'); + expect(result.name).toBe(AIFunction.PatchVaultFile); + }); + + it('should handle invalid arguments', async () => { + const result = await service.performAIFunction({ + name: AIFunction.PatchVaultFile, + arguments: { + // missing required fields + file_path: 'test.md' + // patch and user_message are missing + }, + toolId: 'tool_patch_invalid' + } as any); + + expect(result.response.error).toContain('Invalid arguments for PatchVaultFile'); + expect(mockFileSystemService.patchFile).not.toHaveBeenCalled(); + }); + }); + describe('performAIFunction - DeleteVaultFiles', () => { it('should delete multiple files successfully', async () => { mockFileSystemService.deleteFile @@ -662,5 +888,40 @@ describe('AIFunctionService - Integration Tests', () => { expect(moveResult.response.results[0].success).toBe(true); }); + + it('should handle read -> patch workflow', async () => { + // First read the file + mockFileSystemService.readFile.mockResolvedValue('# Original Title\n\nOriginal content'); + + const readResult = await service.performAIFunction({ + name: AIFunction.ReadVaultFiles, + arguments: { file_paths: ['document.md'], user_message: 'Reading file' }, + toolId: 'read_2' + } as any); + + expect(readResult.response.results[0].contents).toContain('Original'); + + // Then patch it + mockFileSystemService.patchFile.mockResolvedValue(createMockFile('document.md', 'document')); + + const patch = `@@ -1,3 +1,3 @@ +-# Original Title ++# Updated Title + + Original content`; + + const patchResult = await service.performAIFunction({ + name: AIFunction.PatchVaultFile, + arguments: { + file_path: 'document.md', + patch: patch, + user_message: 'Updating title' + }, + toolId: 'patch_1' + } as any); + + expect(patchResult.response.success).toBe(true); + expect(mockFileSystemService.patchFile).toHaveBeenCalledWith('document.md', patch); + }); }); }); diff --git a/__tests__/Services/DiffService.test.ts b/__tests__/Services/DiffService.test.ts index ee45d9c..057d0d5 100644 --- a/__tests__/Services/DiffService.test.ts +++ b/__tests__/Services/DiffService.test.ts @@ -824,4 +824,129 @@ describe('DiffService', () => { expect(() => new DiffService()).toThrow(); }); }); + + describe('applyPatch', () => { + it('should successfully apply a valid patch', () => { + const source = 'line 1\nline 2\nline 3'; + const patch = `--- test.md ++++ test.md +@@ -1,3 +1,3 @@ + line 1 +-line 2 ++line 2 modified + line 3`; + + const result = service.applyPatch(source, patch); + + expect(result).toBe('line 1\nline 2 modified\nline 3'); + }); + + it('should return false when patch does not match source', () => { + const source = 'different content'; + const patch = `--- test.md ++++ test.md +@@ -1,3 +1,3 @@ + line 1 +-line 2 ++line 2 modified + line 3`; + + const result = service.applyPatch(source, patch); + + expect(result).toBe(false); + }); + + it('should return Error when patch format is invalid', () => { + const source = 'line 1\nline 2'; + const patch = `--- test.md ++++ test.md +@@ -1,5 +1,2 @@ +-line 1 ++modified`; + + const result = service.applyPatch(source, patch); + + expect(result).toBeInstanceOf(Error); + expect((result as Error).message).toContain('line count did not match'); + }); + + it('should return false when patch has malformed hunk header', () => { + const source = 'content'; + const patch = `--- test.md ++++ test.md +@@ invalid header @@ +-old ++new`; + + const result = service.applyPatch(source, patch); + + // Malformed headers result in patch not applying (returns false) + expect(result).toBe(false); + }); + + it('should handle empty source', () => { + const source = ''; + const patch = `--- test.md ++++ test.md +@@ -0,0 +1,2 @@ ++line 1 ++line 2`; + + const result = service.applyPatch(source, patch); + + // applyPatch adds a trailing newline + expect(result).toBe('line 1\nline 2\n'); + }); + + it('should handle patch that removes all content', () => { + const source = 'line 1\nline 2'; + const patch = `--- test.md ++++ test.md +@@ -1,2 +0,0 @@ +-line 1 +-line 2`; + + const result = service.applyPatch(source, patch); + + expect(result).toBe(''); + }); + + it('should return Error when patch has extra lines (real-world case)', () => { + // Based on actual failure: AI provided more lines than declared in header + const source = 'line 1\nline 2\nline 3'; + const patch = `--- test.md ++++ test.md +@@ -1,3 +1,4 @@ + line 1 + line 2 + line 3 ++new line + extra line`; // This is 5 lines total but header says 3 old, 4 new + + const result = service.applyPatch(source, patch); + + expect(result).toBeInstanceOf(Error); + // Error message varies: could be "Unknown line" or "line count did not match" + expect((result as Error).message).toMatch(/Unknown line|line count did not match/); + }); + + it('should return Error when missing blank lines (real-world case)', () => { + // Based on actual failure: AI missing consecutive blank lines + const source = 'text\n\n\n---\nmore text'; // Two blank lines before --- + const patch = `--- test.md ++++ test.md +@@ -1,5 +1,6 @@ + text + ++inserted line + --- + more text`; // Only one blank line - doesn't match header count! + + const result = service.applyPatch(source, patch); + + // This throws because line count doesn't match (header says 5->6 but only shows 4->5) + expect(result).toBeInstanceOf(Error); + expect((result as Error).message).toContain('line count did not match'); + }); + }); }); diff --git a/__tests__/Services/FileSystemService.test.ts b/__tests__/Services/FileSystemService.test.ts index f1f86df..77930ac 100644 --- a/__tests__/Services/FileSystemService.test.ts +++ b/__tests__/Services/FileSystemService.test.ts @@ -49,6 +49,7 @@ describe('FileSystemService', () => { read: vi.fn(), create: vi.fn(), modify: vi.fn(), + patch: vi.fn(), delete: vi.fn(), move: vi.fn(), listFilesInDirectory: vi.fn(), @@ -187,6 +188,142 @@ describe('FileSystemService', () => { }); }); + describe('patchFile', () => { + it('should patch existing file successfully', async () => { + const mockFile = createMockFile('existing.md'); + const patch = `@@ -1,3 +1,3 @@ + # Title +-old content ++new content + more lines`; + + mockVaultService.getAbstractFileByPath = vi.fn().mockReturnValue(mockFile); + mockVaultService.patch = vi.fn().mockResolvedValue(mockFile); + + const result = await fileSystemService.patchFile('existing.md', patch); + + expect(result).toBe(mockFile); + expect(mockVaultService.patch).toHaveBeenCalledWith(mockFile, patch, false, true); + }); + + it('should create file with empty content when file does not exist', async () => { + const mockFile = createMockFile('new.md'); + const patch = `@@ -1,0 +1,2 @@ ++# New File ++Content here`; + + // Mock sequence: first call returns null (file doesn't exist), second call returns null (writeFile checks), then create succeeds + mockVaultService.getAbstractFileByPath = vi.fn() + .mockReturnValueOnce(null) // patchFile checks if file exists + .mockReturnValueOnce(null); // writeFile (called by patchFile) checks if file exists + mockVaultService.create = vi.fn().mockResolvedValue(mockFile); + mockVaultService.patch = vi.fn().mockResolvedValue(mockFile); + + const result = await fileSystemService.patchFile('new.md', patch); + + // writeFile should create the file with empty content + expect(mockVaultService.create).toHaveBeenCalledWith('new.md', '', false, true); + expect(mockVaultService.patch).toHaveBeenCalledWith(mockFile, patch, false, true); + expect(result).toBe(mockFile); + }); + + it('should return error when patch fails', async () => { + const mockFile = createMockFile('existing.md'); + const patch = `@@ -1,1 +1,1 @@ +-old ++new`; + const error = new Error('Failed to apply patch - the file may have been modified since the patch was created'); + + mockVaultService.getAbstractFileByPath = vi.fn().mockReturnValue(mockFile); + mockVaultService.patch = vi.fn().mockResolvedValue(error); + + const result = await fileSystemService.patchFile('existing.md', patch); + + expect(result).toBeInstanceOf(Error); + expect((result as Error).message).toContain('Failed to apply patch'); + }); + + it('should return error when creating empty file fails', async () => { + const patch = `@@ -1,0 +1,1 @@ ++New line`; + const error = new Error('Create failed'); + + mockVaultService.getAbstractFileByPath = vi.fn().mockReturnValue(null); + mockVaultService.create = vi.fn().mockResolvedValue(error); + + const result = await fileSystemService.patchFile('new.md', patch); + + expect(result).toBeInstanceOf(Error); + expect((result as Error).message).toBe('Create failed'); + expect(mockVaultService.patch).not.toHaveBeenCalled(); + }); + + it('should respect allowAccessToPluginRoot parameter', async () => { + const mockFile = createMockFile('plugin/config.md'); + const patch = `@@ -1,1 +1,1 @@ +-setting=old ++setting=new`; + + mockVaultService.getAbstractFileByPath = vi.fn().mockReturnValue(mockFile); + mockVaultService.patch = vi.fn().mockResolvedValue(mockFile); + + await fileSystemService.patchFile('plugin/config.md', patch, true); + + expect(mockVaultService.getAbstractFileByPath).toHaveBeenCalledWith('plugin/config.md', true); + expect(mockVaultService.patch).toHaveBeenCalledWith(mockFile, patch, true, true); + }); + + it('should handle complex multi-hunk patches', async () => { + const mockFile = createMockFile('document.md'); + const patch = `@@ -1,3 +1,3 @@ + # Title +-Old intro ++New intro + Content +@@ -10,2 +10,2 @@ + Section +-Old conclusion ++New conclusion`; + + mockVaultService.getAbstractFileByPath = vi.fn().mockReturnValue(mockFile); + mockVaultService.patch = vi.fn().mockResolvedValue(mockFile); + + const result = await fileSystemService.patchFile('document.md', patch); + + expect(result).toBe(mockFile); + expect(mockVaultService.patch).toHaveBeenCalledWith(mockFile, patch, false, true); + }); + + it('should handle file creation when getAbstractFileByPath returns null after create', async () => { + const patch = `@@ -1,0 +1,1 @@ ++content`; + + mockVaultService.getAbstractFileByPath = vi.fn().mockReturnValue(null); + mockVaultService.create = vi.fn().mockResolvedValue(createMockFile('new.md')); + mockVaultService.patch = vi.fn().mockResolvedValue(createMockFile('new.md')); + + await fileSystemService.patchFile('new.md', patch); + + // Should call patch on the created file + expect(mockVaultService.create).toHaveBeenCalledWith('new.md', '', false, true); + expect(mockVaultService.patch).toHaveBeenCalled(); + }); + + it('should respect requiresConfirmation parameter', async () => { + const mockFile = createMockFile('test.md'); + const patch = `@@ -1,1 +1,1 @@ +-old ++new`; + + mockVaultService.getAbstractFileByPath = vi.fn().mockReturnValue(mockFile); + mockVaultService.patch = vi.fn().mockResolvedValue(mockFile); + + await fileSystemService.patchFile('test.md', patch, false, true); + + expect(mockVaultService.patch).toHaveBeenCalledWith(mockFile, patch, false, true); + }); + }); + describe('deleteFile', () => { it('should delete file successfully', async () => { const mockFile = createMockFile('delete-me.md'); diff --git a/__tests__/Services/VaultService.test.ts b/__tests__/Services/VaultService.test.ts index 9618e72..f9f20ad 100644 --- a/__tests__/Services/VaultService.test.ts +++ b/__tests__/Services/VaultService.test.ts @@ -99,6 +99,7 @@ function createMockFolder(path: string, children: TAbstractFile[] = []): TFolder describe('VaultService - Integration Tests', () => { let vaultService: VaultService; let settingsService: SettingsService; + let mockDiffService: any; let consoleErrorSpy: any; beforeEach(() => { @@ -125,8 +126,9 @@ describe('VaultService - Integration Tests', () => { // Mock EventService and DiffService to avoid Obsidian Events dependency const mockEventService = { trigger: vi.fn(), on: vi.fn(), off: vi.fn() }; - const mockDiffService = { + mockDiffService = { requestDiff: vi.fn().mockResolvedValue({ accepted: true }), + applyPatch: vi.fn(), onAccept: vi.fn(), onReject: vi.fn(), onSuggest: vi.fn() @@ -409,6 +411,189 @@ describe('VaultService - Integration Tests', () => { }); }); + describe('patch', () => { + it('should apply patch successfully when file is not excluded', async () => { + const mockFile = createMockFile('note.md'); + const patch = `@@ -1,3 +1,3 @@ + # Title +-old content ++new content + more lines`; + const currentContent = '# Title\nold content\nmore lines'; + const patchedContent = '# Title\nnew content\nmore lines'; + + mockVault.read.mockResolvedValue(currentContent); + mockDiffService.applyPatch.mockReturnValue(patchedContent); + mockDiffService.requestDiff.mockResolvedValue({ accepted: true }); + mockVault.process.mockResolvedValue(undefined); + + const result = await vaultService.patch(mockFile, patch); + + expect(mockVault.read).toHaveBeenCalledWith(mockFile); + expect(mockDiffService.applyPatch).toHaveBeenCalledWith(currentContent, patch); + expect(mockDiffService.requestDiff).toHaveBeenCalled(); + expect(mockVault.process).toHaveBeenCalled(); + expect(result).toBe(mockFile); + }); + + it('should return error when file is excluded', async () => { + const mockFile = createMockFile('Vaultkeeper AI/test.md'); + const patch = `@@ -1,1 +1,1 @@ +-old ++new`; + + const result = await vaultService.patch(mockFile, patch, false); + + expect(result).toBeInstanceOf(Error); + expect((result as Error).message).toContain('File does not exist'); + expect(mockDiffService.applyPatch).not.toHaveBeenCalled(); + expect(mockVault.process).not.toHaveBeenCalled(); + }); + + it('should return error when patch fails to apply', async () => { + const mockFile = createMockFile('note.md'); + const patch = `@@ -1,1 +1,1 @@ +-old ++new`; + const currentContent = '# Different content'; + + mockVault.read.mockResolvedValue(currentContent); + mockDiffService.applyPatch.mockReturnValue(false); + + const result = await vaultService.patch(mockFile, patch); + + expect(result).toBeInstanceOf(Error); + expect((result as Error).message).toContain('Failed to apply patch'); + expect(mockVault.process).not.toHaveBeenCalled(); + }); + + it('should return error when patch format is invalid', async () => { + const mockFile = createMockFile('note.md'); + const patch = `@@ -1,5 +1,1 @@ +-old ++new`; + const currentContent = 'content'; + + mockVault.read.mockResolvedValue(currentContent); + mockDiffService.applyPatch.mockReturnValue(new Error('Removed line count did not match for hunk at line 1')); + + const result = await vaultService.patch(mockFile, patch); + + expect(result).toBeInstanceOf(Error); + expect((result as Error).message).toContain('Failed to apply patch - invalid patch format'); + expect(mockVault.process).not.toHaveBeenCalled(); + }); + + it('should allow patching excluded files when allowAccessToPluginRoot is true', async () => { + const mockFile = createMockFile('Vaultkeeper AI/config.md'); + const patch = `@@ -1,1 +1,1 @@ +-setting=old ++setting=new`; + const currentContent = 'setting=old'; + const patchedContent = 'setting=new'; + + mockVault.read.mockResolvedValue(currentContent); + mockDiffService.applyPatch.mockReturnValue(patchedContent); + mockDiffService.requestDiff.mockResolvedValue({ accepted: true }); + mockVault.process.mockResolvedValue(undefined); + + const result = await vaultService.patch(mockFile, patch, true); + + expect(mockVault.read).toHaveBeenCalledWith(mockFile); + expect(mockDiffService.applyPatch).toHaveBeenCalledWith(currentContent, patch); + expect(result).toBe(mockFile); + }); + + it('should request diff confirmation when requiresConfirmation is true', async () => { + const mockFile = createMockFile('note.md'); + const patch = `@@ -1,1 +1,1 @@ +-old ++new`; + const currentContent = 'old'; + const patchedContent = 'new'; + + mockVault.read.mockResolvedValue(currentContent); + mockDiffService.applyPatch.mockReturnValue(patchedContent); + mockDiffService.requestDiff.mockResolvedValue({ accepted: true }); + mockVault.process.mockResolvedValue(undefined); + + await vaultService.patch(mockFile, patch, false, true); + + expect(mockDiffService.requestDiff).toHaveBeenCalledWith( + mockFile.name, + mockFile.name, + currentContent, + patchedContent + ); + }); + + it('should handle multi-hunk patches', async () => { + const mockFile = createMockFile('document.md'); + const patch = `@@ -1,3 +1,3 @@ + # Title +-Old intro ++New intro + Content +@@ -10,2 +10,2 @@ + Section +-Old conclusion ++New conclusion`; + const currentContent = '# Title\nOld intro\nContent\n...\nSection\nOld conclusion'; + const patchedContent = '# Title\nNew intro\nContent\n...\nSection\nNew conclusion'; + + mockVault.read.mockResolvedValue(currentContent); + mockDiffService.applyPatch.mockReturnValue(patchedContent); + mockDiffService.requestDiff.mockResolvedValue({ accepted: true }); + mockVault.process.mockResolvedValue(undefined); + + const result = await vaultService.patch(mockFile, patch); + + expect(mockDiffService.applyPatch).toHaveBeenCalledWith(currentContent, patch); + expect(result).toBe(mockFile); + }); + + it('should call vault.process with function that returns patched content', async () => { + const mockFile = createMockFile('note.md'); + const patch = `@@ -1,1 +1,1 @@ +-old ++new`; + const patchedContent = 'new'; + let processCallback: any; + + mockVault.read.mockResolvedValue('old'); + mockDiffService.applyPatch.mockReturnValue(patchedContent); + mockDiffService.requestDiff.mockResolvedValue({ accepted: true }); + mockVault.process.mockImplementation((file, fn) => { + processCallback = fn; + return Promise.resolve(); + }); + + await vaultService.patch(mockFile, patch); + + expect(processCallback()).toBe(patchedContent); + }); + + it('should skip confirmation when requiresConfirmation is false', async () => { + const mockFile = createMockFile('note.md'); + const patch = `@@ -1,1 +1,1 @@ +-old ++new`; + + mockVault.read.mockResolvedValue('old'); + mockDiffService.applyPatch.mockReturnValue('new'); + mockVault.process.mockResolvedValue(undefined); + + // Clear previous calls + mockDiffService.requestDiff.mockClear(); + + await vaultService.patch(mockFile, patch, false, false); + + // When requiresConfirmation is false, requestDiff should not be called + // because proposeChange should skip the diff step + expect(mockVault.process).toHaveBeenCalled(); + }); + }); + describe('delete', () => { it('should delete file successfully when not excluded', async () => { const mockFile = createMockFile('note.md');