From fa00464df93e6beb386f89c8a18478f0a8ce9153 Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Fri, 26 Jun 2026 02:19:25 -0700 Subject: [PATCH] Fix noisy Grok tool notifications (#6306) --- ...gent-hook-completion-notifications.test.ts | 78 +++++++++++++ src/shared/agent-hook-listener.test.ts | 39 +++++++ src/shared/agent-hook-listener.ts | 33 ++++++ tests/e2e/droid-notification.spec.ts | 106 +++++++++++++++++- tests/e2e/helpers/agent-hook-endpoint.ts | 29 +++++ 5 files changed, 284 insertions(+), 1 deletion(-) diff --git a/src/renderer/src/hooks/agent-hook-completion-notifications.test.ts b/src/renderer/src/hooks/agent-hook-completion-notifications.test.ts index fdaee70cf..a7efda947 100644 --- a/src/renderer/src/hooks/agent-hook-completion-notifications.test.ts +++ b/src/renderer/src/hooks/agent-hook-completion-notifications.test.ts @@ -1,6 +1,7 @@ /* eslint-disable max-lines -- Why: notification edge cases share one module-scoped coordinator, so keeping setup and regression cases together prevents brittle cross-file mock resets. */ import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' import type { ParsedAgentStatusPayload } from '../../../shared/agent-status-types' +import { createHookListenerState, normalizeHookPayload } from '../../../shared/agent-hook-listener' const dispatchTerminalNotification = vi.fn() @@ -414,6 +415,83 @@ describe('agent hook completion notifications', () => { expect(dispatchTerminalNotification).not.toHaveBeenCalled() }) + it('does not notify on Grok routine permission prompt notifications during tool use', async () => { + const { observeAgentHookCompletionForNotification } = + await import('./agent-hook-completion-notifications') + const listenerState = createHookListenerState() + const observeGrokHook = (payload: Record): void => { + const event = normalizeHookPayload( + listenerState, + 'grok', + { + paneKey, + tabId: 'tab-1', + worktreeId: 'wt-1', + payload + }, + 'production' + ) + if (!event) { + return + } + observeAgentHookCompletionForNotification({ + paneKey: event.paneKey, + worktreeId: event.worktreeId ?? 'wt-1', + payload: event.payload + }) + } + + observeGrokHook({ + hookEventName: 'user_prompt_submit', + prompt: 'run shell and glob' + }) + observeGrokHook({ + hookEventName: 'pre_tool_use', + toolName: 'Shell', + toolInput: { command: 'echo hi' } + }) + observeGrokHook({ + hookEventName: 'notification', + notificationType: 'permission_prompt', + message: 'Tool permission requested', + level: 'info' + }) + observeGrokHook({ + hookEventName: 'pre_tool_use', + toolName: 'Glob', + toolInput: { pattern: '**/package.json' } + }) + observeGrokHook({ + hookEventName: 'notification', + notificationType: 'permission_prompt', + message: 'Tool permission requested', + level: 'info' + }) + + expect(dispatchTerminalNotification).not.toHaveBeenCalled() + + observeGrokHook({ + hookEventName: 'stop', + lastAssistantMessage: 'Done.' + }) + vi.advanceTimersByTime(HOOK_DONE_QUIET_MS) + + expect(dispatchTerminalNotification).toHaveBeenCalledTimes(1) + expect(dispatchTerminalNotification).toHaveBeenCalledWith( + 'wt-1', + expect.objectContaining({ + source: 'agent-task-complete', + paneKey, + agentStatusSnapshot: expect.objectContaining({ + state: 'done', + agentType: 'grok', + prompt: 'run shell and glob', + lastAssistantMessage: 'Done.' + }) + }) + ) + }) + it('suppresses an internal milestone completion when hook work resumes before quiet', async () => { const { observeAgentHookCompletionForNotification } = await import('./agent-hook-completion-notifications') diff --git a/src/shared/agent-hook-listener.test.ts b/src/shared/agent-hook-listener.test.ts index f208450a7..e3d0297f2 100644 --- a/src/shared/agent-hook-listener.test.ts +++ b/src/shared/agent-hook-listener.test.ts @@ -1303,6 +1303,45 @@ describe('shared agent-hook-listener', () => { }) }) + it('ignores Grok routine permission prompt notifications during tool use', () => { + normalizeHookPayload( + state, + 'grok', + { paneKey: PANE_KEY, payload: { hookEventName: 'UserPromptSubmit', prompt: 'ship it' } }, + 'production' + ) + normalizeHookPayload( + state, + 'grok', + { + paneKey: PANE_KEY, + payload: { + hookEventName: 'PreToolUse', + toolName: 'Shell', + toolInput: { command: 'echo hi' } + } + }, + 'production' + ) + + const event = normalizeHookPayload( + state, + 'grok', + { + paneKey: PANE_KEY, + payload: { + hookEventName: 'Notification', + notificationType: 'permission_prompt', + message: 'Tool permission requested', + level: 'info' + } + }, + 'production' + ) + + expect(event).toBeNull() + }) + it('reads Grok final assistant text from chat history on Stop', () => { const tmpDir = mkdtempSync(join(tmpdir(), 'orca-grok-session-')) const sessionId = '019e37f4-5135-7b63-a4ab-6d13aa6bf528' diff --git a/src/shared/agent-hook-listener.ts b/src/shared/agent-hook-listener.ts index f3149fd79..497aa9b21 100644 --- a/src/shared/agent-hook-listener.ts +++ b/src/shared/agent-hook-listener.ts @@ -1864,6 +1864,28 @@ function isGrokPermissionNotification(message: string | undefined): boolean { ) } +function getGrokNotificationType(hookPayload: Record): string | undefined { + return ( + readString(hookPayload, 'notificationType') ?? + readString(hookPayload, 'notification_type') ?? + readString(hookPayload, 'type') + ) +} + +function isGrokRoutinePermissionPromptNotification( + notificationType: string | undefined, + message: string | undefined, + level: string | undefined +): boolean { + // Why: Grok emits this info notification before each tool even under + // bypassPermissions; PreToolUse already captures progress without paging users. + return ( + isGrokEvent(notificationType, 'permission_prompt') && + message?.trim().toLowerCase() === 'tool permission requested' && + (!level || level.trim().toLowerCase() === 'info') + ) +} + function isGrokIdleNotification(message: string | undefined): boolean { if (!message) { return false @@ -2860,6 +2882,8 @@ function normalizeGrokEvent( } const notificationMessage = readString(hookPayload, 'message') + const notificationType = getGrokNotificationType(hookPayload) + const notificationLevel = readString(hookPayload, 'level') let stateName: 'working' | 'waiting' | 'done' | null = null if ( isGrokEvent( @@ -2873,6 +2897,15 @@ function normalizeGrokEvent( stateName = 'working' } else if (isGrokEvent(eventName, 'stop', 'session_end')) { stateName = 'done' + } else if ( + isGrokEvent(eventName, 'notification') && + isGrokRoutinePermissionPromptNotification( + notificationType, + notificationMessage, + notificationLevel + ) + ) { + return null } else if ( isGrokEvent(eventName, 'notification') && isGrokPermissionNotification(notificationMessage) diff --git a/tests/e2e/droid-notification.spec.ts b/tests/e2e/droid-notification.spec.ts index b4e72ed8d..16dc2a69f 100644 --- a/tests/e2e/droid-notification.spec.ts +++ b/tests/e2e/droid-notification.spec.ts @@ -10,7 +10,11 @@ import { waitForTerminalOutput } from './helpers/terminal' import { ensureTerminalVisible, waitForActiveWorktree, waitForSessionReady } from './helpers/store' -import { emitCodexHookStatus, readHookEndpoint } from './helpers/agent-hook-endpoint' +import { + emitCodexHookStatus, + emitGrokHookPayload, + readHookEndpoint +} from './helpers/agent-hook-endpoint' type NotificationDispatch = { source?: string @@ -253,6 +257,106 @@ test.describe('Droid notifications', () => { .toBe(true) }) + test('Grok routine permission prompt hooks stay working and do not notify', async ({ + orcaPage, + electronApp + }) => { + await waitForSessionReady(orcaPage) + await waitForActiveWorktree(orcaPage) + await ensureTerminalVisible(orcaPage) + await waitForActiveTerminalManager(orcaPage, 30_000) + await installMainProcessNotificationDispatchSpy(electronApp) + const endpoint = await readHookEndpoint(electronApp) + + const ptyId = await waitForActivePanePtyId(orcaPage) + const readyMarker = `__GROK_HOOK_NOTIFY_READY_${Date.now()}__` + await sendToTerminal(orcaPage, ptyId, `printf '${readyMarker}\\n'\r`) + await waitForTerminalOutput(orcaPage, readyMarker) + + const { paneKey, worktreeId } = await waitForActivePaneHookDescriptor(orcaPage) + const prompt = `grok-hook-notify-${Date.now()}` + await emitGrokHookPayload(endpoint, { + paneKey, + worktreeId, + payload: { + hookEventName: 'user_prompt_submit', + prompt + } + }) + await expect + .poll( + async () => + (await getAgentStatuses(orcaPage)).some( + (status) => + status.agentType === 'grok' && status.state === 'working' && status.prompt === prompt + ), + { + timeout: 30_000, + message: 'Grok UserPromptSubmit hook did not reach renderer agent status' + } + ) + .toBe(true) + + await emitGrokHookPayload(endpoint, { + paneKey, + worktreeId, + payload: { + hookEventName: 'pre_tool_use', + toolName: 'Shell', + toolInput: { command: 'echo hi' } + } + }) + await emitGrokHookPayload(endpoint, { + paneKey, + worktreeId, + payload: { + hookEventName: 'notification', + notificationType: 'permission_prompt', + message: 'Tool permission requested', + level: 'info' + } + }) + + await orcaPage.waitForTimeout(500) + expect( + (await getAgentStatuses(orcaPage)).some( + (status) => + status.agentType === 'grok' && status.prompt === prompt && status.state === 'waiting' + ) + ).toBe(false) + expect( + (await getNotificationDispatches(electronApp)).filter( + (dispatch) => dispatch.source === 'agent-task-complete' + ) + ).toEqual([]) + + const finalMessage = `Grok hook completed ${Date.now()}` + await emitGrokHookPayload(endpoint, { + paneKey, + worktreeId, + payload: { + hookEventName: 'stop', + lastAssistantMessage: finalMessage + } + }) + await expect + .poll( + async () => + (await getAgentStatuses(orcaPage)).some( + (status) => + status.agentType === 'grok' && + status.state === 'done' && + status.prompt === prompt && + status.lastAssistantMessage === finalMessage + ), + { + timeout: 30_000, + message: 'Grok Stop hook did not reach renderer agent status' + } + ) + .toBe(true) + }) + test('recognized agent title completion dispatches one task-complete notification', async ({ orcaPage, electronApp diff --git a/tests/e2e/helpers/agent-hook-endpoint.ts b/tests/e2e/helpers/agent-hook-endpoint.ts index cca990cb5..84303ee26 100644 --- a/tests/e2e/helpers/agent-hook-endpoint.ts +++ b/tests/e2e/helpers/agent-hook-endpoint.ts @@ -89,3 +89,32 @@ export async function emitCodexHookStatus( throw new Error(`Codex hook POST returned ${response.status}`) } } + +export async function emitGrokHookPayload( + endpoint: AgentHookEndpoint, + event: { + paneKey: string + worktreeId: string + payload: Record + } +): Promise { + const [tabId] = event.paneKey.split(':') + const response = await fetch(`http://127.0.0.1:${endpoint.port}/hook/grok`, { + method: 'POST', + headers: { + 'Content-Type': 'application/json', + 'X-Orca-Agent-Hook-Token': endpoint.token + }, + body: JSON.stringify({ + paneKey: event.paneKey, + tabId, + worktreeId: event.worktreeId, + env: endpoint.env, + version: endpoint.version, + payload: event.payload + }) + }) + if (response.status !== 204) { + throw new Error(`Grok hook POST returned ${response.status}`) + } +}