From 3ab8542cf4321d82ccaaf86aa852af6abd42b8fa Mon Sep 17 00:00:00 2001 From: Florian Egger Date: Thu, 21 May 2026 20:41:58 +0200 Subject: [PATCH] Fix semantic cache distance calculation, tool path validation, and add deferred settings persistence - Convert Chroma cosine distance to similarity (1 - distance) for correct threshold comparison - Simplify forbidden directory check to match path segments instead of string prefixes - Add debounced settings save via onPersist callback in ChatView to prevent data loss - Fix workflow engine to pass availableTools to LLM when includeToolCalls is enabled - Add targetFolder support to AutoLinker config and update tests - Harden vault file indexing to await background indexing before processing create/modify/rename events --- main.js | 83 +++++++++++++++++--------- src/auto-organizer.ts | 4 ++ src/chat-view.ts | 8 ++- src/main.ts | 65 +++++++++++++------- src/semantic-cache.ts | 8 +-- src/tool-executor.ts | 10 ++-- src/workflow-engine/workflow-engine.ts | 4 +- tests/auto-organizer.test.ts | 19 ++++++ tests/semantic-cache.test.ts | 14 ++++- tests/tool-executor.test.ts | 48 +++++++++++++++ 10 files changed, 197 insertions(+), 66 deletions(-) diff --git a/main.js b/main.js index f161a24..53748c9 100644 --- a/main.js +++ b/main.js @@ -8158,7 +8158,9 @@ var SemanticCacheService = class _SemanticCacheService { where: { source: "ollama" } }); if (results.ids[0] && results.ids[0].length > 0) { - if (results.distances && results.distances[0] && results.distances[0][0] > this.config.similarityThreshold) { + const distance = results.distances?.[0]?.[0]; + const similarity = typeof distance === "number" ? 1 - distance : 0; + if (similarity >= this.config.similarityThreshold) { return results.documents[0][0]; } } @@ -8940,14 +8942,12 @@ var ToolExecutor = class { return false; } const normalized = path.replace(/^(\.\/)+/, ""); - if (normalized.split("/").includes("..")) { + const segments = normalized.split("/").filter((segment) => segment.length > 0); + if (segments.includes("..")) { return false; } - for (const dir of FORBIDDEN_DIRS) { - if (normalized.startsWith(`${dir}/`) || normalized.startsWith(`${dir}\\`)) { - return false; - } - if (normalized.includes(`/${dir}/`) || normalized.includes(`\\${dir}\\`)) { + for (const segment of segments) { + if (FORBIDDEN_DIRS.includes(segment)) { return false; } } @@ -9930,6 +9930,7 @@ ${queryResult}` var VARIABLE_PATTERN = /\{\{([\w.]+)\}\}/g; var WorkflowEngine = class _WorkflowEngine { constructor(vault, app, ollamaUrl, model, options) { + this.availableTools = []; this.vaultIndexer = new VaultIndexer(vault); this.vaultIndexer.setApp(app); this.toolExecutor = new ToolExecutor(vault, app, void 0, this.vaultIndexer); @@ -10041,6 +10042,7 @@ var WorkflowEngine = class _WorkflowEngine { */ async executeWorkflowFromQuery(userQuery, availableTools) { Logger.info(`Generating workflow from query: ${userQuery}`, "workflow-engine"); + this.availableTools = availableTools ?? []; const workflow = await this.generateWorkflowFromQuery(userQuery, availableTools); if (!workflow) { return { @@ -10203,7 +10205,7 @@ Rules: role: "user", content: config.userPrompt }); - const tools = config.includeToolCalls ? [] : []; + const tools = config.includeToolCalls ? this.availableTools : []; const response = await this.ollamaClient.chat(messages, tools); return response.content ?? ""; } @@ -10934,7 +10936,7 @@ var ErrorHandler = class { // src/chat-view.ts var ChatView = class extends import_obsidian5.ItemView { - constructor(leaf, settings, vectorStore, structuredMemoryManager, telemetryManager, chatHistoryManager, onModelChange) { + constructor(leaf, settings, vectorStore, structuredMemoryManager, telemetryManager, chatHistoryManager, onModelChange, onPersist) { super(leaf); // State this.messages = []; @@ -11003,6 +11005,7 @@ var ChatView = class extends import_obsidian5.ItemView { this.telemetryManager = telemetryManager; this.chatHistoryManager = chatHistoryManager; this.onModelChange = onModelChange; + this.onPersist = onPersist; this.workflowEngine = new WorkflowEngine( this.app.vault, this.app, @@ -11399,6 +11402,7 @@ var ChatView = class extends import_obsidian5.ItemView { const activeId = this.chatHistoryManager?.getActiveSessionId(); if (activeId) { this.chatHistoryManager?.updateSession(activeId, { agentMode: mode }); + this.onPersist?.(); } } clearConversation() { @@ -11446,6 +11450,7 @@ var ChatView = class extends import_obsidian5.ItemView { agentMode: this.currentAgentMode, title: this.deriveSessionTitle(nonStreamingMessages) }); + this.onPersist?.(); } syncMessagesToSession() { if (!this.chatHistoryManager) return; @@ -11457,6 +11462,7 @@ var ChatView = class extends import_obsidian5.ItemView { agentMode: this.currentAgentMode, title: this.deriveSessionTitle(nonStreamingMessages) }); + this.onPersist?.(); } deriveSessionTitle(messages) { const firstUser = messages.find((m) => m.role === "user"); @@ -13359,6 +13365,9 @@ var AutoLinker = class { } updateConfig(config) { this.config = config; + if (typeof config.targetFolder === "string") { + this.targetFolder = config.targetFolder; + } } setTargetFolder(folder) { this.targetFolder = folder; @@ -13902,7 +13911,8 @@ var OllamaPlugin = class extends import_obsidian7.Plugin { this.settings.model = model; void this.saveSettings(); this.notifyChatViews(); - } + }, + () => this.scheduleSaveSettings() ) ); this.addRibbonIcon("bot", "Open Ollama Chat", async () => { @@ -13958,7 +13968,7 @@ var OllamaPlugin = class extends import_obsidian7.Plugin { this.initializeAutoOrganizer(); if (this.autoLinker) { new import_obsidian7.Notice("Auto-linking related notes..."); - void this.autoLinker.run(); + void this.autoLinker.run(this.settings.autoLinkConfig.dryRun); } } }); @@ -14007,9 +14017,11 @@ var OllamaPlugin = class extends import_obsidian7.Plugin { // eslint-disable-next-line @typescript-eslint/no-misused-promises onunload() { this.cancelBackgroundIndexing(); - if (this.semanticCache) { - void this.semanticCache.clearCache(); + if (this.saveSettingsTimer) { + clearTimeout(this.saveSettingsTimer); + this.saveSettingsTimer = void 0; } + void this.saveSettings(); } async loadSettings() { const data = await this.loadData() ?? {}; @@ -14037,6 +14049,17 @@ var OllamaPlugin = class extends import_obsidian7.Plugin { chatHistory: this.chatHistoryManager?.getData() ?? createDefaultChatHistoryData() }); } + scheduleSaveSettings(delayMs = 500) { + if (this.saveSettingsTimer) { + clearTimeout(this.saveSettingsTimer); + } + this.saveSettingsTimer = setTimeout(() => { + this.saveSettingsTimer = void 0; + void this.saveSettings().catch((error) => { + Logger.warn(`Failed to persist plugin data: ${String(error)}`, "main"); + }); + }, delayMs); + } initializeAutoOrganizer() { this.autoTagger = new AutoTagger( this.app.vault, @@ -14150,26 +14173,32 @@ var OllamaPlugin = class extends import_obsidian7.Plugin { await this.vaultVectorStore.clearIndex(); } } + indexVaultFileWhenReady(file) { + void (async () => { + await this.awaitBackgroundIndexing(); + if (!this.vaultVectorStore) return; + const currentFile = this.app.vault.getAbstractFileByPath(file.path); + if (!(currentFile instanceof import_obsidian7.TFile) || currentFile.extension !== "md") return; + const content = await this.app.vault.read(currentFile); + const cache = this.app.metadataCache.getFileCache(currentFile); + await this.vaultVectorStore.indexFile(currentFile, content, cache ?? void 0); + })().catch((error) => { + const errorMessage = error instanceof Error ? error.message : String(error); + Logger.warn(`Failed to update vault index for ${file.path}: ${errorMessage}`, "main"); + }); + } registerVaultEventListeners() { this.registerEvent( this.app.vault.on("create", (file) => { if (file instanceof import_obsidian7.TFile && file.extension === "md" && this.vaultVectorStore) { - void this.app.vault.read(file).then((content) => { - if (!this.currentIndexingPromise) { - void this.vaultVectorStore?.indexFile(file, content); - } - }); + this.indexVaultFileWhenReady(file); } }) ); this.registerEvent( this.app.vault.on("modify", (file) => { if (file instanceof import_obsidian7.TFile && file.extension === "md" && this.vaultVectorStore) { - void this.app.vault.read(file).then((content) => { - if (!this.currentIndexingPromise) { - void this.vaultVectorStore?.indexFile(file, content); - } - }); + this.indexVaultFileWhenReady(file); } }) ); @@ -14184,11 +14213,7 @@ var OllamaPlugin = class extends import_obsidian7.Plugin { this.app.vault.on("rename", (file, oldPath) => { if (file instanceof import_obsidian7.TFile && file.extension === "md" && this.vaultVectorStore) { void this.vaultVectorStore.deleteFile(oldPath); - void this.app.vault.read(file).then((content) => { - if (!this.currentIndexingPromise) { - void this.vaultVectorStore?.indexFile(file, content); - } - }); + this.indexVaultFileWhenReady(file); } }) ); @@ -14538,7 +14563,7 @@ var OllamaSettingTab = class extends import_obsidian7.PluginSettingTab { try { this.plugin.initializeAutoOrganizer(); if (this.plugin.autoLinker) { - await this.plugin.autoLinker.run(); + await this.plugin.autoLinker.run(this.plugin.settings.autoLinkConfig.dryRun); } } catch { new import_obsidian7.Notice("Auto-linking failed. Check console for details."); diff --git a/src/auto-organizer.ts b/src/auto-organizer.ts index e548a30..bb7480b 100644 --- a/src/auto-organizer.ts +++ b/src/auto-organizer.ts @@ -362,8 +362,12 @@ export class AutoLinker { enabled: boolean; maxLinksPerNote: number; similarityThreshold: number; + targetFolder?: string; }): void { this.config = config; + if (typeof config.targetFolder === 'string') { + this.targetFolder = config.targetFolder; + } } setTargetFolder(folder: string): void { diff --git a/src/chat-view.ts b/src/chat-view.ts index daec1fc..8a4e645 100755 --- a/src/chat-view.ts +++ b/src/chat-view.ts @@ -58,7 +58,8 @@ export class ChatView extends ItemView { structuredMemoryManager?: StructuredMemoryManager, telemetryManager?: TelemetryManager, chatHistoryManager?: ChatHistoryManager, - onModelChange?: (model: string) => void + onModelChange?: (model: string) => void, + onPersist?: () => void ) { super(leaf); this.messages = []; @@ -98,6 +99,7 @@ export class ChatView extends ItemView { this.telemetryManager = telemetryManager; this.chatHistoryManager = chatHistoryManager; this.onModelChange = onModelChange; + this.onPersist = onPersist; this.workflowEngine = new WorkflowEngine( this.app.vault, this.app, @@ -565,6 +567,7 @@ export class ChatView extends ItemView { const activeId = this.chatHistoryManager?.getActiveSessionId(); if (activeId) { this.chatHistoryManager?.updateSession(activeId, { agentMode: mode }); + this.onPersist?.(); } } @@ -620,6 +623,7 @@ export class ChatView extends ItemView { agentMode: this.currentAgentMode, title: this.deriveSessionTitle(nonStreamingMessages), }); + this.onPersist?.(); } private syncMessagesToSession(): void { @@ -632,6 +636,7 @@ export class ChatView extends ItemView { agentMode: this.currentAgentMode, title: this.deriveSessionTitle(nonStreamingMessages), }); + this.onPersist?.(); } private deriveSessionTitle(messages: ChatMessage[]): string { @@ -1885,6 +1890,7 @@ export class ChatView extends ItemView { private chatHistoryManager?: ChatHistoryManager; private vectorStore?: VaultVectorStore; private onModelChange?: (model: string) => void; + private onPersist?: () => void; private modeSelectorEl: HTMLSelectElement | null = null; private modelSelectorEl: HTMLSelectElement | null = null; diff --git a/src/main.ts b/src/main.ts index c4d429d..3d31139 100755 --- a/src/main.ts +++ b/src/main.ts @@ -24,6 +24,7 @@ export default class OllamaPlugin extends Plugin { chatHistoryManager?: ChatHistoryManager; private indexingAbortController?: AbortController; private currentIndexingPromise?: Promise; + private saveSettingsTimer?: ReturnType; async onload() { await this.loadSettings(); @@ -50,7 +51,8 @@ export default class OllamaPlugin extends Plugin { this.settings.model = model; void this.saveSettings(); this.notifyChatViews(); - } + }, + () => this.scheduleSaveSettings() ) ); @@ -120,7 +122,7 @@ export default class OllamaPlugin extends Plugin { this.initializeAutoOrganizer(); if (this.autoLinker) { new Notice('Auto-linking related notes...'); - void this.autoLinker.run(); + void this.autoLinker.run(this.settings.autoLinkConfig.dryRun); } }, }); @@ -182,11 +184,11 @@ export default class OllamaPlugin extends Plugin { // Cancel any ongoing indexing this.cancelBackgroundIndexing(); - // Clean up any active semantic cache resources on plugin unload - // Using fire-and-forget pattern since onunload cannot be async per Obsidian API - if (this.semanticCache) { - void this.semanticCache.clearCache(); + if (this.saveSettingsTimer) { + clearTimeout(this.saveSettingsTimer); + this.saveSettingsTimer = undefined; } + void this.saveSettings(); // No explicit unregisterView needed; relying on Obsidian lifecycle management. } @@ -227,6 +229,18 @@ export default class OllamaPlugin extends Plugin { }); } + private scheduleSaveSettings(delayMs = 500): void { + if (this.saveSettingsTimer) { + clearTimeout(this.saveSettingsTimer); + } + this.saveSettingsTimer = setTimeout(() => { + this.saveSettingsTimer = undefined; + void this.saveSettings().catch((error) => { + Logger.warn(`Failed to persist plugin data: ${String(error)}`, 'main'); + }); + }, delayMs); + } + initializeAutoOrganizer(): void { this.autoTagger = new AutoTagger( this.app.vault, @@ -368,17 +382,30 @@ export default class OllamaPlugin extends Plugin { } } + private indexVaultFileWhenReady(file: TFile): void { + void (async () => { + await this.awaitBackgroundIndexing(); + + if (!this.vaultVectorStore) return; + + const currentFile = this.app.vault.getAbstractFileByPath(file.path); + if (!(currentFile instanceof TFile) || currentFile.extension !== 'md') return; + + const content = await this.app.vault.read(currentFile); + const cache = this.app.metadataCache.getFileCache(currentFile); + await this.vaultVectorStore.indexFile(currentFile, content, cache ?? undefined); + })().catch((error) => { + const errorMessage = error instanceof Error ? error.message : String(error); + Logger.warn(`Failed to update vault index for ${file.path}: ${errorMessage}`, 'main'); + }); + } + registerVaultEventListeners(): void { // Listen for file creation this.registerEvent( this.app.vault.on('create', (file) => { if (file instanceof TFile && file.extension === 'md' && this.vaultVectorStore) { - void this.app.vault.read(file).then((content) => { - // Skip if a full rebuild is in progress to avoid race conditions - if (!this.currentIndexingPromise) { - void this.vaultVectorStore?.indexFile(file, content); - } - }); + this.indexVaultFileWhenReady(file); } }) ); @@ -387,11 +414,7 @@ export default class OllamaPlugin extends Plugin { this.registerEvent( this.app.vault.on('modify', (file) => { if (file instanceof TFile && file.extension === 'md' && this.vaultVectorStore) { - void this.app.vault.read(file).then((content) => { - if (!this.currentIndexingPromise) { - void this.vaultVectorStore?.indexFile(file, content); - } - }); + this.indexVaultFileWhenReady(file); } }) ); @@ -410,11 +433,7 @@ export default class OllamaPlugin extends Plugin { this.app.vault.on('rename', (file, oldPath) => { if (file instanceof TFile && file.extension === 'md' && this.vaultVectorStore) { void this.vaultVectorStore.deleteFile(oldPath); - void this.app.vault.read(file).then((content) => { - if (!this.currentIndexingPromise) { - void this.vaultVectorStore?.indexFile(file, content); - } - }); + this.indexVaultFileWhenReady(file); } }) ); @@ -938,7 +957,7 @@ class OllamaSettingTab extends PluginSettingTab { try { this.plugin.initializeAutoOrganizer(); if (this.plugin.autoLinker) { - await this.plugin.autoLinker.run(); + await this.plugin.autoLinker.run(this.plugin.settings.autoLinkConfig.dryRun); } } catch { new Notice('Auto-linking failed. Check console for details.'); diff --git a/src/semantic-cache.ts b/src/semantic-cache.ts index 4e6eb72..8fe9bbe 100644 --- a/src/semantic-cache.ts +++ b/src/semantic-cache.ts @@ -47,11 +47,9 @@ export class SemanticCacheService { }); if (results.ids[0] && results.ids[0].length > 0) { - if ( - results.distances && - results.distances[0] && - results.distances[0][0] > this.config.similarityThreshold - ) { + const distance = results.distances?.[0]?.[0]; + const similarity = typeof distance === 'number' ? 1 - distance : 0; + if (similarity >= this.config.similarityThreshold) { return results.documents[0][0]; } } diff --git a/src/tool-executor.ts b/src/tool-executor.ts index eafccca..1b49bf1 100644 --- a/src/tool-executor.ts +++ b/src/tool-executor.ts @@ -62,16 +62,14 @@ export class ToolExecutor { // Reject paths that traverse to parent directories const normalized = path.replace(/^(\.\/)+/, ''); - if (normalized.split('/').includes('..')) { + const segments = normalized.split('/').filter((segment) => segment.length > 0); + if (segments.includes('..')) { return false; } // Reject forbidden directories - for (const dir of FORBIDDEN_DIRS) { - if (normalized.startsWith(`${dir}/`) || normalized.startsWith(`${dir}\\`)) { - return false; - } - if (normalized.includes(`/${dir}/`) || normalized.includes(`\\${dir}\\`)) { + for (const segment of segments) { + if (FORBIDDEN_DIRS.includes(segment)) { return false; } } diff --git a/src/workflow-engine/workflow-engine.ts b/src/workflow-engine/workflow-engine.ts index 53cf549..e0e371d 100644 --- a/src/workflow-engine/workflow-engine.ts +++ b/src/workflow-engine/workflow-engine.ts @@ -45,6 +45,7 @@ export class WorkflowEngine { private conversationStateManager: ConversationStateManager; private maxSteps: number; private maxWorkflowDuration: number; + private availableTools: OllamaTool[] = []; constructor( vault: Vault, @@ -200,6 +201,7 @@ export class WorkflowEngine { availableTools?: OllamaTool[] ): Promise { Logger.info(`Generating workflow from query: ${userQuery}`, 'workflow-engine'); + this.availableTools = availableTools ?? []; // First, ask the LLM to generate a workflow plan const workflow = await this.generateWorkflowFromQuery(userQuery, availableTools); @@ -407,7 +409,7 @@ Rules: }); // Determine if we should include tools - const tools = config.includeToolCalls ? [] : []; + const tools = config.includeToolCalls ? this.availableTools : []; const response = await this.ollamaClient.chat(messages, tools); return response.content ?? ''; diff --git a/tests/auto-organizer.test.ts b/tests/auto-organizer.test.ts index 2108046..2634673 100644 --- a/tests/auto-organizer.test.ts +++ b/tests/auto-organizer.test.ts @@ -366,6 +366,25 @@ describe('AutoLinker', () => { expect(mockVault.read).toHaveBeenCalledWith(files[0]); }); + it('should update targetFolder through updateConfig', async () => { + linker.setTargetFolder('Projects'); + linker.updateConfig({ + enabled: true, + maxLinksPerNote: 3, + similarityThreshold: 0.5, + targetFolder: 'Archive', + }); + const files = [{ path: 'Projects/note1.md' }, { path: 'Archive/note2.md' }] as any[]; + mockVault.getMarkdownFiles.mockReturnValue(files); + mockVault.read.mockResolvedValue('Content'); + mockIndexer.searchVault!.mockResolvedValue([]); + + await linker.run(); + + expect(mockVault.read).toHaveBeenCalledTimes(1); + expect(mockVault.read).toHaveBeenCalledWith(files[1]); + }); + it('should return dry-run proposals when dryRun is true', async () => { const files = [{ path: 'note.md' }] as any[]; mockVault.getMarkdownFiles.mockReturnValue(files); diff --git a/tests/semantic-cache.test.ts b/tests/semantic-cache.test.ts index 232b7f3..18bbb5f 100644 --- a/tests/semantic-cache.test.ts +++ b/tests/semantic-cache.test.ts @@ -119,7 +119,7 @@ describe('SemanticCacheService', () => { mockCollection.query.mockResolvedValue({ ids: [['test-id']], documents: [[cachedContent]], - distances: [[0.9]], // Above threshold + distances: [[0.1]], // Similarity 0.9, above threshold }); const result = await cacheService.getCache('test query'); @@ -127,6 +127,18 @@ describe('SemanticCacheService', () => { expect(result).toBe(cachedContent); expect(mockCollection.query).toHaveBeenCalled(); }); + + it('should return null when cosine distance is too high', async () => { + mockCollection.query.mockResolvedValue({ + ids: [['test-id']], + documents: [['unrelated cached response']], + distances: [[0.9]], // Similarity 0.1, below threshold + }); + + const result = await cacheService.getCache('test query'); + + expect(result).toBeNull(); + }); }); describe('setCache', () => { diff --git a/tests/tool-executor.test.ts b/tests/tool-executor.test.ts index 142f216..ffb83d7 100755 --- a/tests/tool-executor.test.ts +++ b/tests/tool-executor.test.ts @@ -213,6 +213,38 @@ describe('ToolExecutor', () => { expect(mockVault.create).not.toHaveBeenCalled(); }); + it('should reject exact forbidden directory paths', async () => { + const call: ToolCall = { + id: 'call_forbidden_exact', + type: 'function', + function: { + name: 'create_file', + arguments: JSON.stringify({ + path: '.obsidian', + content: 'Test content', + }), + }, + }; + await expect(executor.handleToolCall(call)).rejects.toThrow(); + expect(mockVault.create).not.toHaveBeenCalled(); + }); + + it('should reject nested forbidden directory paths', async () => { + const call: ToolCall = { + id: 'call_forbidden_nested', + type: 'function', + function: { + name: 'create_file', + arguments: JSON.stringify({ + path: 'Notes/.git', + content: 'Test content', + }), + }, + }; + await expect(executor.handleToolCall(call)).rejects.toThrow(); + expect(mockVault.create).not.toHaveBeenCalled(); + }); + it('should reject path traversal attempts with .\\', async () => { const call: ToolCall = { id: 'call_8', @@ -1214,6 +1246,22 @@ describe('ToolExecutor', () => { expect(mockVault.createFolder).toHaveBeenCalledWith('Archive'); expect(mockVault.rename).toHaveBeenCalledWith(file, 'Archive/old.md'); }); + + it('should reject moving a note into a forbidden folder', async () => { + const call: ToolCall = { + id: 'call_mn_forbidden', + type: 'function', + function: { + name: 'move_note', + arguments: JSON.stringify({ + path: 'Projects/old.md', + folder: '.obsidian', + }), + }, + }; + await expect(executor.handleToolCall(call)).rejects.toThrow('Invalid folder path detected'); + expect(mockVault.rename).not.toHaveBeenCalled(); + }); }); describe('delete_note tool', () => {