From b7b3a185a0782f8f577bfa9c1d5c0fa9cf148a10 Mon Sep 17 00:00:00 2001 From: Florian Egger Date: Thu, 21 May 2026 20:35:05 +0200 Subject: [PATCH] Add automatic folder creation for file operations and improve chat session handling Implement `ensureFolderExists` in `ToolExecutor` to create parent directories when creating, moving, or renaming notes. Update `ChatView` to properly persist agent mode and derive session titles from the first user message. Fix error handling in tool execution to return structured failures instead of null, and include failure details in follow-up messages. --- __mocks__/obsidian.ts | 10 +++++ main.js | 89 ++++++++++++++++++++++++++++++------- src/chat-view.ts | 54 ++++++++++++++++++---- src/tool-executor.ts | 32 ++++++++++++- tests/chat-view.test.ts | 16 +++++++ tests/tool-executor.test.ts | 31 ++++++++++++- 6 files changed, 204 insertions(+), 28 deletions(-) diff --git a/__mocks__/obsidian.ts b/__mocks__/obsidian.ts index fc05d12..292136f 100755 --- a/__mocks__/obsidian.ts +++ b/__mocks__/obsidian.ts @@ -5,11 +5,13 @@ export class Vault { getMarkdownFiles: () => any[]; read: (file: any) => Promise; create: (path: string, content: string) => Promise; + createFolder: (path: string) => Promise; constructor() { this.getMarkdownFiles = () => []; this.read = async () => ''; this.create = async () => null; + this.createFolder = async () => null; } } @@ -113,6 +115,14 @@ export interface TFile { path: string; } +export class TFolder { + path: string; + + constructor(path: string = '') { + this.path = path; + } +} + // Plugin class (used by main.ts) export class Plugin { app: App; diff --git a/main.js b/main.js index 957ad28..f161a24 100644 --- a/main.js +++ b/main.js @@ -8968,6 +8968,30 @@ var ToolExecutor = class { const file = this.getFile(path); await this.vault.modify(file, content); } + getParentFolderPath(path) { + const parts = path.split("/").filter((part) => part.length > 0); + parts.pop(); + return parts.join("/"); + } + async ensureFolderExists(folderPath) { + const normalizedFolder = folderPath.replace(/\/$/, "").trim(); + if (!normalizedFolder) { + return; + } + const parts = normalizedFolder.split("/").filter((part) => part.length > 0); + let currentPath = ""; + for (const part of parts) { + currentPath = currentPath ? `${currentPath}/${part}` : part; + const existing = this.vault.getAbstractFileByPath(currentPath); + if (existing) { + if (!(existing instanceof import_obsidian.TFolder)) { + throw new Error(`Cannot create folder ${currentPath}: a file already exists at that path`); + } + continue; + } + await this.vault.createFolder(currentPath); + } + } async handleToolCall(toolCall) { const startTime = Date.now(); const toolName = toolCall.function?.name ?? "unknown"; @@ -9062,6 +9086,7 @@ var ToolExecutor = class { throw new Error("Invalid file path detected"); } try { + await this.ensureFolderExists(this.getParentFolderPath(path)); await this.vault.create(path, content); return { success: true, message: "Note created successfully" }; } catch (error) { @@ -9285,6 +9310,7 @@ ${lines.join("\n")} throw new Error("Invalid file path detected"); } const file = this.getFile(oldPath); + await this.ensureFolderExists(this.getParentFolderPath(newPath)); await this.vault.rename(file, newPath); return { success: true, message: `Note renamed from ${oldPath} to ${newPath}` }; } @@ -9307,6 +9333,7 @@ ${lines.join("\n")} const file = this.getFile(path); const fileName = file.name; const newPath = normalizedFolder ? `${normalizedFolder}/${fileName}` : fileName; + await this.ensureFolderExists(normalizedFolder); await this.vault.rename(file, newPath); return { success: true, message: `Note moved to ${newPath}` }; } @@ -11022,6 +11049,7 @@ var ChatView = class extends import_obsidian5.ItemView { newSettings.agentModel ?? newSettings.model, { cacheConfig: newSettings.cacheConfig } ); + this.conversationStateManager.setSystemPrompt(getSystemPromptForMode(this.currentAgentMode)); void this.initializeClientCaches().catch(() => { new import_obsidian5.Notice( "Semantic cache unavailable: could not connect to ChromaDB. Check the ChromaDB URL in settings." @@ -11171,7 +11199,7 @@ var ChatView = class extends import_obsidian5.ItemView { } } this.modeSelectorEl.addEventListener("change", () => { - this.currentAgentMode = this.modeSelectorEl.value; + this.setAgentMode(this.modeSelectorEl.value); }); } else { newChatContainer.appendChild(this.modeSelectorEl); @@ -11368,6 +11396,10 @@ var ChatView = class extends import_obsidian5.ItemView { this.modeSelectorEl.value = mode; } this.conversationStateManager.setSystemPrompt(getSystemPromptForMode(mode)); + const activeId = this.chatHistoryManager?.getActiveSessionId(); + if (activeId) { + this.chatHistoryManager?.updateSession(activeId, { agentMode: mode }); + } } clearConversation() { this.saveCurrentSession(); @@ -11409,14 +11441,28 @@ var ChatView = class extends import_obsidian5.ItemView { const activeId = this.chatHistoryManager.getActiveSessionId(); if (!activeId) return; const nonStreamingMessages = this.messages.filter((msg) => !msg.isStreaming); - this.chatHistoryManager.updateSessionMessages(activeId, nonStreamingMessages); + this.chatHistoryManager.updateSession(activeId, { + messages: nonStreamingMessages, + agentMode: this.currentAgentMode, + title: this.deriveSessionTitle(nonStreamingMessages) + }); } syncMessagesToSession() { if (!this.chatHistoryManager) return; const activeId = this.chatHistoryManager.getActiveSessionId(); if (!activeId) return; const nonStreamingMessages = this.messages.filter((msg) => !msg.isStreaming); - this.chatHistoryManager.updateSessionMessages(activeId, nonStreamingMessages); + this.chatHistoryManager.updateSession(activeId, { + messages: nonStreamingMessages, + agentMode: this.currentAgentMode, + title: this.deriveSessionTitle(nonStreamingMessages) + }); + } + deriveSessionTitle(messages) { + const firstUser = messages.find((m) => m.role === "user"); + const text = firstUser?.content.trim() ?? ""; + if (!text) return "New Chat"; + return text.length > 40 ? text.slice(0, 40) + "\u2026" : text; } populateHistoryDropdown() { if (!this.historySelectEl) return; @@ -11785,7 +11831,7 @@ var ChatView = class extends import_obsidian5.ItemView { if (writePreviews.length > 0 && modeRequiresPreview(this.currentAgentMode)) { this.pendingActions = writePreviews; this.pendingReadResults = readResults; - this.pendingFollowUpContext = { messages, tools, assistantMessageId }; + this.pendingFollowUpContext = { messages, tools, assistantMessageId, allToolCalls: toolCalls, assistantText: fullResponse }; this.updateMessageById(assistantMessageId, { content: `${fullResponse} @@ -11800,17 +11846,21 @@ ${writePreviews.map((a) => `- ${a.description}`).join("\n")}`, } let writeResults = []; if (writePreviews.length > 0 && !modeRequiresPreview(this.currentAgentMode)) { - writeResults = (await Promise.all( + writeResults = await Promise.all( writePreviews.map(async (action) => { try { const toolResult = await this.toolExecutor.handleToolCall(action.toolCall); return { ...toolResult, id: action.toolCall.id }; } catch (error) { ErrorHandler.handleError(error, "ChatView.processToolCalls"); - return null; + return { + success: false, + message: error instanceof Error ? error.message : String(error), + id: action.toolCall.id + }; } }) - )).filter((result) => result !== null); + ); } const allResults = [...readResults, ...writeResults]; const followUpMessages = allResults.map((result) => ({ @@ -11868,19 +11918,24 @@ ${writePreviews.map((a) => `- ${a.description}`).join("\n")}`, if (this.pendingActions.length === 0 || !this.pendingFollowUpContext) { return; } - const { messages, tools, assistantMessageId } = this.pendingFollowUpContext; - const writeResults = (await Promise.all( + const { messages, tools, assistantMessageId, allToolCalls, assistantText } = this.pendingFollowUpContext; + 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; + return { + success: false, + message: error instanceof Error ? error.message : String(error), + id: action.toolCall.id + }; } }) - )).filter((result) => result !== null); + ); const allResults = [...this.pendingReadResults, ...writeResults]; + const failedWrites = writeResults.filter((result) => !result.success); const followUpMessages = allResults.map((result) => ({ role: "tool", content: JSON.stringify(result), @@ -11888,8 +11943,8 @@ ${writePreviews.map((a) => `- ${a.description}`).join("\n")}`, })); const followUp = { role: "assistant", - content: "I have processed your request using the following tools. Here are the results:", - tool_calls: this.pendingActions.map((a) => a.toolCall) + content: assistantText, + tool_calls: allToolCalls }; if (followUpMessages.length > 0) { const finalMessages = [...messages, followUp, ...followUpMessages]; @@ -11920,14 +11975,16 @@ ${writePreviews.map((a) => `- ${a.description}`).join("\n")}`, ); } else { this.updateMessageById(assistantMessageId, { - content: followUpContent || "Actions applied successfully.", + content: followUpContent || (failedWrites.length > 0 ? `Some actions failed: +${failedWrites.map((r) => `- ${r.message}`).join("\n")}` : "Actions applied successfully."), isStreaming: false, isThinking: false }); } } else { this.updateMessageById(assistantMessageId, { - content: "Actions applied successfully.", + content: failedWrites.length > 0 ? `Actions failed: +${failedWrites.map((r) => `- ${r.message}`).join("\n")}` : "Actions applied successfully.", isStreaming: false, isThinking: false }); @@ -12477,7 +12534,7 @@ ${actualMessage}` : actualMessage; return this.isAgenticMode(this.currentAgentMode) ? this.agentOllamaClient : this.ollamaClient; } isAgenticMode(mode) { - return mode === "edit" || mode === "organize" || mode === "workflow"; + return mode === "edit" || mode === "organize" || mode === "research" || mode === "workflow"; } renderLogEntry(entry, container) { const row = container.createEl("div", { cls: "ollama-log-row" }); diff --git a/src/chat-view.ts b/src/chat-view.ts index ef0f990..daec1fc 100755 --- a/src/chat-view.ts +++ b/src/chat-view.ts @@ -140,6 +140,7 @@ export class ChatView extends ItemView { newSettings.agentModel ?? newSettings.model, { cacheConfig: newSettings.cacheConfig } ); + this.conversationStateManager.setSystemPrompt(getSystemPromptForMode(this.currentAgentMode)); void this.initializeClientCaches().catch(() => { new Notice( 'Semantic cache unavailable: could not connect to ChromaDB. Check the ChromaDB URL in settings.' @@ -318,7 +319,7 @@ export class ChatView extends ItemView { } } this.modeSelectorEl.addEventListener('change', () => { - this.currentAgentMode = this.modeSelectorEl!.value as AgentMode; + this.setAgentMode(this.modeSelectorEl!.value as AgentMode); }); } else { newChatContainer.appendChild(this.modeSelectorEl); @@ -561,6 +562,10 @@ export class ChatView extends ItemView { this.modeSelectorEl.value = mode; } this.conversationStateManager.setSystemPrompt(getSystemPromptForMode(mode)); + const activeId = this.chatHistoryManager?.getActiveSessionId(); + if (activeId) { + this.chatHistoryManager?.updateSession(activeId, { agentMode: mode }); + } } clearConversation(): void { @@ -610,7 +615,11 @@ export class ChatView extends ItemView { const activeId = this.chatHistoryManager.getActiveSessionId(); if (!activeId) return; const nonStreamingMessages = this.messages.filter((msg) => !msg.isStreaming); - this.chatHistoryManager.updateSessionMessages(activeId, nonStreamingMessages); + this.chatHistoryManager.updateSession(activeId, { + messages: nonStreamingMessages, + agentMode: this.currentAgentMode, + title: this.deriveSessionTitle(nonStreamingMessages), + }); } private syncMessagesToSession(): void { @@ -618,7 +627,18 @@ export class ChatView extends ItemView { const activeId = this.chatHistoryManager.getActiveSessionId(); if (!activeId) return; const nonStreamingMessages = this.messages.filter((msg) => !msg.isStreaming); - this.chatHistoryManager.updateSessionMessages(activeId, nonStreamingMessages); + this.chatHistoryManager.updateSession(activeId, { + messages: nonStreamingMessages, + agentMode: this.currentAgentMode, + title: this.deriveSessionTitle(nonStreamingMessages), + }); + } + + private deriveSessionTitle(messages: ChatMessage[]): string { + const firstUser = messages.find((m) => m.role === 'user'); + const text = firstUser?.content.trim() ?? ''; + if (!text) return 'New Chat'; + return text.length > 40 ? text.slice(0, 40) + '…' : text; } private populateHistoryDropdown(): void { @@ -1053,11 +1073,15 @@ export class ChatView extends ItemView { return { ...toolResult, id: action.toolCall.id }; } catch (error) { ErrorHandler.handleError(error, 'ChatView.processToolCalls'); - return null; + return { + success: false, + message: error instanceof Error ? error.message : String(error), + id: action.toolCall.id, + }; } }) ) - ).filter((result): result is NonNullable => result !== null); + ); } // No write tools (or they were already executed) — proceed with follow-up @@ -1137,13 +1161,18 @@ export class ChatView extends ItemView { return { ...toolResult, id: action.toolCall.id }; } catch (error) { ErrorHandler.handleError(error, 'ChatView.applyPendingActions'); - return null; + return { + success: false, + message: error instanceof Error ? error.message : String(error), + id: action.toolCall.id, + }; } }) ) - ).filter((result): result is NonNullable => result !== null); + ); const allResults = [...this.pendingReadResults, ...writeResults]; + const failedWrites = writeResults.filter((result) => !result.success); const followUpMessages: OllamaMessage[] = allResults.map((result) => ({ role: 'tool', @@ -1191,14 +1220,21 @@ export class ChatView extends ItemView { ); } else { this.updateMessageById(assistantMessageId, { - content: followUpContent || 'Actions applied successfully.', + content: + followUpContent || + (failedWrites.length > 0 + ? `Some actions failed:\n${failedWrites.map((r) => `- ${r.message}`).join('\n')}` + : 'Actions applied successfully.'), isStreaming: false, isThinking: false, }); } } else { this.updateMessageById(assistantMessageId, { - content: 'Actions applied successfully.', + content: + failedWrites.length > 0 + ? `Actions failed:\n${failedWrites.map((r) => `- ${r.message}`).join('\n')}` + : 'Actions applied successfully.', isStreaming: false, isThinking: false, }); diff --git a/src/tool-executor.ts b/src/tool-executor.ts index 344c9d4..eafccca 100644 --- a/src/tool-executor.ts +++ b/src/tool-executor.ts @@ -1,6 +1,6 @@ // src/tool-executor.ts -import { Vault, App, TFile } from 'obsidian'; +import { Vault, App, TFile, TFolder } from 'obsidian'; import type { ToolCall, ToolResult, VaultIndexEntry } from './types'; import { safeParseJson } from './utils'; import { TelemetryManager } from './tool-telemetry'; @@ -97,6 +97,33 @@ export class ToolExecutor { await this.vault.modify(file, content); } + private getParentFolderPath(path: string): string { + const parts = path.split('/').filter((part) => part.length > 0); + parts.pop(); + return parts.join('/'); + } + + private async ensureFolderExists(folderPath: string): Promise { + const normalizedFolder = folderPath.replace(/\/$/, '').trim(); + if (!normalizedFolder) { + return; + } + + const parts = normalizedFolder.split('/').filter((part) => part.length > 0); + let currentPath = ''; + for (const part of parts) { + currentPath = currentPath ? `${currentPath}/${part}` : part; + const existing = this.vault.getAbstractFileByPath(currentPath); + if (existing) { + if (!(existing instanceof TFolder)) { + throw new Error(`Cannot create folder ${currentPath}: a file already exists at that path`); + } + continue; + } + await this.vault.createFolder(currentPath); + } + } + async handleToolCall(toolCall: ToolCall): Promise { const startTime = Date.now(); const toolName = toolCall.function?.name ?? 'unknown'; @@ -203,6 +230,7 @@ export class ToolExecutor { } try { + await this.ensureFolderExists(this.getParentFolderPath(path)); await this.vault.create(path, content); return { success: true, message: 'Note created successfully' }; } catch (error) { @@ -494,6 +522,7 @@ export class ToolExecutor { } const file = this.getFile(oldPath); + await this.ensureFolderExists(this.getParentFolderPath(newPath)); await this.vault.rename(file, newPath); return { success: true, message: `Note renamed from ${oldPath} to ${newPath}` }; } @@ -522,6 +551,7 @@ export class ToolExecutor { const fileName = file.name; const newPath = normalizedFolder ? `${normalizedFolder}/${fileName}` : fileName; + await this.ensureFolderExists(normalizedFolder); await this.vault.rename(file, newPath); return { success: true, message: `Note moved to ${newPath}` }; } diff --git a/tests/chat-view.test.ts b/tests/chat-view.test.ts index 73efa38..c3d22fe 100755 --- a/tests/chat-view.test.ts +++ b/tests/chat-view.test.ts @@ -234,6 +234,18 @@ describe('ChatView', () => { const messages = view.contentEl.querySelectorAll('.ollama-message'); expect(messages.length).toBe(1); }); + + it('should update the system prompt when mode selector changes', async () => { + await view.render(); + const selector = view.contentEl.querySelector('.ollama-mode-selector') as HTMLSelectElement; + selector.appendChild(new Option('Edit', 'edit')); + selector.value = 'edit'; + selector.dispatchEvent(new Event('change')); + + const systemPrompt = view['conversationStateManager'].getLongTermContext()[0].content; + expect(view.getAgentMode()).toBe('edit'); + expect(systemPrompt).toContain('helps edit and manage notes'); + }); }); describe('handleUserInput', () => { @@ -600,6 +612,8 @@ describe('ChatView', () => { messages: [], tools: [], assistantMessageId, + allToolCalls: [], + assistantText: 'test', }; const followUpSpy = jest @@ -653,6 +667,8 @@ describe('ChatView', () => { messages: [], tools: [], assistantMessageId, + allToolCalls: [], + assistantText: 'Proposed actions...', }; view.cancelPendingActions(); diff --git a/tests/tool-executor.test.ts b/tests/tool-executor.test.ts index 482e1a3..142f216 100755 --- a/tests/tool-executor.test.ts +++ b/tests/tool-executor.test.ts @@ -1,5 +1,5 @@ import { ToolExecutor } from '../src/tool-executor'; -import { TFile } from 'obsidian'; +import { TFile, TFolder } from 'obsidian'; import { ToolCall, ToolResult } from '../src/types'; import { ErrorHandler } from '../src/error-handler'; import { TelemetryManager } from '../src/tool-telemetry'; @@ -7,6 +7,7 @@ import { TelemetryManager } from '../src/tool-telemetry'; // Mock Obsidian types interface MockVault { create: (path: string, content: string) => Promise; + createFolder: (path: string) => Promise; getAbstractFileByPath: (path: string) => any; cachedRead: (file: any) => Promise; getMarkdownFiles: () => any[]; @@ -32,6 +33,7 @@ jest.mock('obsidian', () => { App: jest.fn(), Notice: jest.fn(), TFile, + TFolder: class TFolder {}, }; }); @@ -50,6 +52,7 @@ describe('ToolExecutor', () => { beforeEach(() => { mockVault = { create: jest.fn().mockResolvedValue(null), + createFolder: jest.fn().mockResolvedValue(null), getAbstractFileByPath: jest.fn(), cachedRead: jest.fn().mockResolvedValue(''), getMarkdownFiles: jest.fn().mockReturnValue([]), @@ -103,6 +106,7 @@ describe('ToolExecutor', () => { }); it('should successfully create a file in a subdirectory', async () => { + mockVault.getAbstractFileByPath = jest.fn().mockReturnValue(null); const call: ToolCall = { id: 'call_3', type: 'function', @@ -116,12 +120,32 @@ describe('ToolExecutor', () => { }; const result = await executor.handleToolCall(call); expect(result).toEqual({ success: true, message: 'Note created successfully' }); + expect(mockVault.createFolder).toHaveBeenCalledWith('subdirectory'); expect(mockVault.create).toHaveBeenCalledWith( 'subdirectory/test-file.md', 'Subdir content' ); }); + it('should not recreate existing parent folders', async () => { + mockVault.getAbstractFileByPath = jest.fn().mockReturnValue(new TFolder()); + const call: ToolCall = { + id: 'call_existing_folder', + type: 'function', + function: { + name: 'create_file', + arguments: JSON.stringify({ + path: 'existing/test-file.md', + content: 'Subdir content', + }), + }, + }; + const result = await executor.handleToolCall(call); + expect(result).toEqual({ success: true, message: 'Note created successfully' }); + expect(mockVault.createFolder).not.toHaveBeenCalled(); + expect(mockVault.create).toHaveBeenCalledWith('existing/test-file.md', 'Subdir content'); + }); + it('should handle multiple slashes gracefully by normalizing path', async () => { const call: ToolCall = { id: 'call_4', @@ -1170,7 +1194,9 @@ describe('ToolExecutor', () => { } } const file = new MockTFile('Projects/old.md'); - mockVault.getAbstractFileByPath = jest.fn().mockReturnValue(file); + mockVault.getAbstractFileByPath = jest + .fn() + .mockImplementation((path: string) => (path === 'Projects/old.md' ? file : null)); const call: ToolCall = { id: 'call_mn1', @@ -1185,6 +1211,7 @@ describe('ToolExecutor', () => { }; const result = await executor.handleToolCall(call); expect(result.success).toBe(true); + expect(mockVault.createFolder).toHaveBeenCalledWith('Archive'); expect(mockVault.rename).toHaveBeenCalledWith(file, 'Archive/old.md'); }); });