From d6f297abf3ab1095b30b95f3d33cc310100fb16b Mon Sep 17 00:00:00 2001 From: Bill Date: Sun, 26 Oct 2025 09:16:51 -0400 Subject: [PATCH] feat: improve notification message clarity with MCP Tool Called label - Update notification format to multi-line with explicit label - First line: 'MCP Tool Called: tool_name' - Second line: parameters (if enabled) - Add comprehensive tests for notification formatting Co-Authored-By: Claude --- src/ui/notifications.ts | 20 ++--- tests/notifications.test.ts | 141 ++++++++++++++++++++++++++++++++++++ 2 files changed, 152 insertions(+), 9 deletions(-) create mode 100644 tests/notifications.test.ts diff --git a/src/ui/notifications.ts b/src/ui/notifications.ts index 125988b..34a702a 100644 --- a/src/ui/notifications.ts +++ b/src/ui/notifications.ts @@ -81,8 +81,10 @@ export class NotificationManager { const icon = TOOL_ICONS[toolName] || '🔧'; const argsStr = this.formatArgs(args); - const message = `${icon} MCP: ${toolName}${argsStr}`; - + const message = argsStr + ? `${icon} MCP Tool Called: ${toolName}\n${argsStr}` + : `${icon} MCP Tool Called: ${toolName}`; + this.queueNotification(() => { new Notice(message, duration || this.settings.notificationDuration); }); @@ -144,13 +146,13 @@ export class NotificationManager { } if (!args || Object.keys(args).length === 0) { - return '()'; + return ''; } try { // Extract key parameters for display const keyParams: string[] = []; - + if (args.path) { keyParams.push(`path: "${this.truncateString(args.path, 30)}"`); } @@ -163,16 +165,16 @@ export class NotificationManager { if (args.recursive !== undefined) { keyParams.push(`recursive: ${args.recursive}`); } - + // If no key params, show first 50 chars of JSON if (keyParams.length === 0) { const json = JSON.stringify(args); - return `(${this.truncateString(json, 50)})`; + return this.truncateString(json, 50); } - - return `({ ${keyParams.join(', ')} })`; + + return keyParams.join(', '); } catch (e) { - return '(...)'; + return ''; } } diff --git a/tests/notifications.test.ts b/tests/notifications.test.ts new file mode 100644 index 0000000..fec9290 --- /dev/null +++ b/tests/notifications.test.ts @@ -0,0 +1,141 @@ +import { App, Notice } from 'obsidian'; +import { NotificationManager } from '../src/ui/notifications'; +import { MCPPluginSettings } from '../src/types/settings-types'; + +// Mock Notice constructor +jest.mock('obsidian', () => { + const actualObsidian = jest.requireActual('obsidian'); + return { + ...actualObsidian, + Notice: jest.fn() + }; +}); + +describe('NotificationManager', () => { + let app: App; + let settings: MCPPluginSettings; + let manager: NotificationManager; + + beforeEach(() => { + jest.clearAllMocks(); + app = {} as App; + settings = { + port: 3000, + autoStart: false, + apiKey: 'test-key', + notificationsEnabled: true, + showParameters: true, + notificationDuration: 3000, + logToConsole: false + }; + manager = new NotificationManager(app, settings); + }); + + describe('showToolCall', () => { + it('should format message with MCP Tool Called label and newline when parameters shown', () => { + manager.showToolCall('read_note', { path: 'daily/2025-01-15.md' }); + + expect(Notice).toHaveBeenCalledWith( + expect.stringContaining('📖 MCP Tool Called: read_note\npath: "daily/2025-01-15.md"'), + 3000 + ); + }); + + it('should format message without newline when parameters hidden', () => { + settings.showParameters = false; + manager = new NotificationManager(app, settings); + + manager.showToolCall('read_note', { path: 'daily/2025-01-15.md' }); + + expect(Notice).toHaveBeenCalledWith( + '📖 MCP Tool Called: read_note', + 3000 + ); + }); + + it('should format multiple parameters correctly', () => { + manager.showToolCall('search', { + query: 'test query', + folder: 'notes', + recursive: true + }); + + expect(Notice).toHaveBeenCalledWith( + expect.stringContaining('🔍 MCP Tool Called: search\nquery: "test query", folder: "notes", recursive: true'), + 3000 + ); + }); + + it('should handle empty arguments object', () => { + manager.showToolCall('get_vault_info', {}); + + expect(Notice).toHaveBeenCalledWith( + 'ℹī¸ MCP Tool Called: get_vault_info', + 3000 + ); + }); + + it('should handle null arguments', () => { + manager.showToolCall('get_vault_info', null); + + expect(Notice).toHaveBeenCalledWith( + 'ℹī¸ MCP Tool Called: get_vault_info', + 3000 + ); + }); + + it('should handle undefined arguments', () => { + manager.showToolCall('get_vault_info', undefined); + + expect(Notice).toHaveBeenCalledWith( + 'ℹī¸ MCP Tool Called: get_vault_info', + 3000 + ); + }); + + it('should use fallback icon for unknown tool', () => { + manager.showToolCall('unknown_tool', { path: 'test.md' }); + + expect(Notice).toHaveBeenCalledWith( + expect.stringContaining('🔧 MCP Tool Called: unknown_tool\npath: "test.md"'), + 3000 + ); + }); + + it('should use JSON fallback for arguments with no known keys', () => { + manager.showToolCall('custom_tool', { + customKey: 'value', + anotherKey: 123 + }); + + expect(Notice).toHaveBeenCalledWith( + '🔧 MCP Tool Called: custom_tool\n{"customKey":"value","anotherKey":123}', + 3000 + ); + }); + + it('should truncate path when exceeds 30 characters', () => { + const longPath = 'very/long/path/to/my/notes/folder/file.md'; + manager.showToolCall('read_note', { path: longPath }); + + expect(Notice).toHaveBeenCalledWith( + '📖 MCP Tool Called: read_note\npath: "very/long/path/to/my/notes/..."', + 3000 + ); + }); + + it('should truncate JSON fallback when exceeds 50 characters', () => { + const longJson = { + veryLongKeyName: 'very long value that exceeds the character limit', + anotherKey: 'more data' + }; + manager.showToolCall('custom_tool', longJson); + + const call = (Notice as jest.Mock).mock.calls[0][0]; + const lines = call.split('\n'); + expect(lines[0]).toBe('🔧 MCP Tool Called: custom_tool'); + expect(lines[1].length).toBeLessThanOrEqual(50); + expect(lines[1]).toMatch(/\.\.\.$/); + }); + }); +});