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`.
This commit is contained in:
@@ -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('');
|
||||
});
|
||||
});
|
||||
});
|
||||
+161
-8
@@ -6,7 +6,12 @@ import { ErrorHandler } from '../src/error-handler';
|
||||
interface MockVault {
|
||||
getMarkdownFiles: () => any[];
|
||||
read: () => Promise<string>;
|
||||
cachedRead: () => Promise<string>;
|
||||
getAbstractFileByPath: () => any;
|
||||
create: () => Promise<any>;
|
||||
modify: () => Promise<void>;
|
||||
rename: () => Promise<void>;
|
||||
delete: () => Promise<void>;
|
||||
}
|
||||
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();
|
||||
|
||||
Reference in New Issue
Block a user