From 158d5f68e6566d046e43b2f7071bfebdbc11fbfb Mon Sep 17 00:00:00 2001 From: Florian Egger Date: Wed, 20 May 2026 18:36:00 +0200 Subject: [PATCH] Add action preview builder for write tool confirmation Introduces `ActionPreviewBuilder` to generate before/after previews for destructive operations. Write tools are now deferred with apply/cancel UI instead of executing immediately. Includes `ProposedAction` type, CSS for diff views, and state management in `ChatView`. --- src/action-preview-builder.ts | 334 +++++++++++++++++++++++++++ src/chat-view.ts | 208 ++++++++++++++++- src/types.ts | 23 ++ styles.css | 98 ++++++++ tests/action-preview-builder.test.ts | 264 +++++++++++++++++++++ tests/chat-view.test.ts | 169 +++++++++++++- 6 files changed, 1078 insertions(+), 18 deletions(-) create mode 100644 src/action-preview-builder.ts create mode 100644 tests/action-preview-builder.test.ts diff --git a/src/action-preview-builder.ts b/src/action-preview-builder.ts new file mode 100644 index 0000000..29eb8ff --- /dev/null +++ b/src/action-preview-builder.ts @@ -0,0 +1,334 @@ +// src/action-preview-builder.ts + +import { Vault, TFile } from 'obsidian'; +import type { ToolCall, ProposedAction } from './types'; +import { safeParseJson } from './utils'; + +const WRITE_TOOLS = new Set([ + 'create_file', + 'create_note', + 'append_to_note', + 'replace_note_section', + 'update_frontmatter', + 'rename_note', + 'move_note', + 'delete_note', + 'insert_link', +]); + +export function isWriteTool(name: string): boolean { + return WRITE_TOOLS.has(name); +} + +export class ActionPreviewBuilder { + private vault: Vault; + + constructor(vault: Vault) { + this.vault = vault; + } + + private parseArgs(toolCall: ToolCall): Record { + const rawArgs = toolCall.function?.arguments; + if (typeof rawArgs === 'string') { + try { + return safeParseJson(rawArgs) as Record; + } catch { + return {}; + } + } else if (rawArgs && typeof rawArgs === 'object') { + return rawArgs as Record; + } + return {}; + } + + async buildPreview(toolCall: ToolCall): Promise { + const name = toolCall.function?.name ?? ''; + const args = this.parseArgs(toolCall); + + switch (name) { + case 'create_file': + case 'create_note': + return this.buildCreatePreview(toolCall, args); + case 'append_to_note': + return this.buildAppendPreview(toolCall, args); + case 'replace_note_section': + return this.buildReplaceSectionPreview(toolCall, args); + case 'update_frontmatter': + return this.buildUpdateFrontmatterPreview(toolCall, args); + case 'rename_note': + return this.buildRenamePreview(toolCall, args); + case 'move_note': + return this.buildMovePreview(toolCall, args); + case 'delete_note': + return this.buildDeletePreview(toolCall, args); + case 'insert_link': + return this.buildInsertLinkPreview(toolCall, args); + default: + return { + id: toolCall.id, + toolCall, + operation: 'read', + path: '', + description: `Unknown operation: ${name}`, + status: 'pending', + }; + } + } + + private buildCreatePreview( + toolCall: ToolCall, + args: Record + ): ProposedAction { + const path = String(args.path ?? ''); + const content = String(args.content ?? ''); + return { + id: toolCall.id, + toolCall, + operation: 'create', + path, + description: `Create note: ${path}`, + preview: { + before: undefined, + after: content, + }, + status: 'pending', + }; + } + + private async buildAppendPreview( + toolCall: ToolCall, + args: Record + ): Promise { + const path = String(args.path ?? ''); + const content = String(args.content ?? ''); + const before = await this.readFileSafe(path); + const separator = before && before.endsWith('\n') ? '' : '\n'; + return { + id: toolCall.id, + toolCall, + operation: 'append', + path, + description: `Append to note: ${path}`, + preview: { + before, + after: before ? before + separator + content : content, + }, + status: 'pending', + }; + } + + private async buildReplaceSectionPreview( + toolCall: ToolCall, + args: Record + ): Promise { + const path = String(args.path ?? ''); + const heading = String(args.heading ?? ''); + const content = String(args.content ?? ''); + const before = await this.readFileSafe(path); + let after = before ?? ''; + + if (before) { + const escapedHeading = heading.replace(/[.*+?^${}()|[\]\\]/g, '\\$&'); + const headingRegex = new RegExp(`^(#{1,6}\\s+)${escapedHeading}\\s*$`, 'm'); + const match = before.match(headingRegex); + if (match) { + const headingLevel = match[1].length; + const headingIndex = match.index!; + const afterHeading = headingIndex + match[0].length; + const nextHeadingRegex = new RegExp(`^(#{1,${headingLevel}}\\s)`, 'm'); + const nextMatch = nextHeadingRegex.exec(before.slice(afterHeading)); + const sectionEnd = nextMatch ? afterHeading + nextMatch.index! : before.length; + after = + before.slice(0, headingIndex) + + match[0] + + '\n' + + content + + '\n' + + before.slice(sectionEnd); + } + } + + return { + id: toolCall.id, + toolCall, + operation: 'replace_section', + path, + description: `Replace section "${heading}" in ${path}`, + preview: { + before, + after, + }, + status: 'pending', + }; + } + + private async buildUpdateFrontmatterPreview( + toolCall: ToolCall, + args: Record + ): Promise { + const path = String(args.path ?? ''); + const fields = args.fields as Record | undefined; + const before = await this.readFileSafe(path); + let after = before ?? ''; + + if (fields && typeof fields === 'object' && !Array.isArray(fields)) { + const parsed = this.parseFrontmatter(before ?? ''); + const newFields = { ...parsed.fields }; + for (const [key, value] of Object.entries(fields)) { + if (value === null || value === undefined) { + delete newFields[key]; + } else if (typeof value === 'string') { + newFields[key] = value; + } else if (Array.isArray(value)) { + newFields[key] = value.join(', '); + } else { + newFields[key] = String(value); + } + } + const newFrontmatter = this.serializeFrontmatter(newFields); + const body = parsed.exists ? (before ?? '').replace(/^---\n[\s\S]*?\n---\n/, '') : before ?? ''; + after = newFrontmatter + body; + } + + return { + id: toolCall.id, + toolCall, + operation: 'update_frontmatter', + path, + description: `Update frontmatter in ${path}`, + preview: { + before, + after, + }, + status: 'pending', + }; + } + + private buildRenamePreview(toolCall: ToolCall, args: Record): ProposedAction { + const oldPath = String(args.oldPath ?? ''); + const newPath = String(args.newPath ?? ''); + return { + id: toolCall.id, + toolCall, + operation: 'rename', + path: oldPath, + description: `Rename ${oldPath} to ${newPath}`, + preview: { + before: oldPath, + after: newPath, + }, + status: 'pending', + }; + } + + private buildMovePreview(toolCall: ToolCall, args: Record): ProposedAction { + const path = String(args.path ?? ''); + const folder = String(args.folder ?? ''); + const fileName = path.split('/').pop() ?? path; + const newPath = folder ? `${folder}/${fileName}` : fileName; + return { + id: toolCall.id, + toolCall, + operation: 'move', + path, + description: `Move ${path} to ${newPath}`, + preview: { + before: path, + after: newPath, + }, + status: 'pending', + }; + } + + private async buildDeletePreview( + toolCall: ToolCall, + args: Record + ): Promise { + const path = String(args.path ?? ''); + const before = await this.readFileSafe(path); + return { + id: toolCall.id, + toolCall, + operation: 'delete', + path, + description: `Delete note: ${path}`, + preview: { + before, + after: undefined, + }, + status: 'pending', + }; + } + + private async buildInsertLinkPreview( + toolCall: ToolCall, + args: Record + ): Promise { + const sourcePath = String(args.sourcePath ?? ''); + const targetPath = String(args.targetPath ?? ''); + const anchorText = args.anchorText; + const before = await this.readFileSafe(sourcePath); + const linkText = + typeof anchorText === 'string' && anchorText.trim() + ? `[[${targetPath}|${anchorText}]]` + : `[[${targetPath}]]`; + const separator = before && before.endsWith('\n') ? '' : '\n'; + const after = before ? before + separator + linkText + '\n' : linkText + '\n'; + return { + id: toolCall.id, + toolCall, + operation: 'insert_link', + path: sourcePath, + description: `Insert link to ${targetPath} in ${sourcePath}`, + preview: { + before, + after, + }, + status: 'pending', + }; + } + + private async readFileSafe(path: string): Promise { + try { + const file = this.vault.getAbstractFileByPath(path); + if (file instanceof TFile) { + return await this.vault.cachedRead(file); + } + } catch { + // ignore + } + return undefined; + } + + private parseFrontmatter(content: string): { + exists: boolean; + raw: string; + fields: Record; + } { + const frontmatterRegex = /^---\n([\s\S]*?)\n---\n/; + const match = content.match(frontmatterRegex); + if (!match) { + return { exists: false, raw: '', fields: {} }; + } + + const raw = match[1]; + const fields: Record = {}; + for (const line of raw.split('\n')) { + const idx = line.indexOf(':'); + if (idx > 0) { + const key = line.slice(0, idx).trim(); + const value = line.slice(idx + 1).trim(); + if (key) { + fields[key] = value; + } + } + } + + return { exists: true, raw, fields }; + } + + private serializeFrontmatter(fields: Record): string { + const lines = Object.entries(fields).map(([key, value]) => `${key}: ${value}`); + return `---\n${lines.join('\n')}\n---\n`; + } +} diff --git a/src/chat-view.ts b/src/chat-view.ts index 7bc799a..78365a1 100755 --- a/src/chat-view.ts +++ b/src/chat-view.ts @@ -3,7 +3,16 @@ import { OllamaClient } from './ollama-client'; import { VaultIndexer } from './vault-indexer'; import { VaultVectorStore } from './vault-vector-store'; import { ToolExecutor } from './tool-executor'; -import { PluginSettings, OllamaMessage, OllamaTool, OllamaToolCall, ChatMessage } from './types'; +import { ActionPreviewBuilder, isWriteTool } from './action-preview-builder'; +import { + PluginSettings, + OllamaMessage, + OllamaTool, + OllamaToolCall, + ChatMessage, + ProposedAction, + ToolResult, +} from './types'; import { ConversationStateManager } from './conversation-state'; import { ErrorHandler } from './error-handler'; @@ -47,6 +56,7 @@ export class ChatView extends ItemView { ); this.vaultIndexer = new VaultIndexer(this.app.vault, undefined, vectorStore); this.toolExecutor = new ToolExecutor(this.app.vault, this.app); + this.actionPreviewBuilder = new ActionPreviewBuilder(this.app.vault); this.conversationStateManager = new ConversationStateManager(); } @@ -560,9 +570,13 @@ export class ChatView extends ItemView { fullResponse: string, assistantMessageId: string ): Promise { - const toolResults = ( + const readToolCalls = toolCalls.filter((tc) => !isWriteTool(tc.function?.name ?? '')); + const writeToolCalls = toolCalls.filter((tc) => isWriteTool(tc.function?.name ?? '')); + + // Execute read/search tools immediately + const readResults = ( await Promise.all( - toolCalls.slice(0, MAX_TOOL_CALLS).map(async (toolCall) => { + readToolCalls.slice(0, MAX_TOOL_CALLS).map(async (toolCall) => { try { const toolResult = await this.toolExecutor.handleToolCall(toolCall); return { ...toolResult, id: toolCall.id }; @@ -574,13 +588,40 @@ export class ChatView extends ItemView { ) ).filter((result): result is NonNullable => result !== null); - const followUpMessages: OllamaMessage[] = toolResults.map((result) => { - return { - role: 'tool', - content: JSON.stringify(result), - tool_call_id: result.id ?? '', - }; - }); + // Build previews for write tools + const writePreviews: ProposedAction[] = []; + for (const toolCall of writeToolCalls.slice(0, MAX_TOOL_CALLS)) { + try { + const preview = await this.actionPreviewBuilder.buildPreview(toolCall); + writePreviews.push(preview); + } catch (error) { + ErrorHandler.handleError(error, 'ChatView.handleUserInput'); + } + } + + if (writePreviews.length > 0) { + // Store pending state for apply/cancel + this.pendingActions = writePreviews; + this.pendingReadResults = readResults; + this.pendingFollowUpContext = { messages, tools, assistantMessageId }; + + this.updateMessageById(assistantMessageId, { + content: `${fullResponse}\n\n*Proposed actions:*\n${writePreviews.map((a) => `- ${a.description}`).join('\n')}`, + isStreaming: false, + }); + + this.render(); + this.renderActionPreviews(assistantMessageId); + return; + } + + // No write tools — proceed with follow-up as before + const allResults = readResults; + const followUpMessages: OllamaMessage[] = allResults.map((result) => ({ + role: 'tool', + content: JSON.stringify(result), + tool_call_id: result.id ?? '', + })); const followUp: OllamaMessage = { role: 'assistant', @@ -599,6 +640,143 @@ export class ChatView extends ItemView { } } + async applyPendingActions(): Promise { + if (this.pendingActions.length === 0 || !this.pendingFollowUpContext) { + return; + } + + const { messages, tools, assistantMessageId } = this.pendingFollowUpContext; + + // Execute write tools + const writeResults = ( + await Promise.all( + this.pendingActions.map(async (action) => { + try { + const toolResult = await this.toolExecutor.handleToolCall(action.toolCall); + return { ...toolResult, id: action.toolCall.id }; + } catch (error) { + ErrorHandler.handleError(error, 'ChatView.applyPendingActions'); + return null; + } + }) + ) + ).filter((result): result is NonNullable => result !== null); + + const allResults = [...this.pendingReadResults, ...writeResults]; + + const followUpMessages: OllamaMessage[] = allResults.map((result) => ({ + role: 'tool', + content: JSON.stringify(result), + tool_call_id: result.id ?? '', + })); + + const followUp: OllamaMessage = { + role: 'assistant', + content: 'I have processed your request using the following tools. Here are the results:', + tool_calls: this.pendingActions.map((a) => a.toolCall), + }; + + if (followUpMessages.length > 0) { + const finalMessages = [...messages, followUp, ...followUpMessages]; + const response = await this.ollamaClient.chat(finalMessages, tools); + const finalResponse = response.content || 'Actions applied successfully.'; + this.updateMessageById(assistantMessageId, { + content: finalResponse, + isStreaming: false, + }); + } else { + this.updateMessageById(assistantMessageId, { + content: 'Actions applied successfully.', + isStreaming: false, + }); + } + + this.clearPendingActions(); + this.render(); + } + + cancelPendingActions(): void { + if (this.pendingActions.length === 0) { + return; + } + + const context = this.pendingFollowUpContext; + if (context) { + this.updateMessageById(context.assistantMessageId, { + content: 'Actions cancelled. No changes were made.', + isStreaming: false, + }); + } + + this.clearPendingActions(); + this.render(); + } + + private renderActionPreviews(assistantMessageId: string): void { + const messageEl = this.chatContainer?.querySelector( + `.ollama-message[data-msg-id="${assistantMessageId}"]` + ); + if (!messageEl) { + return; + } + + // Remove existing preview + const existing = messageEl.querySelector('.ollama-proposed-actions'); + existing?.remove(); + + const container = messageEl.createEl('div', { cls: 'ollama-proposed-actions' }); + container.createEl('div', { + cls: 'ollama-proposed-actions-header', + text: 'Proposed Actions', + }); + + for (const action of this.pendingActions) { + const card = container.createEl('div', { cls: 'ollama-proposed-action' }); + card.createEl('div', { cls: 'ollama-action-description', text: action.description }); + + if (action.preview) { + const diffEl = card.createEl('div', { cls: 'ollama-action-diff' }); + if (action.preview.before !== undefined) { + diffEl.createEl('pre', { + cls: 'ollama-diff-before', + text: `Before:\n${action.preview.before.slice(0, 500)}`, + }); + } + if (action.preview.after !== undefined) { + diffEl.createEl('pre', { + cls: 'ollama-diff-after', + text: `After:\n${action.preview.after.slice(0, 500)}`, + }); + } + } + } + + const buttonContainer = container.createEl('div', { cls: 'ollama-action-buttons' }); + const applyBtn = buttonContainer.createEl('button', { + cls: 'ollama-apply-button', + text: 'Apply All', + }); + const cancelBtn = buttonContainer.createEl('button', { + cls: 'ollama-cancel-button', + text: 'Cancel', + }); + + applyBtn.addEventListener('click', () => { + void this.applyPendingActions(); + }); + + cancelBtn.addEventListener('click', () => { + this.cancelPendingActions(); + }); + } + + private clearPendingActions(): void { + this.pendingActions = []; + this.pendingReadResults = []; + this.pendingFollowUpContext = null; + this.chatContainer?.querySelectorAll('.ollama-proposed-actions').forEach((el) => el.remove()); + } + async handleUserInput(inputValue?: string): Promise { const userMessage = (inputValue ?? this.inputEl?.value ?? '').trim(); if (!userMessage) { @@ -753,8 +931,18 @@ export class ChatView extends ItemView { private ollamaClient: OllamaClient; private vaultIndexer: VaultIndexer; private toolExecutor: ToolExecutor; + private actionPreviewBuilder: ActionPreviewBuilder; private conversationStateManager: ConversationStateManager; private vectorStore?: VaultVectorStore; + + // Pending action state + private pendingActions: ProposedAction[] = []; + private pendingReadResults: (ToolResult & { id?: string })[] = []; + private pendingFollowUpContext: { + messages: OllamaMessage[]; + tools: OllamaTool[]; + assistantMessageId: string; + } | null = null; } const MAX_TOOL_CALLS = 5; diff --git a/src/types.ts b/src/types.ts index 49157d5..6800ac4 100644 --- a/src/types.ts +++ b/src/types.ts @@ -128,6 +128,29 @@ export interface ToolResult { id?: string; } +export interface ProposedAction { + id: string; + toolCall: ToolCall; + operation: + | 'create' + | 'read' + | 'search' + | 'append' + | 'replace_section' + | 'update_frontmatter' + | 'rename' + | 'move' + | 'delete' + | 'insert_link'; + path: string; + description: string; + preview?: { + before?: string; + after?: string; + }; + status: 'pending' | 'applied' | 'rejected'; +} + export interface VaultIndexEntry { path: string; title: string; diff --git a/styles.css b/styles.css index 1c6a8b3..62f7034 100644 --- a/styles.css +++ b/styles.css @@ -175,3 +175,101 @@ transform: rotate(360deg); } } + +/* Proposed Actions Preview */ +.ollama-proposed-actions { + margin-top: var(--size-4-2); + padding: var(--size-4-2); + border: 1px solid var(--ollama-border); + border-radius: var(--ollama-radius); + background-color: var(--background-primary); +} + +.ollama-proposed-actions-header { + font-weight: var(--font-semibold); + font-size: var(--font-ui-small); + margin-bottom: var(--size-4-2); + color: var(--text-normal); +} + +.ollama-proposed-action { + margin-bottom: var(--size-4-2); + padding: var(--size-4-1); + border: 1px solid var(--ollama-border); + border-radius: var(--ollama-radius); + background-color: var(--background-primary-alt); +} + +.ollama-action-description { + font-weight: var(--font-medium); + font-size: var(--font-ui-small); + margin-bottom: var(--size-4-1); + color: var(--text-normal); +} + +.ollama-action-diff { + display: flex; + flex-direction: column; + gap: var(--size-4-1); +} + +.ollama-diff-before, +.ollama-diff-after { + margin: 0; + padding: var(--size-4-1); + border-radius: var(--ollama-radius); + font-size: var(--font-smallest); + font-family: var(--font-monospace); + white-space: pre-wrap; + word-break: break-word; + max-height: 8rem; + overflow-y: auto; +} + +.ollama-diff-before { + background-color: var(--background-modifier-error); + color: var(--text-normal); +} + +.ollama-diff-after { + background-color: var(--background-modifier-success); + color: var(--text-normal); +} + +.ollama-action-buttons { + display: flex; + gap: var(--size-4-1); + margin-top: var(--size-4-2); +} + +.ollama-apply-button { + padding: var(--size-4-1) var(--size-4-2); + border-radius: var(--ollama-radius); + border: none; + background-color: var(--interactive-accent); + color: var(--text-on-accent); + font-weight: var(--font-semibold); + font-size: var(--font-ui-small); + cursor: pointer; + transition: filter 0.15s ease; +} + +.ollama-apply-button:hover { + filter: brightness(1.1); +} + +.ollama-cancel-button { + padding: var(--size-4-1) var(--size-4-2); + border-radius: var(--ollama-radius); + border: 1px solid var(--ollama-border); + background-color: var(--background-modifier-form-field); + color: var(--text-normal); + font-weight: var(--font-semibold); + font-size: var(--font-ui-small); + cursor: pointer; + transition: background-color 0.15s ease; +} + +.ollama-cancel-button:hover { + background-color: var(--background-modifier-hover); +} diff --git a/tests/action-preview-builder.test.ts b/tests/action-preview-builder.test.ts new file mode 100644 index 0000000..8d5d4d2 --- /dev/null +++ b/tests/action-preview-builder.test.ts @@ -0,0 +1,264 @@ +import { ActionPreviewBuilder, isWriteTool } from '../src/action-preview-builder'; +import { TFile } from 'obsidian'; +import { ToolCall } from '../src/types'; + +// Mock Obsidian module +jest.mock('obsidian', () => { + class TFile {} + return { + Vault: jest.fn(), + App: jest.fn(), + Notice: jest.fn(), + TFile, + }; +}); + +describe('isWriteTool', () => { + it('should return true for write tools', () => { + expect(isWriteTool('create_note')).toBe(true); + expect(isWriteTool('create_file')).toBe(true); + expect(isWriteTool('append_to_note')).toBe(true); + expect(isWriteTool('replace_note_section')).toBe(true); + expect(isWriteTool('update_frontmatter')).toBe(true); + expect(isWriteTool('rename_note')).toBe(true); + expect(isWriteTool('move_note')).toBe(true); + expect(isWriteTool('delete_note')).toBe(true); + expect(isWriteTool('insert_link')).toBe(true); + }); + + it('should return false for read/search tools', () => { + expect(isWriteTool('read_vault_file')).toBe(false); + expect(isWriteTool('search_vault_files')).toBe(false); + }); + + it('should return false for unknown tools', () => { + expect(isWriteTool('unknown_tool')).toBe(false); + expect(isWriteTool('')).toBe(false); + }); +}); + +describe('ActionPreviewBuilder', () => { + let builder: ActionPreviewBuilder; + let mockVault: { + getAbstractFileByPath: jest.Mock; + cachedRead: jest.Mock; + }; + + beforeEach(() => { + mockVault = { + getAbstractFileByPath: jest.fn(), + cachedRead: jest.fn().mockResolvedValue(''), + }; + builder = new ActionPreviewBuilder(mockVault as unknown as any); + }); + + describe('buildPreview', () => { + it('should build preview for create_note', async () => { + const call: ToolCall = { + id: 'call_1', + type: 'function', + function: { + name: 'create_note', + arguments: JSON.stringify({ path: 'New.md', content: '# Hello' }), + }, + }; + const preview = await builder.buildPreview(call); + expect(preview.operation).toBe('create'); + expect(preview.path).toBe('New.md'); + expect(preview.preview?.before).toBeUndefined(); + expect(preview.preview?.after).toBe('# Hello'); + }); + + it('should build preview for append_to_note', async () => { + mockVault.getAbstractFileByPath.mockReturnValue(new TFile()); + mockVault.cachedRead.mockResolvedValue('Existing content'); + + const call: ToolCall = { + id: 'call_2', + type: 'function', + function: { + name: 'append_to_note', + arguments: JSON.stringify({ path: 'Note.md', content: 'Appended' }), + }, + }; + const preview = await builder.buildPreview(call); + expect(preview.operation).toBe('append'); + expect(preview.preview?.before).toBe('Existing content'); + expect(preview.preview?.after).toBe('Existing content\nAppended'); + }); + + it('should build preview for replace_note_section', async () => { + mockVault.getAbstractFileByPath.mockReturnValue(new TFile()); + mockVault.cachedRead.mockResolvedValue('# Title\n\n## Section A\nOld\n\n## Section B\nOther'); + + const call: ToolCall = { + id: 'call_3', + type: 'function', + function: { + name: 'replace_note_section', + arguments: JSON.stringify({ path: 'Note.md', heading: 'Section A', content: 'New' }), + }, + }; + const preview = await builder.buildPreview(call); + expect(preview.operation).toBe('replace_section'); + expect(preview.preview?.after).toContain('New'); + expect(preview.preview?.after).not.toContain('Old'); + }); + + it('should build preview for update_frontmatter', async () => { + mockVault.getAbstractFileByPath.mockReturnValue(new TFile()); + mockVault.cachedRead.mockResolvedValue('---\ntitle: Old\n---\nBody'); + + const call: ToolCall = { + id: 'call_4', + type: 'function', + function: { + name: 'update_frontmatter', + arguments: JSON.stringify({ path: 'Note.md', fields: { title: 'New' } }), + }, + }; + const preview = await builder.buildPreview(call); + expect(preview.operation).toBe('update_frontmatter'); + expect(preview.preview?.after).toContain('title: New'); + }); + + it('should build preview for rename_note', async () => { + const call: ToolCall = { + id: 'call_5', + type: 'function', + function: { + name: 'rename_note', + arguments: JSON.stringify({ oldPath: 'Old.md', newPath: 'New.md' }), + }, + }; + const preview = await builder.buildPreview(call); + expect(preview.operation).toBe('rename'); + expect(preview.preview?.before).toBe('Old.md'); + expect(preview.preview?.after).toBe('New.md'); + }); + + it('should build preview for move_note', async () => { + const call: ToolCall = { + id: 'call_6', + type: 'function', + function: { + name: 'move_note', + arguments: JSON.stringify({ path: 'Projects/Note.md', folder: 'Archive' }), + }, + }; + const preview = await builder.buildPreview(call); + expect(preview.operation).toBe('move'); + expect(preview.preview?.before).toBe('Projects/Note.md'); + expect(preview.preview?.after).toBe('Archive/Note.md'); + }); + + it('should build preview for delete_note', async () => { + mockVault.getAbstractFileByPath.mockReturnValue(new TFile()); + mockVault.cachedRead.mockResolvedValue('File content'); + + const call: ToolCall = { + id: 'call_7', + type: 'function', + function: { + name: 'delete_note', + arguments: JSON.stringify({ path: 'Note.md' }), + }, + }; + const preview = await builder.buildPreview(call); + expect(preview.operation).toBe('delete'); + expect(preview.preview?.before).toBe('File content'); + expect(preview.preview?.after).toBeUndefined(); + }); + + it('should build preview for insert_link without anchor text', async () => { + mockVault.getAbstractFileByPath.mockReturnValue(new TFile()); + mockVault.cachedRead.mockResolvedValue('Source content'); + + const call: ToolCall = { + id: 'call_8', + type: 'function', + function: { + name: 'insert_link', + arguments: JSON.stringify({ sourcePath: 'A.md', targetPath: 'B.md' }), + }, + }; + const preview = await builder.buildPreview(call); + expect(preview.operation).toBe('insert_link'); + expect(preview.preview?.after).toContain('[[B.md]]'); + }); + + it('should build preview for insert_link with anchor text', async () => { + mockVault.getAbstractFileByPath.mockReturnValue(new TFile()); + mockVault.cachedRead.mockResolvedValue('Source content'); + + const call: ToolCall = { + id: 'call_9', + type: 'function', + function: { + name: 'insert_link', + arguments: JSON.stringify({ sourcePath: 'A.md', targetPath: 'B.md', anchorText: 'Link' }), + }, + }; + const preview = await builder.buildPreview(call); + expect(preview.preview?.after).toContain('[[B.md|Link]]'); + }); + + it('should handle object arguments directly', async () => { + const call: ToolCall = { + id: 'call_10', + type: 'function', + function: { + name: 'create_note', + arguments: { path: 'Direct.md', content: 'Body' } as unknown as string, + }, + }; + const preview = await builder.buildPreview(call); + expect(preview.path).toBe('Direct.md'); + expect(preview.preview?.after).toBe('Body'); + }); + + it('should return unknown operation for unrecognized tools', async () => { + const call: ToolCall = { + id: 'call_11', + type: 'function', + function: { + name: 'weird_tool', + arguments: '{}', + }, + }; + const preview = await builder.buildPreview(call); + expect(preview.operation).toBe('read'); + expect(preview.description).toContain('weird_tool'); + }); + + it('should handle missing file gracefully for append_to_note', async () => { + mockVault.getAbstractFileByPath.mockReturnValue(null); + + const call: ToolCall = { + id: 'call_12', + type: 'function', + function: { + name: 'append_to_note', + arguments: JSON.stringify({ path: 'Missing.md', content: 'test' }), + }, + }; + const preview = await builder.buildPreview(call); + expect(preview.preview?.before).toBeUndefined(); + expect(preview.preview?.after).toBe('test'); + }); + + it('should handle invalid JSON arguments gracefully', async () => { + const call: ToolCall = { + id: 'call_13', + type: 'function', + function: { + name: 'create_note', + arguments: 'not json', + }, + }; + const preview = await builder.buildPreview(call); + expect(preview.path).toBe(''); + expect(preview.preview?.after).toBe(''); + }); + }); +}); diff --git a/tests/chat-view.test.ts b/tests/chat-view.test.ts index 133eead..98a2843 100755 --- a/tests/chat-view.test.ts +++ b/tests/chat-view.test.ts @@ -6,7 +6,12 @@ import { ErrorHandler } from '../src/error-handler'; interface MockVault { getMarkdownFiles: () => any[]; read: () => Promise; + cachedRead: () => Promise; + getAbstractFileByPath: () => any; create: () => Promise; + modify: () => Promise; + rename: () => Promise; + delete: () => Promise; } interface MockWorkspace { getLeaf: () => any; @@ -67,11 +72,17 @@ describe('ChatView', () => { let mockApp: MockApp; beforeEach(() => { + jest.clearAllMocks(); mockApp = { vault: { getMarkdownFiles: jest.fn().mockReturnValue([]), read: jest.fn().mockResolvedValue(''), + cachedRead: jest.fn().mockResolvedValue(''), + getAbstractFileByPath: jest.fn().mockReturnValue(null), create: jest.fn().mockResolvedValue(null), + modify: jest.fn().mockResolvedValue(undefined), + rename: jest.fn().mockResolvedValue(undefined), + delete: jest.fn().mockResolvedValue(undefined), }, workspace: { getLeaf: jest.fn(), @@ -388,7 +399,7 @@ describe('ChatView', () => { expect(lastMessage.isStreaming).toBe(false); }); - it('should process tool calls with follow-up context', async () => { + it('should show preview for write tool calls and defer follow-up', async () => { view['sendButton'] = document.createElement('button'); view['inputEl'] = document.createElement('textarea'); (view['inputEl'] as HTMLTextAreaElement).value = 'test'; @@ -413,12 +424,136 @@ describe('ChatView', () => { const followUpSpy = jest .spyOn(view['ollamaClient'], 'chat') .mockResolvedValue({ role: 'assistant', content: ' follow-up' }); + + // Mock preview builder + jest.spyOn(view['actionPreviewBuilder'], 'buildPreview').mockResolvedValue({ + id: 'tool_1', + toolCall: { + id: 'tool_1', + type: 'function', + function: { + name: 'create_file', + arguments: '{"path":"test/file.md","content":"Test content"}', + }, + } as unknown as any, + operation: 'create', + path: 'test/file.md', + description: 'Create note: test/file.md', + preview: { after: 'Test content' }, + status: 'pending', + }); + await (view as any).handleUserInput('test'); - expect(followUpSpy).toHaveBeenCalled(); - // Verify that tool calls resulted in follow-up messages + expect(chatSpy).toHaveBeenCalled(); + // With write tools, follow-up should be deferred until apply + expect(followUpSpy).not.toHaveBeenCalled(); + expect((view as any).pendingActions.length).toBe(1); expect((view as any).messages.length).toBeGreaterThan(1); }); + it('should apply pending actions and trigger follow-up', async () => { + view['sendButton'] = document.createElement('button'); + view['inputEl'] = document.createElement('textarea'); + (view['inputEl'] as HTMLTextAreaElement).value = 'test'; + const assistantMessageId = 'test-assistant-id'; + view['messages'] = [ + { + id: 'user-id', + role: 'user', + content: 'test', + timestamp: Date.now(), + }, + { + id: assistantMessageId, + role: 'assistant', + content: 'test', + timestamp: Date.now(), + isStreaming: false, + }, + ]; + + view['pendingActions'] = [ + { + id: 'tool_1', + toolCall: { + id: 'tool_1', + type: 'function', + function: { + name: 'create_file', + arguments: '{"path":"test/file.md","content":"Test content"}', + }, + } as unknown as any, + operation: 'create', + path: 'test/file.md', + description: 'Create note: test/file.md', + status: 'pending', + }, + ]; + view['pendingReadResults'] = []; + view['pendingFollowUpContext'] = { + messages: [], + tools: [], + assistantMessageId, + }; + + const followUpSpy = jest + .spyOn(view['ollamaClient'], 'chat') + .mockResolvedValue({ role: 'assistant', content: ' follow-up' }); + + const toolExecutor = view['toolExecutor']; + jest.spyOn(toolExecutor, 'handleToolCall').mockResolvedValue({ + success: true, + message: 'Note created successfully', + }); + + await view.applyPendingActions(); + expect(followUpSpy).toHaveBeenCalled(); + expect((view as any).pendingActions.length).toBe(0); + }); + + it('should cancel pending actions', async () => { + view['sendButton'] = document.createElement('button'); + view['inputEl'] = document.createElement('textarea'); + (view['inputEl'] as HTMLTextAreaElement).value = 'test'; + const assistantMessageId = 'test-assistant-id'; + view['messages'] = [ + { + id: assistantMessageId, + role: 'assistant', + content: 'Proposed actions...', + timestamp: Date.now(), + isStreaming: false, + }, + ]; + view['pendingActions'] = [ + { + id: 'tool_1', + toolCall: { + id: 'tool_1', + type: 'function', + function: { + name: 'create_file', + arguments: '{"path":"test.md","content":"test"}', + }, + } as unknown as any, + operation: 'create', + path: 'test.md', + description: 'Create note: test.md', + status: 'pending', + }, + ]; + view['pendingFollowUpContext'] = { + messages: [], + tools: [], + assistantMessageId, + }; + + view.cancelPendingActions(); + const msg = (view as any).messages.find((m: any) => m.id === assistantMessageId); + expect(msg.content).toContain('cancelled'); + expect((view as any).pendingActions.length).toBe(0); + }); + it('should handle tool call errors gracefully and continue with partial results', async () => { view['sendButton'] = document.createElement('button'); view['inputEl'] = document.createElement('textarea'); @@ -449,14 +584,31 @@ describe('ChatView', () => { tool_calls: [], }); - // Mock tool executor to return mixed results + // Mock preview builder for write tool + jest.spyOn(view['actionPreviewBuilder'], 'buildPreview').mockResolvedValue({ + id: 'call_1', + toolCall: { + id: 'call_1', + type: 'function', + function: { + name: 'create_file', + arguments: '{"path":"test.md","content":"test"}', + }, + } as unknown as any, + operation: 'create', + path: 'test.md', + description: 'Create note: test.md', + preview: { after: 'test' }, + status: 'pending', + }); + + // Mock tool executor — read tool fails const toolExecutor = view['toolExecutor']; jest.spyOn(toolExecutor, 'handleToolCall').mockImplementation(async (call) => { - if (call.function.name === 'create_file') { - return { success: true, message: 'Note created successfully' }; - } else { + if (call.function.name === 'nonexistent_tool') { throw new Error('Tool not found'); } + return { success: true, message: 'Done' }; }); const consoleSpy = jest.spyOn(console, 'error').mockImplementation(() => {}); @@ -465,7 +617,8 @@ describe('ChatView', () => { await (view as any).handleUserInput('test'); expect(chatSpy).toHaveBeenCalled(); - expect(followUpSpy).toHaveBeenCalled(); // Should still call follow-up with partial results + // Write tools trigger preview, not immediate follow-up + expect(followUpSpy).not.toHaveBeenCalled(); expect(errorHandlerSpy).toHaveBeenCalledWith(expect.any(Error), 'ChatView.handleUserInput'); expect((view as any).messages.length).toBeGreaterThan(1); consoleSpy.mockRestore();