From f7ff10699d71220c840fdde63625db75ed545fc1 Mon Sep 17 00:00:00 2001 From: Andrew Beal Date: Fri, 5 Dec 2025 18:39:01 +0000 Subject: [PATCH] Add PatchVaultFile function for incremental file updates Implements unified diff patching as an alternative to WriteVaultFile for making targeted changes without replacing entire files. Includes new PatchVaultFile AI function, DiffService.applyPatch method, and comprehensive test coverage. --- .../AIFunctionDefinitions.ts | 2 + .../Functions/PatchVaultFile.ts | 54 ++++ .../Functions/WriteVaultFile.ts | 10 +- AIClasses/Schemas/AIFunctionSchemas.ts | 7 + Enums/AIFunction.ts | 1 + Services/AIFunctionService.ts | 23 +- Services/ChatService.ts | 2 +- Services/DiffService.ts | 9 + Services/FileSystemService.ts | 18 ++ Services/VaultService.ts | 28 +- Views/DiffView.ts | 6 + __tests__/Services/AIFunctionService.test.ts | 261 ++++++++++++++++++ __tests__/Services/DiffService.test.ts | 125 +++++++++ __tests__/Services/FileSystemService.test.ts | 137 +++++++++ __tests__/Services/VaultService.test.ts | 187 ++++++++++++- 15 files changed, 861 insertions(+), 9 deletions(-) create mode 100644 AIClasses/FunctionDefinitions/Functions/PatchVaultFile.ts 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');