From 2dc99e646a003fc1012e66c013f2fe12ab052523 Mon Sep 17 00:00:00 2001 From: Pedram Amini Date: Sun, 1 Feb 2026 20:06:34 -0600 Subject: [PATCH] feat(notifications): allow any custom notification command - Remove TTS command whitelist validation - users can now configure any command for custom notifications (e.g., fabric piped to 11s) - Enable shell mode for spawn to support pipes and command chains - Add visual feedback to Test button (running/success/error states) - Remove speech bubble from AI response messages in TerminalOutput - Rename internal TTS terminology to more generic "notification command" - Update tests to reflect new behavior --- .../main/ipc/handlers/notifications.test.ts | 231 ++++------- .../main/preload/notifications.test.ts | 38 +- .../components/TerminalOutput.test.tsx | 64 --- src/main/ipc/handlers/notifications.ts | 379 ++++++++---------- src/main/preload/index.ts | 2 +- src/main/preload/notifications.ts | 39 +- src/renderer/App.tsx | 3 - src/renderer/components/MainPanel.tsx | 4 - .../components/NotificationsPanel.tsx | 112 +++++- src/renderer/components/TerminalOutput.tsx | 109 ----- src/renderer/hooks/props/useMainPanelProps.ts | 5 - 11 files changed, 387 insertions(+), 599 deletions(-) diff --git a/src/__tests__/main/ipc/handlers/notifications.test.ts b/src/__tests__/main/ipc/handlers/notifications.test.ts index a7e916da..d5eca76c 100644 --- a/src/__tests__/main/ipc/handlers/notifications.test.ts +++ b/src/__tests__/main/ipc/handlers/notifications.test.ts @@ -1,7 +1,7 @@ /** * Tests for notification IPC handlers * - * Note: TTS-related tests are simplified due to the complexity of mocking + * Note: Notification command tests are simplified due to the complexity of mocking * child_process spawn with all the event listeners and stdin handling. */ @@ -88,8 +88,7 @@ import { getActiveTtsCount, clearTtsQueue, getTtsMaxQueueSize, - getAllowedTtsCommands, - validateTtsCommand, + parseNotificationCommand, } from '../../../../main/ipc/handlers/notifications'; describe('Notification IPC Handlers', () => { @@ -183,30 +182,30 @@ describe('Notification IPC Handlers', () => { }); describe('notification:stopSpeak', () => { - it('should return error when no active TTS process', async () => { + it('should return error when no active notification process', async () => { const handler = handlers.get('notification:stopSpeak')!; const result = await handler({}, 999); expect(result.success).toBe(false); - expect(result.error).toBe('No active TTS process with that ID'); + expect(result.error).toBe('No active notification process with that ID'); }); }); - describe('TTS state utilities', () => { - it('should track TTS queue length', () => { + describe('notification state utilities', () => { + it('should track notification queue length', () => { expect(getTtsQueueLength()).toBe(0); }); - it('should track active TTS count', () => { + it('should track active notification count', () => { expect(getActiveTtsCount()).toBe(0); }); - it('should clear TTS queue', () => { + it('should clear notification queue', () => { clearTtsQueue(); expect(getTtsQueueLength()).toBe(0); }); - it('should reset TTS state', () => { + it('should reset notification state', () => { resetTtsState(); expect(getTtsQueueLength()).toBe(0); expect(getActiveTtsCount()).toBe(0); @@ -215,168 +214,88 @@ describe('Notification IPC Handlers', () => { it('should return max queue size', () => { expect(getTtsMaxQueueSize()).toBe(10); }); + }); - it('should return allowed TTS commands', () => { - const commands = getAllowedTtsCommands(); - expect(commands).toContain('say'); - expect(commands).toContain('espeak'); - expect(commands).toContain('espeak-ng'); - expect(commands).toContain('spd-say'); - expect(commands).toContain('festival'); - expect(commands).toContain('flite'); + describe('notification command parsing', () => { + it('should return default command when none provided', () => { + const result = parseNotificationCommand(); + expect(result).toBe('say'); + }); + + it('should return default command for empty string', () => { + const result = parseNotificationCommand(''); + expect(result).toBe('say'); + }); + + it('should return default command for whitespace-only string', () => { + const result = parseNotificationCommand(' '); + expect(result).toBe('say'); + }); + + it('should accept any command - user has full control', () => { + const result = parseNotificationCommand('say'); + expect(result).toBe('say'); + }); + + it('should accept custom commands with full paths', () => { + const result = parseNotificationCommand('/usr/local/bin/my-tts'); + expect(result).toBe('/usr/local/bin/my-tts'); + }); + + it('should accept commands with arguments', () => { + const result = parseNotificationCommand('say -v Alex'); + expect(result).toBe('say -v Alex'); + }); + + it('should accept command chains with pipes', () => { + const result = parseNotificationCommand('tee ~/log.txt | say'); + expect(result).toBe('tee ~/log.txt | say'); + }); + + it('should accept fabric pattern commands', () => { + const result = parseNotificationCommand( + '/Users/pedram/go/bin/fabric --pattern ped_summarize_conversational --model gpt-5-mini --raw 2>/dev/null | /Users/pedram/.local/bin/11s --voice NFQv27BRKPFgprCm0xgr' + ); + expect(result).toBe( + '/Users/pedram/go/bin/fabric --pattern ped_summarize_conversational --model gpt-5-mini --raw 2>/dev/null | /Users/pedram/.local/bin/11s --voice NFQv27BRKPFgprCm0xgr' + ); + }); + + it('should trim leading and trailing whitespace', () => { + const result = parseNotificationCommand(' say '); + expect(result).toBe('say'); + }); + + it('should accept espeak command', () => { + const result = parseNotificationCommand('espeak'); + expect(result).toBe('espeak'); + }); + + it('should accept festival command with flags', () => { + const result = parseNotificationCommand('festival --tts'); + expect(result).toBe('festival --tts'); }); }); - describe('TTS command validation (security)', () => { - it('should accept default command when none provided', () => { - const result = validateTtsCommand(); - expect(result.valid).toBe(true); - expect(result.command).toBe('say'); - }); - - it('should accept empty string and use default', () => { - const result = validateTtsCommand(''); - expect(result.valid).toBe(true); - expect(result.command).toBe('say'); - }); - - it('should accept whitespace-only string and use default', () => { - const result = validateTtsCommand(' '); - expect(result.valid).toBe(true); - expect(result.command).toBe('say'); - }); - - it('should accept whitelisted command: say', () => { - const result = validateTtsCommand('say'); - expect(result.valid).toBe(true); - expect(result.command).toBe('say'); - }); - - it('should accept whitelisted command: espeak', () => { - const result = validateTtsCommand('espeak'); - expect(result.valid).toBe(true); - expect(result.command).toBe('espeak'); - }); - - it('should accept whitelisted command: espeak-ng', () => { - const result = validateTtsCommand('espeak-ng'); - expect(result.valid).toBe(true); - expect(result.command).toBe('espeak-ng'); - }); - - it('should accept whitelisted command: spd-say', () => { - const result = validateTtsCommand('spd-say'); - expect(result.valid).toBe(true); - expect(result.command).toBe('spd-say'); - }); - - it('should accept whitelisted command: festival', () => { - const result = validateTtsCommand('festival'); - expect(result.valid).toBe(true); - expect(result.command).toBe('festival'); - }); - - it('should accept whitelisted command: flite', () => { - const result = validateTtsCommand('flite'); - expect(result.valid).toBe(true); - expect(result.command).toBe('flite'); - }); - - it('should reject non-whitelisted command', () => { - const result = validateTtsCommand('rm'); - expect(result.valid).toBe(false); - expect(result.error).toContain('Invalid TTS command'); - expect(result.error).toContain('rm'); - }); - - it('should reject command injection attempt with &&', () => { - const result = validateTtsCommand('say && rm -rf /'); - expect(result.valid).toBe(false); - expect(result.error).toContain('arguments are not allowed'); - }); - - it('should reject command injection attempt with ;', () => { - const result = validateTtsCommand('say; rm -rf /'); - expect(result.valid).toBe(false); - // The semicolon makes 'say;' the base command, which is not whitelisted - expect(result.error).toContain('Invalid TTS command'); - }); - - it('should reject command injection attempt with |', () => { - const result = validateTtsCommand('say | cat /etc/passwd'); - expect(result.valid).toBe(false); - expect(result.error).toContain('arguments are not allowed'); - }); - - it('should reject command with arguments (security)', () => { - const result = validateTtsCommand('say -v Alex'); - expect(result.valid).toBe(false); - expect(result.error).toContain('arguments are not allowed'); - }); - - it('should reject command with subshell attempt', () => { - const result = validateTtsCommand('$(whoami)'); - expect(result.valid).toBe(false); - expect(result.error).toContain('Invalid TTS command'); - }); - - it('should reject command with backtick attempt', () => { - const result = validateTtsCommand('`whoami`'); - expect(result.valid).toBe(false); - expect(result.error).toContain('Invalid TTS command'); - }); - - it('should reject arbitrary shell command', () => { - const result = validateTtsCommand('/bin/bash -c "echo hacked"'); - expect(result.valid).toBe(false); - expect(result.error).toContain('Invalid TTS command'); - }); - - it('should reject curl command', () => { - const result = validateTtsCommand('curl http://evil.com'); - expect(result.valid).toBe(false); - expect(result.error).toContain('Invalid TTS command'); - }); - - it('should reject wget command', () => { - const result = validateTtsCommand('wget http://evil.com'); - expect(result.valid).toBe(false); - expect(result.error).toContain('Invalid TTS command'); - }); - - it('should handle command with leading whitespace', () => { - const result = validateTtsCommand(' say'); - expect(result.valid).toBe(true); - expect(result.command).toBe('say'); - }); - - it('should handle command with trailing whitespace', () => { - // Trailing whitespace is trimmed, so 'say ' becomes 'say' which is valid - const result = validateTtsCommand('say '); - expect(result.valid).toBe(true); - expect(result.command).toBe('say'); - }); - }); - - describe('TTS queue size limit', () => { + describe('notification queue size limit', () => { it('should reject requests when queue is full', async () => { const handler = handlers.get('notification:speak')!; const maxSize = getTtsMaxQueueSize(); // The flow is: - // 1. First call: item added to queue, processNextTts() shifts it out to process - // 2. executeTts() creates a spawn that never completes, so isTtsProcessing stays true - // 3. Subsequent calls: items are added to queue but not processed (isTtsProcessing is true) + // 1. First call: item added to queue, processNextNotification() shifts it out to process + // 2. executeNotificationCommand() creates a spawn that never completes, so isNotificationProcessing stays true + // 3. Subsequent calls: items are added to queue but not processed (isNotificationProcessing is true) // 4. Queue accumulates items 2 through maxSize (first one was shifted out) // 5. We need maxSize + 1 calls total to fill the queue to maxSize items // First call - this item gets shifted out of queue immediately for processing handler({}, 'Message 0'); - // Allow the async processNextTts to start (shifts item from queue) + // Allow the async processNextNotification to start (shifts item from queue) await new Promise((resolve) => setTimeout(resolve, 10)); - // Now isTtsProcessing is true, so subsequent items stay in queue + // Now isNotificationProcessing is true, so subsequent items stay in queue // Add maxSize more items - this should fill the queue to maxSize for (let i = 1; i <= maxSize; i++) { handler({}, `Message ${i}`); @@ -396,7 +315,7 @@ describe('Notification IPC Handlers', () => { expect(result.error).toContain('queue is full'); expect(result.error).toContain(`max ${maxSize}`); - // Clean up - reset all TTS state including clearing the queue + // Clean up - reset all notification state including clearing the queue resetTtsState(); }); }); diff --git a/src/__tests__/main/preload/notifications.test.ts b/src/__tests__/main/preload/notifications.test.ts index 517c9ae5..5457ef0c 100644 --- a/src/__tests__/main/preload/notifications.test.ts +++ b/src/__tests__/main/preload/notifications.test.ts @@ -65,10 +65,31 @@ describe('Notification Preload API', () => { expect(mockInvoke).toHaveBeenCalledWith('notification:speak', 'Hello', 'espeak'); expect(result.ttsId).toBe(456); }); + + it('should accept complex command chains with pipes', async () => { + mockInvoke.mockResolvedValue({ success: true, ttsId: 789 }); + + const complexCommand = 'tee ~/log.txt | say'; + const result = await api.speak('Test message', complexCommand); + + expect(mockInvoke).toHaveBeenCalledWith('notification:speak', 'Test message', complexCommand); + expect(result.ttsId).toBe(789); + }); + + it('should accept commands with full paths and arguments', async () => { + mockInvoke.mockResolvedValue({ success: true, ttsId: 111 }); + + const fullPathCommand = + '/Users/pedram/go/bin/fabric --pattern ped_summarize_conversational --model gpt-5-mini'; + const result = await api.speak('Test', fullPathCommand); + + expect(mockInvoke).toHaveBeenCalledWith('notification:speak', 'Test', fullPathCommand); + expect(result.success).toBe(true); + }); }); describe('stopSpeak', () => { - it('should invoke notification:stopSpeak with ttsId', async () => { + it('should invoke notification:stopSpeak with notificationId', async () => { mockInvoke.mockResolvedValue({ success: true }); const result = await api.stopSpeak(123); @@ -84,16 +105,16 @@ describe('Notification Preload API', () => { const cleanup = api.onTtsCompleted(callback); - expect(mockOn).toHaveBeenCalledWith('tts:completed', expect.any(Function)); + expect(mockOn).toHaveBeenCalledWith('notification:commandCompleted', expect.any(Function)); expect(typeof cleanup).toBe('function'); }); it('should call callback when event is received', () => { const callback = vi.fn(); - let registeredHandler: (event: unknown, ttsId: number) => void; + let registeredHandler: (event: unknown, notificationId: number) => void; mockOn.mockImplementation( - (_channel: string, handler: (event: unknown, ttsId: number) => void) => { + (_channel: string, handler: (event: unknown, notificationId: number) => void) => { registeredHandler = handler; } ); @@ -108,10 +129,10 @@ describe('Notification Preload API', () => { it('should remove listener when cleanup is called', () => { const callback = vi.fn(); - let registeredHandler: (event: unknown, ttsId: number) => void; + let registeredHandler: (event: unknown, notificationId: number) => void; mockOn.mockImplementation( - (_channel: string, handler: (event: unknown, ttsId: number) => void) => { + (_channel: string, handler: (event: unknown, notificationId: number) => void) => { registeredHandler = handler; } ); @@ -119,7 +140,10 @@ describe('Notification Preload API', () => { const cleanup = api.onTtsCompleted(callback); cleanup(); - expect(mockRemoveListener).toHaveBeenCalledWith('tts:completed', registeredHandler!); + expect(mockRemoveListener).toHaveBeenCalledWith( + 'notification:commandCompleted', + registeredHandler! + ); }); }); }); diff --git a/src/__tests__/renderer/components/TerminalOutput.test.tsx b/src/__tests__/renderer/components/TerminalOutput.test.tsx index 938a24cf..d39715d2 100644 --- a/src/__tests__/renderer/components/TerminalOutput.test.tsx +++ b/src/__tests__/renderer/components/TerminalOutput.test.tsx @@ -1018,70 +1018,6 @@ describe('TerminalOutput', () => { }); }); - describe('Custom notification functionality', () => { - it('shows speak button when audioFeedbackCommand is provided', () => { - const logs: LogEntry[] = [createLogEntry({ text: 'Text to speak', source: 'stdout' })]; - - const session = createDefaultSession({ - tabs: [{ id: 'tab-1', agentSessionId: 'claude-123', logs, isUnread: false }], - activeTabId: 'tab-1', - }); - - const props = createDefaultProps({ - session, - audioFeedbackCommand: 'say "{text}"', - }); - - render(); - - expect(screen.getByTitle('Speak text')).toBeInTheDocument(); - }); - - it('does not show speak button for user messages', () => { - const logs: LogEntry[] = [createLogEntry({ text: 'User message', source: 'user' })]; - - const session = createDefaultSession({ - tabs: [{ id: 'tab-1', agentSessionId: 'claude-123', logs, isUnread: false }], - activeTabId: 'tab-1', - }); - - const props = createDefaultProps({ - session, - audioFeedbackCommand: 'say "{text}"', - }); - - render(); - - expect(screen.queryByTitle('Speak text')).not.toBeInTheDocument(); - }); - - it('calls speak API when speak button is clicked', async () => { - const logs: LogEntry[] = [createLogEntry({ text: 'Text to speak', source: 'stdout' })]; - - const session = createDefaultSession({ - tabs: [{ id: 'tab-1', agentSessionId: 'claude-123', logs, isUnread: false }], - activeTabId: 'tab-1', - }); - - const props = createDefaultProps({ - session, - audioFeedbackCommand: 'say "{text}"', - }); - - render(); - - const speakButton = screen.getByTitle('Speak text'); - await act(async () => { - fireEvent.click(speakButton); - }); - - expect(window.maestro.notification.speak).toHaveBeenCalledWith( - 'Text to speak', - 'say "{text}"' - ); - }); - }); - describe('delete functionality', () => { it('shows delete button for user messages when onDeleteLog is provided', () => { const logs: LogEntry[] = [createLogEntry({ text: 'User message', source: 'user' })]; diff --git a/src/main/ipc/handlers/notifications.ts b/src/main/ipc/handlers/notifications.ts index 54b11860..5a284481 100644 --- a/src/main/ipc/handlers/notifications.ts +++ b/src/main/ipc/handlers/notifications.ts @@ -3,11 +3,11 @@ * * Handles all notification-related IPC operations: * - Showing OS notifications - * - Text-to-speech (TTS) functionality with queueing - * - Stopping active TTS processes + * - Custom notification commands with queueing + * - Stopping active notification processes * - * Security Note: TTS commands are validated against a whitelist to prevent - * command injection attacks from the renderer process. + * Note: Custom notification commands are user-configured and can be any command + * that accepts text via stdin. The user has full control over what command is executed. */ import { ipcMain, Notification, BrowserWindow } from 'electron'; @@ -20,44 +20,28 @@ import { isWebContentsAvailable } from '../../utils/safe-send'; // ========================================================================== /** - * Minimum delay between TTS calls to prevent audio overlap. + * Minimum delay between notification command calls to prevent audio overlap. * * 15 seconds was chosen to: - * 1. Allow sufficient time for most TTS messages to complete naturally + * 1. Allow sufficient time for most messages to complete naturally * 2. Prevent rapid-fire notifications from overwhelming the user - * 3. Give users time to process each audio message before the next one + * 3. Give users time to process each notification before the next one * - * This value balances responsiveness with preventing audio chaos when + * This value balances responsiveness with preventing notification chaos when * multiple notifications trigger in quick succession. */ -const TTS_MIN_DELAY_MS = 15000; +const NOTIFICATION_MIN_DELAY_MS = 15000; /** - * Maximum number of items allowed in the TTS queue. - * Prevents memory issues if TTS requests accumulate faster than they can be processed. + * Maximum number of items allowed in the notification queue. + * Prevents memory issues if requests accumulate faster than they can be processed. */ -const TTS_MAX_QUEUE_SIZE = 10; +const NOTIFICATION_MAX_QUEUE_SIZE = 10; /** - * Whitelist of allowed TTS commands to prevent command injection. - * - * These are common TTS commands across different platforms: - * - say: macOS built-in TTS - * - espeak: Linux TTS (common on Ubuntu/Debian) - * - espeak-ng: Modern fork of espeak - * - spd-say: Speech Dispatcher client (Linux) - * - festival: Festival TTS system (Linux) - * - flite: Lightweight TTS (Linux) - * - * SECURITY: Only the base command name is checked. Arguments are NOT allowed - * to be passed through the command parameter to prevent injection attacks. + * Default notification command (macOS TTS) */ -const ALLOWED_TTS_COMMANDS = ['say', 'espeak', 'espeak-ng', 'spd-say', 'festival', 'flite']; - -/** - * Default TTS command (macOS) - */ -const DEFAULT_TTS_COMMAND = 'say'; +const DEFAULT_NOTIFICATION_COMMAND = 'say'; // ========================================================================== // Types @@ -72,27 +56,27 @@ export interface NotificationShowResponse { } /** - * Response from TTS operations + * Response from custom notification command operations */ -export interface TtsResponse { +export interface NotificationCommandResponse { success: boolean; - ttsId?: number; + notificationId?: number; error?: string; } /** - * Item in the TTS queue + * Item in the notification command queue */ -interface TtsQueueItem { +interface NotificationQueueItem { text: string; command?: string; - resolve: (result: TtsResponse) => void; + resolve: (result: NotificationCommandResponse) => void; } /** - * Active TTS process tracking + * Active notification command process tracking */ -interface ActiveTtsProcess { +interface ActiveNotificationProcess { process: ChildProcess; command: string; } @@ -101,90 +85,52 @@ interface ActiveTtsProcess { // Module State // ========================================================================== -/** Track active TTS processes by ID for stopping */ -const activeTtsProcesses = new Map(); +/** Track active notification command processes by ID for stopping */ +const activeNotificationProcesses = new Map(); -/** Counter for generating unique TTS process IDs */ -let ttsProcessIdCounter = 0; +/** Counter for generating unique notification process IDs */ +let notificationProcessIdCounter = 0; -/** Timestamp when the last TTS completed */ -let lastTtsEndTime = 0; +/** Timestamp when the last notification command completed */ +let lastNotificationEndTime = 0; -/** Queue of pending TTS requests */ -const ttsQueue: TtsQueueItem[] = []; +/** Queue of pending notification command requests */ +const notificationQueue: NotificationQueueItem[] = []; -/** Flag indicating if TTS is currently being processed */ -let isTtsProcessing = false; +/** Flag indicating if notification command is currently being processed */ +let isNotificationProcessing = false; // ========================================================================== // Helper Functions // ========================================================================== /** - * Validate and sanitize TTS command to prevent command injection. + * Parse the notification command configuration. * - * SECURITY: This function is critical for preventing arbitrary command execution. - * It ensures only whitelisted commands can be executed, with no arguments allowed - * through the command parameter. + * The user can configure any command they want - this is intentional. + * The command is executed with shell: true to support pipes and command chains. * - * @param command - The requested TTS command - * @returns Object with validated command or error + * @param command - The user-configured notification command + * @returns The command to execute (or default if empty) */ -export function validateTtsCommand(command?: string): { - valid: boolean; - command: string; - error?: string; -} { +export function parseNotificationCommand(command?: string): string { // Use default if no command provided if (!command || command.trim() === '') { - return { valid: true, command: DEFAULT_TTS_COMMAND }; + return DEFAULT_NOTIFICATION_COMMAND; } - // Extract the base command (first word only, no arguments allowed) - const trimmedCommand = command.trim(); - const baseCommand = trimmedCommand.split(/\s+/)[0]; - - // Check if the base command is in the whitelist - if (!ALLOWED_TTS_COMMANDS.includes(baseCommand)) { - logger.warn('TTS command rejected - not in whitelist', 'TTS', { - requestedCommand: baseCommand, - allowedCommands: ALLOWED_TTS_COMMANDS, - }); - return { - valid: false, - command: DEFAULT_TTS_COMMAND, - error: `Invalid TTS command '${baseCommand}'. Allowed commands: ${ALLOWED_TTS_COMMANDS.join(', ')}`, - }; - } - - // If the command has arguments, reject it for security - if (trimmedCommand !== baseCommand) { - logger.warn('TTS command rejected - arguments not allowed', 'TTS', { - requestedCommand: trimmedCommand, - baseCommand, - }); - return { - valid: false, - command: DEFAULT_TTS_COMMAND, - error: `TTS command arguments are not allowed for security reasons. Use only the command name: ${baseCommand}`, - }; - } - - return { valid: true, command: baseCommand }; + return command.trim(); } /** - * Execute TTS - the actual implementation - * Returns a Promise that resolves when the TTS process completes (not just when it starts) + * Execute notification command - the actual implementation + * Returns a Promise that resolves when the process completes (not just when it starts) */ -async function executeTts(text: string, command?: string): Promise { - // Validate and sanitize the command - const validation = validateTtsCommand(command); - if (!validation.valid) { - return { success: false, error: validation.error }; - } - - const fullCommand = validation.command; +async function executeNotificationCommand( + text: string, + command?: string +): Promise { + const fullCommand = parseNotificationCommand(command); const textLength = text?.length || 0; const textPreview = text ? text.length > 200 @@ -193,7 +139,7 @@ async function executeTts(text: string, command?: string): Promise : '(no text)'; // Log the incoming request with full details for debugging - logger.info('TTS speak request received', 'TTS', { + logger.info('Notification command request received', 'Notification', { command: fullCommand, textLength, textPreview, @@ -201,23 +147,23 @@ async function executeTts(text: string, command?: string): Promise try { // Log the full command being executed - logger.debug('TTS executing command', 'TTS', { + logger.debug('Notification executing command', 'Notification', { command: fullCommand, textLength, }); - // Spawn the TTS process WITHOUT shell mode to prevent injection + // Spawn the process with shell mode to support pipes and command chains // The text is passed via stdin, not as command arguments const child = spawn(fullCommand, [], { stdio: ['pipe', 'ignore', 'pipe'], // stdin: pipe, stdout: ignore, stderr: pipe for errors - shell: false, // SECURITY: shell: false prevents command injection + shell: true, // Enable shell mode to support pipes (e.g., "cmd1 | cmd2") }); - // Generate a unique ID for this TTS process - const ttsId = ++ttsProcessIdCounter; - activeTtsProcesses.set(ttsId, { process: child, command: fullCommand }); + // Generate a unique ID for this notification process + const notificationId = ++notificationProcessIdCounter; + activeNotificationProcesses.set(notificationId, { process: child, command: fullCommand }); - // Return a Promise that resolves when the TTS process completes + // Return a Promise that resolves when the process completes return new Promise((resolve) => { let resolved = false; let stderrOutput = ''; @@ -233,27 +179,33 @@ async function executeTts(text: string, command?: string): Promise : undefined; if (errorCode === 'EPIPE') { - logger.debug('TTS stdin EPIPE - process closed before write completed', 'TTS'); + logger.debug( + 'Notification stdin EPIPE - process closed before write completed', + 'Notification' + ); } else { - logger.error('TTS stdin error', 'TTS', { error: String(err), code: errorCode }); + logger.error('Notification stdin error', 'Notification', { + error: String(err), + code: errorCode, + }); } }); - logger.debug('TTS writing to stdin', 'TTS', { textLength }); + logger.debug('Notification writing to stdin', 'Notification', { textLength }); child.stdin.write(text, 'utf8', (err) => { if (err) { - logger.error('TTS stdin write error', 'TTS', { error: String(err) }); + logger.error('Notification stdin write error', 'Notification', { error: String(err) }); } else { - logger.debug('TTS stdin write completed', 'TTS'); + logger.debug('Notification stdin write completed', 'Notification'); } child.stdin!.end(); }); } else { - logger.error('TTS no stdin available on child process', 'TTS'); + logger.error('Notification no stdin available on child process', 'Notification'); } child.on('error', (err) => { - logger.error('TTS spawn error', 'TTS', { + logger.error('Notification spawn error', 'Notification', { error: String(err), command: fullCommand, textPreview: text @@ -262,10 +214,10 @@ async function executeTts(text: string, command?: string): Promise : text : '(no text)', }); - activeTtsProcesses.delete(ttsId); + activeNotificationProcesses.delete(notificationId); if (!resolved) { resolved = true; - resolve({ success: false, ttsId, error: String(err) }); + resolve({ success: false, notificationId, error: String(err) }); } }); @@ -278,8 +230,8 @@ async function executeTts(text: string, command?: string): Promise child.on('close', (code, signal) => { // Always log close event for debugging production issues - logger.info('TTS process closed', 'TTS', { - ttsId, + logger.info('Notification process closed', 'Notification', { + notificationId, exitCode: code, signal, stderr: stderrOutput || '(none)', @@ -287,37 +239,37 @@ async function executeTts(text: string, command?: string): Promise }); if (code !== 0 && stderrOutput) { - logger.error('TTS process error output', 'TTS', { + logger.error('Notification process error output', 'Notification', { exitCode: code, stderr: stderrOutput, command: fullCommand, }); } - activeTtsProcesses.delete(ttsId); + activeNotificationProcesses.delete(notificationId); - // Notify renderer that TTS has completed + // Notify renderer that notification command has completed BrowserWindow.getAllWindows().forEach((win) => { if (isWebContentsAvailable(win)) { - win.webContents.send('tts:completed', ttsId); + win.webContents.send('notification:commandCompleted', notificationId); } }); - // Resolve the promise now that TTS has completed + // Resolve the promise now that process has completed if (!resolved) { resolved = true; - resolve({ success: code === 0, ttsId }); + resolve({ success: code === 0, notificationId }); } }); - logger.info('TTS process spawned successfully', 'TTS', { - ttsId, + logger.info('Notification process spawned successfully', 'Notification', { + notificationId, command: fullCommand, textLength, }); }); } catch (error) { - logger.error('TTS error starting audio feedback', 'TTS', { + logger.error('Notification error starting command', 'Notification', { error: String(error), command: fullCommand, textPreview, @@ -327,50 +279,50 @@ async function executeTts(text: string, command?: string): Promise } /** - * Process the next item in the TTS queue. + * Process the next item in the notification queue. * * Uses a flag-first approach to prevent race conditions: * 1. Check and set the processing flag atomically * 2. Then check the queue - * This ensures only one processNextTts call can proceed at a time. + * This ensures only one processNextNotification call can proceed at a time. */ -async function processNextTts(): Promise { +async function processNextNotification(): Promise { // Check queue first - if empty, nothing to do - if (ttsQueue.length === 0) return; + if (notificationQueue.length === 0) return; // Set flag BEFORE processing to prevent race condition - // where multiple calls could pass the isTtsProcessing check simultaneously - if (isTtsProcessing) return; - isTtsProcessing = true; + // where multiple calls could pass the isNotificationProcessing check simultaneously + if (isNotificationProcessing) return; + isNotificationProcessing = true; // Double-check queue after setting flag (another call might have emptied it) - if (ttsQueue.length === 0) { - isTtsProcessing = false; + if (notificationQueue.length === 0) { + isNotificationProcessing = false; return; } - const item = ttsQueue.shift()!; + const item = notificationQueue.shift()!; // Calculate delay needed to maintain minimum gap const now = Date.now(); - const timeSinceLastTts = now - lastTtsEndTime; - const delayNeeded = Math.max(0, TTS_MIN_DELAY_MS - timeSinceLastTts); + const timeSinceLastNotification = now - lastNotificationEndTime; + const delayNeeded = Math.max(0, NOTIFICATION_MIN_DELAY_MS - timeSinceLastNotification); if (delayNeeded > 0) { - logger.debug(`TTS queue waiting ${delayNeeded}ms before next speech`, 'TTS'); + logger.debug(`Notification queue waiting ${delayNeeded}ms before next command`, 'Notification'); await new Promise((resolve) => setTimeout(resolve, delayNeeded)); } - // Execute the TTS - const result = await executeTts(item.text, item.command); + // Execute the notification command + const result = await executeNotificationCommand(item.text, item.command); item.resolve(result); - // Record when this TTS ended - lastTtsEndTime = Date.now(); - isTtsProcessing = false; + // Record when this notification ended + lastNotificationEndTime = Date.now(); + isNotificationProcessing = false; // Process next item in queue - processNextTts(); + processNextNotification(); } // ========================================================================== @@ -406,60 +358,63 @@ export function registerNotificationsHandlers(): void { } ); - // Audio feedback using system TTS command - queued to prevent overlap + // Custom notification command - queued to prevent overlap ipcMain.handle( 'notification:speak', - async (_event, text: string, command?: string): Promise => { + async (_event, text: string, command?: string): Promise => { // Check queue size limit to prevent memory issues - if (ttsQueue.length >= TTS_MAX_QUEUE_SIZE) { - logger.warn('TTS queue is full, rejecting request', 'TTS', { - queueLength: ttsQueue.length, - maxSize: TTS_MAX_QUEUE_SIZE, + if (notificationQueue.length >= NOTIFICATION_MAX_QUEUE_SIZE) { + logger.warn('Notification queue is full, rejecting request', 'Notification', { + queueLength: notificationQueue.length, + maxSize: NOTIFICATION_MAX_QUEUE_SIZE, }); return { success: false, - error: `TTS queue is full (max ${TTS_MAX_QUEUE_SIZE} items). Please wait for current items to complete.`, + error: `Notification queue is full (max ${NOTIFICATION_MAX_QUEUE_SIZE} items). Please wait for current items to complete.`, }; } - // Add to queue and return a promise that resolves when this TTS completes - return new Promise((resolve) => { - ttsQueue.push({ text, command, resolve }); - logger.debug(`TTS queued, queue length: ${ttsQueue.length}`, 'TTS'); - processNextTts(); + // Add to queue and return a promise that resolves when this notification completes + return new Promise((resolve) => { + notificationQueue.push({ text, command, resolve }); + logger.debug(`Notification queued, queue length: ${notificationQueue.length}`, 'Notification'); + processNextNotification(); }); } ); - // Stop a running TTS process - ipcMain.handle('notification:stopSpeak', async (_event, ttsId: number): Promise => { - logger.debug('TTS stop requested', 'TTS', { ttsId }); + // Stop a running notification command process + ipcMain.handle( + 'notification:stopSpeak', + async (_event, notificationId: number): Promise => { + logger.debug('Notification stop requested', 'Notification', { notificationId }); - const ttsProcess = activeTtsProcesses.get(ttsId); - if (!ttsProcess) { - logger.debug('TTS no active process found', 'TTS', { ttsId }); - return { success: false, error: 'No active TTS process with that ID' }; + const notificationProcess = activeNotificationProcesses.get(notificationId); + if (!notificationProcess) { + logger.debug('Notification no active process found', 'Notification', { notificationId }); + return { success: false, error: 'No active notification process with that ID' }; + } + + try { + // Kill the process and all its children + notificationProcess.process.kill('SIGTERM'); + activeNotificationProcesses.delete(notificationId); + + logger.info('Notification process stopped', 'Notification', { + notificationId, + command: notificationProcess.command, + }); + + return { success: true }; + } catch (error) { + logger.error('Notification error stopping process', 'Notification', { + notificationId, + error: String(error), + }); + return { success: false, error: String(error) }; + } } - - try { - // Kill the process and all its children - ttsProcess.process.kill('SIGTERM'); - activeTtsProcesses.delete(ttsId); - - logger.info('TTS process stopped', 'TTS', { - ttsId, - command: ttsProcess.command, - }); - - return { success: true }; - } catch (error) { - logger.error('TTS error stopping process', 'TTS', { - ttsId, - error: String(error), - }); - return { success: false, error: String(error) }; - } - }); + ); } // ========================================================================== @@ -467,47 +422,47 @@ export function registerNotificationsHandlers(): void { // ========================================================================== /** - * Get the current TTS queue length (for testing) + * Get the current notification queue length (for testing) */ -export function getTtsQueueLength(): number { - return ttsQueue.length; +export function getNotificationQueueLength(): number { + return notificationQueue.length; } /** - * Get the count of active TTS processes (for testing) + * Get the count of active notification processes (for testing) */ -export function getActiveTtsCount(): number { - return activeTtsProcesses.size; +export function getActiveNotificationCount(): number { + return activeNotificationProcesses.size; } /** - * Clear the TTS queue (for testing) + * Clear the notification queue (for testing) */ -export function clearTtsQueue(): void { - ttsQueue.length = 0; +export function clearNotificationQueue(): void { + notificationQueue.length = 0; } /** - * Reset TTS state (for testing) + * Reset notification state (for testing) */ -export function resetTtsState(): void { - ttsQueue.length = 0; - activeTtsProcesses.clear(); - ttsProcessIdCounter = 0; - lastTtsEndTime = 0; - isTtsProcessing = false; +export function resetNotificationState(): void { + notificationQueue.length = 0; + activeNotificationProcesses.clear(); + notificationProcessIdCounter = 0; + lastNotificationEndTime = 0; + isNotificationProcessing = false; } /** - * Get the maximum TTS queue size (for testing) + * Get the maximum notification queue size (for testing) */ -export function getTtsMaxQueueSize(): number { - return TTS_MAX_QUEUE_SIZE; +export function getNotificationMaxQueueSize(): number { + return NOTIFICATION_MAX_QUEUE_SIZE; } -/** - * Get the list of allowed TTS commands (for testing) - */ -export function getAllowedTtsCommands(): string[] { - return [...ALLOWED_TTS_COMMANDS]; -} +// Legacy aliases for backward compatibility with existing tests +export const getTtsQueueLength = getNotificationQueueLength; +export const getActiveTtsCount = getActiveNotificationCount; +export const clearTtsQueue = clearNotificationQueue; +export const resetTtsState = resetNotificationState; +export const getTtsMaxQueueSize = getNotificationMaxQueueSize; diff --git a/src/main/preload/index.ts b/src/main/preload/index.ts index 430dd716..ddabd122 100644 --- a/src/main/preload/index.ts +++ b/src/main/preload/index.ts @@ -352,7 +352,7 @@ export type { // From notifications NotificationApi, NotificationShowResponse, - TtsResponse, + NotificationCommandResponse, } from './notifications'; export type { // From leaderboard diff --git a/src/main/preload/notifications.ts b/src/main/preload/notifications.ts index 954e1c6e..b7567108 100644 --- a/src/main/preload/notifications.ts +++ b/src/main/preload/notifications.ts @@ -3,8 +3,8 @@ * * Provides the window.maestro.notification namespace for: * - Showing OS notifications - * - Text-to-speech (TTS) functionality - * - TTS completion events + * - Custom notification commands (e.g., TTS, logging, etc.) + * - Notification command completion events */ import { ipcRenderer } from 'electron'; @@ -18,11 +18,11 @@ export interface NotificationShowResponse { } /** - * Response from TTS operations + * Response from notification command operations */ -export interface TtsResponse { +export interface NotificationCommandResponse { success: boolean; - ttsId?: number; + ttsId?: number; // Legacy name kept for backward compatibility error?: string; } @@ -40,29 +40,30 @@ export function createNotificationApi() { ipcRenderer.invoke('notification:show', title, body), /** - * Speak text using system TTS - * @param text - Text to speak - * @param command - Optional TTS command (default: 'say' on macOS) + * Execute a custom notification command (e.g., TTS, logging) + * @param text - Text to pass to the command via stdin + * @param command - Command to execute (default: 'say' on macOS) */ - speak: (text: string, command?: string): Promise => + speak: (text: string, command?: string): Promise => ipcRenderer.invoke('notification:speak', text, command), /** - * Stop a running TTS process - * @param ttsId - ID of the TTS process to stop + * Stop a running notification command process + * @param notificationId - ID of the notification process to stop */ - stopSpeak: (ttsId: number): Promise => - ipcRenderer.invoke('notification:stopSpeak', ttsId), + stopSpeak: (notificationId: number): Promise => + ipcRenderer.invoke('notification:stopSpeak', notificationId), /** - * Subscribe to TTS completion events - * @param handler - Callback when a TTS process completes + * Subscribe to notification command completion events + * @param handler - Callback when a notification command completes * @returns Cleanup function to unsubscribe */ - onTtsCompleted: (handler: (ttsId: number) => void): (() => void) => { - const wrappedHandler = (_event: Electron.IpcRendererEvent, ttsId: number) => handler(ttsId); - ipcRenderer.on('tts:completed', wrappedHandler); - return () => ipcRenderer.removeListener('tts:completed', wrappedHandler); + onTtsCompleted: (handler: (notificationId: number) => void): (() => void) => { + const wrappedHandler = (_event: Electron.IpcRendererEvent, notificationId: number) => + handler(notificationId); + ipcRenderer.on('notification:commandCompleted', wrappedHandler); + return () => ipcRenderer.removeListener('notification:commandCompleted', wrappedHandler); }, }; } diff --git a/src/renderer/App.tsx b/src/renderer/App.tsx index 6d84622b..46304202 100644 --- a/src/renderer/App.tsx +++ b/src/renderer/App.tsx @@ -12159,9 +12159,6 @@ You are taking over this conversation. Based on the context above, provide a bri // Unread filter showUnreadOnly, - // Audio feedback - audioFeedbackCommand, - // Setters setLogViewerSelectedLevels, setGitDiffPreview, diff --git a/src/renderer/components/MainPanel.tsx b/src/renderer/components/MainPanel.tsx index be952a42..d490222d 100644 --- a/src/renderer/components/MainPanel.tsx +++ b/src/renderer/components/MainPanel.tsx @@ -182,9 +182,6 @@ interface MainPanelProps { onStopBatchRun?: (sessionId?: string) => void; showConfirmation?: (message: string, onConfirm: () => void) => void; - // TTS settings - audioFeedbackCommand?: string; - // Tab management for AI sessions onTabSelect?: (tabId: string) => void; onTabClose?: (tabId: string) => void; @@ -1641,7 +1638,6 @@ export const MainPanel = React.memo( onDeleteLog={props.onDeleteLog} onRemoveQueuedItem={onRemoveQueuedItem} onInterrupt={handleInterrupt} - audioFeedbackCommand={props.audioFeedbackCommand} onScrollPositionChange={props.onScrollPositionChange} onAtBottomChange={props.onAtBottomChange} initialScrollTop={ diff --git a/src/renderer/components/NotificationsPanel.tsx b/src/renderer/components/NotificationsPanel.tsx index e8164cbd..7437e742 100644 --- a/src/renderer/components/NotificationsPanel.tsx +++ b/src/renderer/components/NotificationsPanel.tsx @@ -1,5 +1,5 @@ -import React, { useState } from 'react'; -import { Bell, Volume2, Clock, Square } from 'lucide-react'; +import React, { useState, useEffect } from 'react'; +import { Bell, Volume2, Clock, Square, Check, AlertCircle, Loader2 } from 'lucide-react'; import type { Theme } from '../types'; import { SettingCheckbox } from './SettingCheckbox'; import { ToggleButtonGroup } from './ToggleButtonGroup'; @@ -16,6 +16,8 @@ interface NotificationsPanelProps { theme: Theme; } +type TestStatus = 'idle' | 'running' | 'success' | 'error'; + export function NotificationsPanel({ osNotificationsEnabled, setOsNotificationsEnabled, @@ -27,8 +29,21 @@ export function NotificationsPanel({ setToastDuration, theme, }: NotificationsPanelProps) { - // TTS test state - const [testTtsId, setTestTtsId] = useState(null); + // Notification command test state + const [testNotificationId, setTestNotificationId] = useState(null); + const [testStatus, setTestStatus] = useState('idle'); + const [testError, setTestError] = useState(null); + + // Clear success/error status after a delay + useEffect(() => { + if (testStatus === 'success' || testStatus === 'error') { + const timer = setTimeout(() => { + setTestStatus('idle'); + setTestError(null); + }, 3000); + return () => clearTimeout(timer); + } + }, [testStatus]); return (
@@ -85,16 +100,20 @@ export function NotificationsPanel({ className="flex-1 p-2 rounded border bg-transparent outline-none text-sm font-mono" style={{ borderColor: theme.colors.border, color: theme.colors.textMain }} /> - {testTtsId !== null ? ( + {testNotificationId !== null ? ( )}
+ {/* Error message display */} + {testError && ( +

+ {testError} +

+ )}

Command that accepts text via stdin. Chain multiple commands using pipes (e.g.,{' '} void; copyToClipboard: (text: string) => void; - speakText?: (text: string, logId: string) => void; - stopSpeaking?: () => void; - speakingLogId: string | null; - audioFeedbackCommand?: string; // ANSI converter ansiConverter: Convert; // Markdown rendering mode for AI responses (when true, shows raw text) @@ -125,10 +119,6 @@ const LogItemComponent = memo( scrollContainerRef, setLightboxImage, copyToClipboard, - speakText, - stopSpeaking, - speakingLogId, - audioFeedbackCommand, ansiConverter, markdownEditMode, onToggleMarkdownEditMode, @@ -780,28 +770,6 @@ const LogItemComponent = memo( {markdownEditMode ? : } )} - {/* Speak/Stop Button - only show for non-user messages when TTS is configured */} - {audioFeedbackCommand && - log.source !== 'user' && - (speakingLogId === log.id ? ( - - ) : ( - - ))} {/* Replay button for user messages in AI mode */} {isUserMessage && isAIMode && onReplayMessage && (