From 6ed499d0b4e38aeff9b413e9142945c542d12f95 Mon Sep 17 00:00:00 2001 From: gatsby74 <166927047+gatsby74@users.noreply.github.com> Date: Sat, 11 Jul 2026 02:58:10 +0200 Subject: [PATCH] fix terminal false unread from title-only idle (#7818) * fix(terminal): ignore title-only idle while hooks are active * fix(terminal): preserve hook authority across title races Co-authored-by: Orca * fix(terminal): preserve confirmed process-exit notifications Co-authored-by: Orca * fix(terminal): preserve authoritative agent lifecycle Co-authored-by: Orca * fix(agent-status): keep lifecycle tracking bounded and ordered Co-authored-by: Orca --------- Co-authored-by: Jinwoo-H Co-authored-by: Orca --- .../agent-completion-coordinator-types.ts | 8 + .../agent-completion-coordinator.test.ts | 91 ++- .../agent-completion-coordinator.ts | 58 +- ...ent-completion-no-evidence-cadence.test.ts | 6 +- .../agent-hook-terminal-lifecycle.test.ts | 50 ++ .../agent-hook-terminal-lifecycle.ts | 26 + .../terminal-pane/pty-connection.test.ts | 622 ++++++++++++++++++ .../terminal-pane/pty-connection.ts | 235 ++++++- .../use-notification-dispatch.test.ts | 129 +++- .../use-notification-dispatch.ts | 44 +- ...gent-hook-completion-notifications.test.ts | 81 ++- .../agent-hook-completion-notifications.ts | 66 +- src/renderer/src/hooks/useIpcEvents.test.ts | 72 ++ src/renderer/src/hooks/useIpcEvents.ts | 5 + src/shared/agent-status-identity.ts | 3 +- src/shared/agent-status-types.ts | 9 + 16 files changed, 1419 insertions(+), 86 deletions(-) create mode 100644 src/renderer/src/components/terminal-pane/agent-hook-terminal-lifecycle.test.ts create mode 100644 src/renderer/src/components/terminal-pane/agent-hook-terminal-lifecycle.ts diff --git a/src/renderer/src/components/terminal-pane/agent-completion-coordinator-types.ts b/src/renderer/src/components/terminal-pane/agent-completion-coordinator-types.ts index 6855c2f28..097ecd792 100644 --- a/src/renderer/src/components/terminal-pane/agent-completion-coordinator-types.ts +++ b/src/renderer/src/components/terminal-pane/agent-completion-coordinator-types.ts @@ -1,5 +1,6 @@ import type { ParsedAgentStatusPayload } from '../../../../shared/agent-status-types' import type { GlobalSettings } from '../../../../shared/types' +import type { RecognizedAgentProcess } from '../../../../shared/agent-process-recognition' import type { RuntimeTerminalProcessInspection } from '@/runtime/runtime-terminal-inspection' export type AgentCompletionStatusSnapshot = ParsedAgentStatusPayload & { @@ -9,6 +10,7 @@ export type AgentCompletionStatusSnapshot = ParsedAgentStatusPayload & { export type AgentCompletionDispatchMeta = { source: 'hook' | 'title' | 'process-exit' quietedHookDone: boolean + terminalIdleConfirmed?: boolean agentStatus?: AgentCompletionStatusSnapshot } @@ -27,6 +29,12 @@ export type AgentCompletionCoordinatorOptions = { ) => Promise dispatchCompletion: (title: string, meta?: AgentCompletionDispatchMeta) => void dispatchAttention?: (title: string, meta: AgentAttentionDispatchMeta) => void + dispatchHookLifecycle?: (payload: AgentCompletionStatusSnapshot) => void + shouldSuppressProcessReplacementCompletion?: ( + exited: RecognizedAgentProcess, + replacement: RecognizedAgentProcess + ) => boolean + shouldSuppressConfirmedProcessExitCompletion?: (exited: RecognizedAgentProcess) => boolean isLive: () => boolean shouldPollProcessCadence?: () => boolean // Why: on hosts where one inspection forks a whole-process-table scan (local diff --git a/src/renderer/src/components/terminal-pane/agent-completion-coordinator.test.ts b/src/renderer/src/components/terminal-pane/agent-completion-coordinator.test.ts index 61ce8e339..d48f509c9 100644 --- a/src/renderer/src/components/terminal-pane/agent-completion-coordinator.test.ts +++ b/src/renderer/src/components/terminal-pane/agent-completion-coordinator.test.ts @@ -204,7 +204,11 @@ describe('agent completion coordinator', () => { // Second idle sample confirms the exit ~2 hidden polls (~6s) after it happened. await vi.advanceTimersByTimeAsync(3_000) expect(dispatchCompletion).toHaveBeenCalledTimes(1) - expect(dispatchCompletion).toHaveBeenCalledWith('codex') + expect(dispatchCompletion).toHaveBeenCalledWith('codex', { + source: 'process-exit', + quietedHookDone: false, + terminalIdleConfirmed: true + }) }) it('clears process evidence after agent exit so later non-agent spinner titles do not notify', async () => { @@ -272,7 +276,90 @@ describe('agent completion coordinator', () => { await flushAsyncTicks() expect(dispatchCompletion).toHaveBeenCalledTimes(1) - expect(dispatchCompletion).toHaveBeenCalledWith('codex') + expect(dispatchCompletion).toHaveBeenCalledWith('codex', { + source: 'process-exit', + quietedHookDone: false, + terminalIdleConfirmed: true + }) + }) + + it('does not mark an agent-to-agent process replacement as terminal idle', async () => { + let foregroundProcess = 'codex' + const dispatchCompletion = vi.fn() + const coordinator = createAgentCompletionCoordinator({ + paneKey: 'tab-1:leaf-1', + getPtyId: () => 'pty-1', + getSettings: () => null, + inspectProcess: vi.fn(async () => processResult(foregroundProcess)), + dispatchCompletion, + isLive: () => true + }) + + coordinator.startProcessTracking() + coordinator.observeTitle('Codex working') + await vi.advanceTimersByTimeAsync(2_000) + + foregroundProcess = 'claude' + await vi.advanceTimersByTimeAsync(750) + + expect(dispatchCompletion).toHaveBeenCalledWith('codex', { + source: 'process-exit', + quietedHookDone: false + }) + }) + + it('suppresses replacement completion before coordinator state mutation', async () => { + let foregroundProcess = 'codex' + const dispatchCompletion = vi.fn() + const coordinator = createAgentCompletionCoordinator({ + paneKey: 'tab-1:leaf-1', + getPtyId: () => 'pty-1', + getSettings: () => null, + inspectProcess: vi.fn(async () => processResult(foregroundProcess)), + dispatchCompletion, + shouldSuppressProcessReplacementCompletion: () => true, + isLive: () => true + }) + + coordinator.startProcessTracking() + coordinator.observeTitle('Codex working') + await vi.advanceTimersByTimeAsync(2_000) + + foregroundProcess = 'claude' + await vi.advanceTimersByTimeAsync(750) + expect(dispatchCompletion).not.toHaveBeenCalled() + + coordinator.observeTitle('Claude done') + expect(dispatchCompletion).toHaveBeenCalledTimes(1) + expect(dispatchCompletion).toHaveBeenCalledWith('Claude done') + }) + + it('suppresses confirmed process exit when the owner vetoes the exited process', async () => { + let foregroundProcess: string | null = 'codex' + const dispatchCompletion = vi.fn() + const shouldSuppressConfirmedProcessExitCompletion = vi.fn(() => true) + const coordinator = createAgentCompletionCoordinator({ + paneKey: 'tab-1:leaf-1', + getPtyId: () => 'pty-1', + getSettings: () => null, + inspectProcess: vi.fn(async () => processResult(foregroundProcess)), + dispatchCompletion, + shouldSuppressConfirmedProcessExitCompletion, + isLive: () => true + }) + + coordinator.startProcessTracking() + coordinator.observeTitle('Codex working') + await vi.advanceTimersByTimeAsync(2_000) + + foregroundProcess = null + await vi.advanceTimersByTimeAsync(1_500) + + expect(shouldSuppressConfirmedProcessExitCompletion).toHaveBeenCalledWith({ + agent: 'codex', + processName: 'codex' + }) + expect(dispatchCompletion).not.toHaveBeenCalled() }) it('suppresses process-exit backstop after a title completion already notified the turn', async () => { diff --git a/src/renderer/src/components/terminal-pane/agent-completion-coordinator.ts b/src/renderer/src/components/terminal-pane/agent-completion-coordinator.ts index f2a6dca11..fe3e8208a 100644 --- a/src/renderer/src/components/terminal-pane/agent-completion-coordinator.ts +++ b/src/renderer/src/components/terminal-pane/agent-completion-coordinator.ts @@ -277,6 +277,7 @@ export function createAgentCompletionCoordinator( title: string, optionsOverride: { quietedHookDone?: boolean + terminalIdleConfirmed?: boolean agentStatus?: AgentCompletionStatusSnapshot completionIdentity?: LastCompletionIdentity | null } = {} @@ -306,10 +307,16 @@ export function createAgentCompletionCoordinator( if (optionsOverride.completionIdentity) { lastCompletionIdentityByPaneKey.set(options.paneKey, optionsOverride.completionIdentity) } - if (optionsOverride.quietedHookDone === true) { + if (source === 'hook' && optionsOverride.agentStatus) { + options.dispatchHookLifecycle?.(optionsOverride.agentStatus) + } + if (optionsOverride.quietedHookDone === true || source === 'process-exit') { + // Why: confirmed process death is independent completion evidence; keep + // its provenance so stale hook rows cannot veto the notification later. options.dispatchCompletion(title, { source, - quietedHookDone: true, + quietedHookDone: optionsOverride.quietedHookDone === true, + ...(optionsOverride.terminalIdleConfirmed === true ? { terminalIdleConfirmed: true } : {}), ...(optionsOverride.agentStatus ? { agentStatus: optionsOverride.agentStatus } : {}) }) } else { @@ -326,6 +333,7 @@ export function createAgentCompletionCoordinator( return } lastAttentionToken = token + options.dispatchHookLifecycle?.(payload) options.dispatchAttention(payload.agentType ?? options.paneKey, { source: 'hook', agentStatus: payload @@ -438,13 +446,18 @@ export function createAgentCompletionCoordinator( pendingProcessExitAgent = null if (lastForegroundAgent?.agent !== process.agent) { if (lastForegroundAgent && hasAgentRunEvidence) { - dispatchCompletion('process-exit', lastForegroundAgent.processName, { - completionIdentity: { - source: 'process-exit', - identity: `${lastForegroundAgent.agent}:${lastForegroundAgent.processName}`, - agentIdentity: lastForegroundAgent.agent - } - }) + if ( + options.shouldSuppressProcessReplacementCompletion?.(lastForegroundAgent, process) !== + true + ) { + dispatchCompletion('process-exit', lastForegroundAgent.processName, { + completionIdentity: { + source: 'process-exit', + identity: `${lastForegroundAgent.agent}:${lastForegroundAgent.processName}`, + agentIdentity: lastForegroundAgent.agent + } + }) + } } processSession += 1 } @@ -486,13 +499,16 @@ export function createAgentCompletionCoordinator( } const exited = lastForegroundAgent pendingProcessExitAgent = null - dispatchCompletion('process-exit', exited.processName, { - completionIdentity: { - source: 'process-exit', - identity: `${exited.agent}:${exited.processName}`, - agentIdentity: exited.agent - } - }) + if (options.shouldSuppressConfirmedProcessExitCompletion?.(exited) !== true) { + dispatchCompletion('process-exit', exited.processName, { + terminalIdleConfirmed: true, + completionIdentity: { + source: 'process-exit', + identity: `${exited.agent}:${exited.processName}`, + agentIdentity: exited.agent + } + }) + } lastForegroundAgent = null clearAgentRunEvidence() } else { @@ -787,6 +803,7 @@ export function createAgentCompletionCoordinator( lastAttentionToken = null currentTurn += 1 dropPendingTitle() + options.dispatchHookLifecycle?.(payload) return } if (isAttentionHookState(payload.state)) { @@ -842,11 +859,10 @@ export function createAgentCompletionCoordinator( agentIdentity: hookCompletionAgentIdentity(payload) } : null - dispatchCompletion( - 'hook', - payload.agentType ?? options.paneKey, - lastCompletionIdentity ? { completionIdentity: lastCompletionIdentity } : {} - ) + dispatchCompletion('hook', payload.agentType ?? options.paneKey, { + agentStatus: payload, + ...(lastCompletionIdentity ? { completionIdentity: lastCompletionIdentity } : {}) + }) } } diff --git a/src/renderer/src/components/terminal-pane/agent-completion-no-evidence-cadence.test.ts b/src/renderer/src/components/terminal-pane/agent-completion-no-evidence-cadence.test.ts index b22cd614d..132815852 100644 --- a/src/renderer/src/components/terminal-pane/agent-completion-no-evidence-cadence.test.ts +++ b/src/renderer/src/components/terminal-pane/agent-completion-no-evidence-cadence.test.ts @@ -203,6 +203,10 @@ describe('agent completion no-evidence inspection cadence', () => { await vi.advanceTimersByTimeAsync(3_000) expect(inspectProcess.mock.calls.length).toBeGreaterThan(callsAtExit) expect(dispatchCompletion).toHaveBeenCalledTimes(1) - expect(dispatchCompletion).toHaveBeenCalledWith('codex') + expect(dispatchCompletion).toHaveBeenCalledWith('codex', { + source: 'process-exit', + quietedHookDone: false, + terminalIdleConfirmed: true + }) }) }) diff --git a/src/renderer/src/components/terminal-pane/agent-hook-terminal-lifecycle.test.ts b/src/renderer/src/components/terminal-pane/agent-hook-terminal-lifecycle.test.ts new file mode 100644 index 000000000..a3ebb0fb3 --- /dev/null +++ b/src/renderer/src/components/terminal-pane/agent-hook-terminal-lifecycle.test.ts @@ -0,0 +1,50 @@ +import { describe, expect, it, vi } from 'vitest' +import { + dispatchAgentHookTerminalLifecycle, + registerAgentHookTerminalLifecycleHandler +} from './agent-hook-terminal-lifecycle' + +describe('agent hook terminal lifecycle', () => { + it('routes pane-scoped lifecycle events until unregister', () => { + const handler = vi.fn() + const unregister = registerAgentHookTerminalLifecycleHandler('tab-1:leaf-1', handler) + + dispatchAgentHookTerminalLifecycle('tab-1:leaf-1', { + state: 'done', + prompt: 'finish', + agentType: 'codex' + }) + unregister() + dispatchAgentHookTerminalLifecycle('tab-1:leaf-1', { + state: 'working', + prompt: 'next', + agentType: 'codex' + }) + + expect(handler).toHaveBeenCalledTimes(1) + expect(handler).toHaveBeenCalledWith( + expect.objectContaining({ state: 'done', agentType: 'codex' }) + ) + }) + + it('does not let stale unregister remove a replacement pane handler', () => { + const staleHandler = vi.fn() + const replacementHandler = vi.fn() + const unregisterStale = registerAgentHookTerminalLifecycleHandler('tab-2:leaf-2', staleHandler) + const unregisterReplacement = registerAgentHookTerminalLifecycleHandler( + 'tab-2:leaf-2', + replacementHandler + ) + + unregisterStale() + dispatchAgentHookTerminalLifecycle('tab-2:leaf-2', { + state: 'blocked', + prompt: 'approval', + agentType: 'claude' + }) + unregisterReplacement() + + expect(staleHandler).not.toHaveBeenCalled() + expect(replacementHandler).toHaveBeenCalledTimes(1) + }) +}) diff --git a/src/renderer/src/components/terminal-pane/agent-hook-terminal-lifecycle.ts b/src/renderer/src/components/terminal-pane/agent-hook-terminal-lifecycle.ts new file mode 100644 index 000000000..b46dea1b7 --- /dev/null +++ b/src/renderer/src/components/terminal-pane/agent-hook-terminal-lifecycle.ts @@ -0,0 +1,26 @@ +import type { AgentCompletionStatusSnapshot } from './agent-completion-coordinator-types' + +type AgentHookTerminalLifecycleHandler = (payload: AgentCompletionStatusSnapshot) => void + +// Why: hook completion authority may live in the global IPC coordinator while +// cursor/cache effects belong to the mounted pane; route accepted events by pane. +const handlersByPaneKey = new Map() + +export function registerAgentHookTerminalLifecycleHandler( + paneKey: string, + handler: AgentHookTerminalLifecycleHandler +): () => void { + handlersByPaneKey.set(paneKey, handler) + return () => { + if (handlersByPaneKey.get(paneKey) === handler) { + handlersByPaneKey.delete(paneKey) + } + } +} + +export function dispatchAgentHookTerminalLifecycle( + paneKey: string, + payload: AgentCompletionStatusSnapshot +): void { + handlersByPaneKey.get(paneKey)?.(payload) +} diff --git a/src/renderer/src/components/terminal-pane/pty-connection.test.ts b/src/renderer/src/components/terminal-pane/pty-connection.test.ts index f4e801525..52457e170 100644 --- a/src/renderer/src/components/terminal-pane/pty-connection.test.ts +++ b/src/renderer/src/components/terminal-pane/pty-connection.test.ts @@ -15436,6 +15436,628 @@ describe('connectPanePty', () => { ) }) + it('ignores title-only idle while fresh hook status is still working', async () => { + const { connectPanePty } = await import('./pty-connection') + const transport = createMockTransport() + transportFactoryQueue.push(transport) + const paneKey = makePaneKey('tab-1', LEAF_1) + mockStoreState.agentStatusByPaneKey[paneKey] = { + state: 'working', + prompt: 'still thinking', + updatedAt: Date.now() - 60_000, + stateStartedAt: Date.now() - 60_000, + agentType: 'pi', + paneKey, + stateHistory: [] + } + + const pane = createPane(1) + const manager = createManager(1) + const deps = createDeps() + + connectPanePty(pane as never, manager as never, deps as never) + + const idleHandler = createdTransportOptions[0]?.onAgentBecameIdle as + | ((title: string) => void) + | undefined + if (!idleHandler) { + throw new Error('Expected onAgentBecameIdle to be registered') + } + + idleHandler('/var/folders/false-idle-title') + + expect(deps.dispatchNotification).not.toHaveBeenCalled() + expect(deps.setCacheTimerStartedAt).not.toHaveBeenCalledWith(paneKey, expect.any(Number)) + expect(pane.terminal.write).not.toHaveBeenCalledWith( + RESET_TERMINAL_CURSOR_STYLE, + expect.any(Function) + ) + }) + + it('allows an explicit idle title from a different agent than fresh hook status', async () => { + const { connectPanePty } = await import('./pty-connection') + const transport = createMockTransport() + transportFactoryQueue.push(transport) + vi.useFakeTimers() + const paneKey = makePaneKey('tab-1', LEAF_1) + mockStoreState.agentStatusByPaneKey[paneKey] = { + state: 'working', + prompt: 'previous agent state', + updatedAt: Date.now() - 60_000, + stateStartedAt: Date.now() - 60_000, + agentType: 'pi', + paneKey, + stateHistory: [] + } + + const pane = createPane(1) + const manager = createManager(1) + const deps = createDeps() + + connectPanePty(pane as never, manager as never, deps as never) + + const idleHandler = createdTransportOptions[0]?.onAgentBecameIdle as + | ((title: string) => void) + | undefined + if (!idleHandler) { + throw new Error('Expected onAgentBecameIdle to be registered') + } + + idleHandler('* Claude cross-agent done') + vi.advanceTimersByTime(AGENT_TASK_COMPLETE_NOTIFICATION_MAX_WAIT_MS) + + expect(deps.dispatchNotification).toHaveBeenCalledWith({ + source: 'agent-task-complete', + terminalTitle: '* Claude cross-agent done', + paneKey + }) + expect(pane.terminal.write).toHaveBeenCalledWith( + RESET_TERMINAL_CURSOR_STYLE, + expect.any(Function) + ) + }) + + it('ignores an explicit idle title while fresh hook identity is unknown', async () => { + const { connectPanePty } = await import('./pty-connection') + const transport = createMockTransport() + transportFactoryQueue.push(transport) + const paneKey = makePaneKey('tab-1', LEAF_1) + mockStoreState.agentStatusByPaneKey[paneKey] = { + state: 'working', + prompt: 'still working without a known agent identity', + updatedAt: Date.now() - 60_000, + stateStartedAt: Date.now() - 60_000, + agentType: 'unknown', + paneKey, + stateHistory: [] + } + + const pane = createPane(1) + const manager = createManager(1) + const deps = createDeps() + + connectPanePty(pane as never, manager as never, deps as never) + + const idleHandler = createdTransportOptions[0]?.onAgentBecameIdle as + | ((title: string) => void) + | undefined + if (!idleHandler) { + throw new Error('Expected onAgentBecameIdle to be registered') + } + + idleHandler('Claude Code done') + + expect(deps.dispatchNotification).not.toHaveBeenCalled() + expect(deps.setCacheTimerStartedAt).not.toHaveBeenCalled() + expect(pane.terminal.write).not.toHaveBeenCalledWith( + RESET_TERMINAL_CURSOR_STYLE, + expect.any(Function) + ) + }) + + it('ignores a Pi idle title while compatible OMP hook status is active', async () => { + const { connectPanePty } = await import('./pty-connection') + const transport = createMockTransport() + transportFactoryQueue.push(transport) + const paneKey = makePaneKey('tab-1', LEAF_1) + mockStoreState.agentStatusByPaneKey[paneKey] = { + state: 'working', + prompt: 'OMP is still working', + updatedAt: Date.now() - 60_000, + stateStartedAt: Date.now() - 60_000, + agentType: 'omp', + paneKey, + stateHistory: [] + } + + const pane = createPane(1) + const manager = createManager(1) + const deps = createDeps() + + connectPanePty(pane as never, manager as never, deps as never) + + const idleHandler = createdTransportOptions[0]?.onAgentBecameIdle as + | ((title: string) => void) + | undefined + if (!idleHandler) { + throw new Error('Expected onAgentBecameIdle to be registered') + } + + idleHandler('Pi ready') + + expect(deps.dispatchNotification).not.toHaveBeenCalled() + expect(pane.terminal.write).not.toHaveBeenCalledWith( + RESET_TERMINAL_CURSOR_STYLE, + expect.any(Function) + ) + }) + + it('preserves permission-title cursor and cache side effects through authoritative hook done', async () => { + const { connectPanePty } = await import('./pty-connection') + const transport = createMockTransport('pty-hook') + transportFactoryQueue.push(transport) + enableActiveRuntimeEnvironment() + const paneKey = makePaneKey('tab-1', LEAF_1) + mockStoreState.agentStatusByPaneKey[paneKey] = { + state: 'waiting', + prompt: 'approve the tool call', + updatedAt: Date.now(), + stateStartedAt: Date.now(), + agentType: 'claude', + paneKey, + stateHistory: [] + } + const pane = createPane(1) + const manager = createManager(1) + const deps = createDeps() + + connectPanePty(pane as never, manager as never, deps as never) + const idleHandler = createdTransportOptions[0]?.onAgentBecameIdle as + | ((title: string) => void) + | undefined + const statusHandler = createdTransportOptions[0]?.onAgentStatus as + | ((payload: { + state: 'done' + prompt: string + agentType: 'claude' + lastAssistantMessage: string + }) => void) + | undefined + if (!idleHandler || !statusHandler) { + throw new Error('Expected idle and hook status handlers to be registered') + } + + idleHandler('Claude Code permission') + + expect(deps.dispatchNotification).not.toHaveBeenCalled() + expect(deps.setCacheTimerStartedAt).not.toHaveBeenCalled() + expect(pane.terminal.write).toHaveBeenCalledWith( + RESET_TERMINAL_CURSOR_STYLE, + expect.any(Function) + ) + + statusHandler({ + state: 'done', + prompt: 'approve the tool call', + agentType: 'claude', + lastAssistantMessage: 'Done.' + }) + + expect(deps.setCacheTimerStartedAt).toHaveBeenCalledWith(paneKey, expect.any(Number)) + }) + + it('preserves a genuine hook completion after suppressing an earlier idle title', async () => { + const { connectPanePty } = await import('./pty-connection') + const transport = createMockTransport('pty-hook') + transportFactoryQueue.push(transport) + enableActiveRuntimeEnvironment() + vi.useFakeTimers() + const paneKey = makePaneKey('tab-1', LEAF_1) + mockStoreState.agentStatusByPaneKey[paneKey] = { + state: 'working', + prompt: 'still working', + updatedAt: Date.now(), + stateStartedAt: Date.now(), + agentType: 'claude', + paneKey, + stateHistory: [] + } + + const pane = createPane(1) + const manager = createManager(1) + const deps = createDeps() + + connectPanePty(pane as never, manager as never, deps as never) + + const titleHandler = createdTransportOptions[0]?.onTitleChange as + | ((title: string, rawTitle: string) => void) + | undefined + const idleHandler = createdTransportOptions[0]?.onAgentBecameIdle as + | ((title: string) => void) + | undefined + const statusHandler = createdTransportOptions[0]?.onAgentStatus as + | ((payload: { + state: 'done' + prompt: string + agentType: 'claude' + lastAssistantMessage: string + }) => void) + | undefined + if (!titleHandler || !idleHandler || !statusHandler) { + throw new Error('Expected title, idle, and hook status handlers to be registered') + } + + titleHandler('Claude working', 'Claude working') + titleHandler('Claude done', 'Claude done') + idleHandler('Claude done') + vi.advanceTimersByTime(AGENT_TASK_COMPLETE_NOTIFICATION_MAX_WAIT_MS) + + expect(deps.dispatchNotification).not.toHaveBeenCalled() + + statusHandler({ + state: 'done', + prompt: 'finish the implementation', + agentType: 'claude', + lastAssistantMessage: 'Done.' + }) + notifyStoreSubscribers() + expect(deps.setCacheTimerStartedAt).not.toHaveBeenCalledWith(paneKey, expect.any(Number)) + vi.advanceTimersByTime(AGENT_TASK_COMPLETE_NOTIFICATION_MAX_WAIT_MS * 2) + + expect(deps.dispatchNotification).toHaveBeenCalledWith( + expect.objectContaining({ + source: 'agent-task-complete', + paneKey, + agentStatusSnapshot: expect.objectContaining({ + state: 'done', + agentType: 'claude', + lastAssistantMessage: 'Done.' + }) + }) + ) + expect(deps.setCacheTimerStartedAt).toHaveBeenCalledWith(paneKey, expect.any(Number)) + expect(pane.terminal.write).toHaveBeenCalledWith( + RESET_TERMINAL_CURSOR_STYLE, + expect.any(Function) + ) + expect(storeSubscribers).toHaveLength(1) + }) + + it('applies accepted hook side effects when every completion alert consumer is disabled', async () => { + const { connectPanePty } = await import('./pty-connection') + const transport = createMockTransport('pty-hook') + transportFactoryQueue.push(transport) + enableActiveRuntimeEnvironment() + vi.useFakeTimers() + mockStoreState.settings = { + ...mockStoreState.settings, + experimentalTerminalAttention: false, + notifications: { + enabled: true, + agentTaskComplete: false, + terminalBell: true, + suppressWhenFocused: false, + customSoundPath: null + } + } + const paneKey = makePaneKey('tab-1', LEAF_1) + const pane = createPane(1) + const manager = createManager(1) + const deps = createDeps() + + connectPanePty(pane as never, manager as never, deps as never) + + const idleHandler = createdTransportOptions[0]?.onAgentBecameIdle as + | ((title: string) => void) + | undefined + const statusHandler = createdTransportOptions[0]?.onAgentStatus as + | ((payload: { + state: 'working' | 'done' + prompt: string + agentType: 'claude' + lastAssistantMessage?: string + }) => void) + | undefined + if (!idleHandler || !statusHandler) { + throw new Error('Expected idle and hook status handlers to be registered') + } + + statusHandler({ + state: 'working', + prompt: 'finish the implementation', + agentType: 'claude' + }) + idleHandler('Claude done') + + expect(deps.setCacheTimerStartedAt).not.toHaveBeenCalledWith(paneKey, expect.any(Number)) + expect(pane.terminal.write).not.toHaveBeenCalledWith( + RESET_TERMINAL_CURSOR_STYLE, + expect.any(Function) + ) + + statusHandler({ + state: 'done', + prompt: 'finish the implementation', + agentType: 'claude', + lastAssistantMessage: 'Done.' + }) + vi.advanceTimersByTime(AGENT_TASK_COMPLETE_NOTIFICATION_MAX_WAIT_MS) + + expect(deps.dispatchNotification).not.toHaveBeenCalled() + expect(deps.setCacheTimerStartedAt).toHaveBeenCalledWith(paneKey, expect.any(Number)) + expect(pane.terminal.write).toHaveBeenCalledWith( + RESET_TERMINAL_CURSOR_STYLE, + expect.any(Function) + ) + }) + + it.each([ + { + name: 'delivers confirmed process exit despite the same stale working hook row', + hookUpdateBeforeDispatch: 'none' + }, + { + name: 'delivers confirmed process exit after a same-turn working hook refresh', + hookUpdateBeforeDispatch: 'same-turn' + }, + { + name: 'delivers confirmed process exit after same-turn hook identity becomes known', + hookUpdateBeforeDispatch: 'same-turn-known-agent' + }, + { + name: 'cancels confirmed process exit delivery when a newer working hook row arrives', + hookUpdateBeforeDispatch: 'new-turn' + } + ] as const)('$name', async ({ hookUpdateBeforeDispatch }) => { + const restoreUserAgent = temporarilySetNavigatorUserAgent( + 'Mozilla/5.0 (Windows NT 10.0; Win64; x64)' + ) + const { connectPanePty } = await import('./pty-connection') + const transport = createMockTransport('pty-crashed-codex') + transportFactoryQueue.push(transport) + vi.useFakeTimers() + + try { + const paneKey = makePaneKey('tab-1', LEAF_1) + const crashedTurnStartedAt = Date.now() + const initialAgentType = + hookUpdateBeforeDispatch === 'same-turn-known-agent' ? 'unknown' : 'codex' + mockStoreState.agentStatusByPaneKey[paneKey] = { + state: 'working', + prompt: 'crash before done hook', + updatedAt: crashedTurnStartedAt, + stateStartedAt: crashedTurnStartedAt, + agentType: initialAgentType, + paneKey, + stateHistory: [] + } + const getForegroundProcess = vi.mocked(window.api.pty.getForegroundProcess) + getForegroundProcess.mockResolvedValue('codex') + const pane = createPane(1) + const manager = createManager(1) + const deps = createDeps() + + connectPanePty(pane as never, manager as never, deps as never) + await flushAsyncTicks() + const titleHandler = createdTransportOptions[0]?.onTitleChange as + | ((title: string, rawTitle: string) => void) + | undefined + if (!titleHandler) { + throw new Error('Expected onTitleChange to be registered') + } + + titleHandler('Codex working', 'Codex working') + await vi.advanceTimersByTimeAsync(2_500) + getForegroundProcess.mockResolvedValue(null) + await vi.advanceTimersByTimeAsync(1_800) + if (hookUpdateBeforeDispatch !== 'none') { + mockStoreState.agentStatusByPaneKey[paneKey] = { + state: 'working', + prompt: + hookUpdateBeforeDispatch === 'new-turn' + ? 'new turn after the prior process exited' + : 'same turn hook detail refresh', + updatedAt: Date.now(), + stateStartedAt: + hookUpdateBeforeDispatch === 'new-turn' ? Date.now() : crashedTurnStartedAt, + agentType: 'codex', + paneKey, + stateHistory: [] + } + notifyStoreSubscribers() + } + await vi.advanceTimersByTimeAsync(AGENT_TASK_COMPLETE_NOTIFICATION_MAX_WAIT_MS) + + const expectedNotification = { + source: 'agent-task-complete', + terminalTitle: 'codex', + paneKey, + agentCompletionSource: 'process-exit' + } + if (hookUpdateBeforeDispatch === 'new-turn') { + expect(deps.dispatchNotification).not.toHaveBeenCalledWith(expectedNotification) + } else { + expect(deps.dispatchNotification).toHaveBeenCalledWith(expectedNotification) + } + expect(pane.terminal.write).toHaveBeenCalledWith( + `${RESET_TERMINAL_CURSOR_STYLE}${RESET_KITTY_KEYBOARD_PROTOCOL}`, + expect.any(Function) + ) + } finally { + restoreUserAgent() + } + }) + + it('drops an exited agent completion when a replacement agent hook row is active', async () => { + const { connectPanePty } = await import('./pty-connection') + const transport = createMockTransport('pty-replaced-codex') + transportFactoryQueue.push(transport) + vi.useFakeTimers() + const getForegroundProcess = vi.mocked(window.api.pty.getForegroundProcess) + getForegroundProcess.mockResolvedValue('codex') + const paneKey = makePaneKey('tab-1', LEAF_1) + const pane = createPane(1) + const manager = createManager(1) + const deps = createDeps() + + connectPanePty(pane as never, manager as never, deps as never) + await flushAsyncTicks() + const titleHandler = createdTransportOptions[0]?.onTitleChange as + | ((title: string, rawTitle: string) => void) + | undefined + if (!titleHandler) { + throw new Error('Expected onTitleChange to be registered') + } + + titleHandler('Codex working', 'Codex working') + await vi.advanceTimersByTimeAsync(2_500) + mockStoreState.agentStatusByPaneKey[paneKey] = { + state: 'working', + prompt: 'replacement agent turn', + updatedAt: Date.now(), + stateStartedAt: Date.now(), + agentType: 'claude', + paneKey, + stateHistory: [] + } + getForegroundProcess.mockResolvedValue('claude') + await vi.advanceTimersByTimeAsync(1_000) + await vi.advanceTimersByTimeAsync(AGENT_TASK_COMPLETE_NOTIFICATION_MAX_WAIT_MS) + + expect(deps.dispatchNotification).not.toHaveBeenCalledWith( + expect.objectContaining({ source: 'agent-task-complete' }) + ) + expect(pane.terminal.write).not.toHaveBeenCalledWith( + RESET_TERMINAL_CURSOR_STYLE, + expect.any(Function) + ) + + mockStoreState.agentStatusByPaneKey[paneKey] = { + state: 'done', + prompt: 'replacement agent turn', + updatedAt: Date.now(), + stateStartedAt: Date.now(), + agentType: 'claude', + paneKey, + stateHistory: [] + } + titleHandler('Claude done', 'Claude done') + await vi.advanceTimersByTimeAsync(AGENT_TASK_COMPLETE_NOTIFICATION_MAX_WAIT_MS) + + expect(deps.dispatchNotification).toHaveBeenCalledWith( + expect.objectContaining({ + source: 'agent-task-complete', + terminalTitle: 'Claude done', + paneKey + }) + ) + }) + + it('drops confirmed idle exit when a different hook owner appears between null samples', async () => { + const { connectPanePty } = await import('./pty-connection') + const transport = createMockTransport('pty-replaced-codex') + transportFactoryQueue.push(transport) + vi.useFakeTimers() + const getForegroundProcess = vi.mocked(window.api.pty.getForegroundProcess) + getForegroundProcess.mockResolvedValue('codex') + const paneKey = makePaneKey('tab-1', LEAF_1) + const pane = createPane(1) + const manager = createManager(1) + const deps = createDeps() + + connectPanePty(pane as never, manager as never, deps as never) + await flushAsyncTicks() + const titleHandler = createdTransportOptions[0]?.onTitleChange as + | ((title: string, rawTitle: string) => void) + | undefined + const idleHandler = createdTransportOptions[0]?.onAgentBecameIdle as + | ((title: string) => void) + | undefined + if (!titleHandler || !idleHandler) { + throw new Error('Expected title and idle handlers to be registered') + } + + titleHandler('Codex working', 'Codex working') + await vi.advanceTimersByTimeAsync(2_500) + getForegroundProcess.mockResolvedValue(null) + await vi.advanceTimersByTimeAsync(800) + + mockStoreState.agentStatusByPaneKey[paneKey] = { + state: 'working', + prompt: 'replacement agent turn', + updatedAt: Date.now(), + stateStartedAt: Date.now(), + agentType: 'claude', + paneKey, + stateHistory: [] + } + idleHandler('Claude done') + await vi.advanceTimersByTimeAsync(800) + await vi.advanceTimersByTimeAsync(AGENT_TASK_COMPLETE_NOTIFICATION_MAX_WAIT_MS) + + expect(deps.dispatchNotification).not.toHaveBeenCalledWith( + expect.objectContaining({ source: 'agent-task-complete' }) + ) + expect(pane.terminal.write).not.toHaveBeenCalledWith( + RESET_TERMINAL_CURSOR_STYLE, + expect.any(Function) + ) + }) + + it('preserves replacement-agent title side effects through the process replacement veto', async () => { + const { dispatchAgentHookTerminalLifecycle } = await import('./agent-hook-terminal-lifecycle') + const { connectPanePty } = await import('./pty-connection') + const transport = createMockTransport('pty-replaced-codex') + transportFactoryQueue.push(transport) + vi.useFakeTimers() + const getForegroundProcess = vi.mocked(window.api.pty.getForegroundProcess) + getForegroundProcess.mockResolvedValue('codex') + const paneKey = makePaneKey('tab-1', LEAF_1) + const pane = createPane(1) + const manager = createManager(1) + const deps = createDeps() + + connectPanePty(pane as never, manager as never, deps as never) + await flushAsyncTicks() + const titleHandler = createdTransportOptions[0]?.onTitleChange as + | ((title: string, rawTitle: string) => void) + | undefined + const idleHandler = createdTransportOptions[0]?.onAgentBecameIdle as + | ((title: string) => void) + | undefined + if (!titleHandler || !idleHandler) { + throw new Error('Expected title and idle handlers to be registered') + } + + titleHandler('Codex working', 'Codex working') + await vi.advanceTimersByTimeAsync(2_500) + mockStoreState.agentStatusByPaneKey[paneKey] = { + state: 'working', + prompt: 'replacement agent turn', + updatedAt: Date.now(), + stateStartedAt: Date.now(), + agentType: 'claude', + paneKey, + stateHistory: [] + } + idleHandler('Claude done') + getForegroundProcess.mockResolvedValue('claude') + await vi.advanceTimersByTimeAsync(1_000) + + dispatchAgentHookTerminalLifecycle(paneKey, { + state: 'done', + prompt: 'replacement agent turn', + agentType: 'claude', + lastAssistantMessage: 'Done.' + }) + + expect(deps.setCacheTimerStartedAt).toHaveBeenCalledWith(paneKey, expect.any(Number)) + expect(pane.terminal.write).toHaveBeenCalledWith( + RESET_TERMINAL_CURSOR_STYLE, + expect.any(Function) + ) + }) + it('resets renderer cursor style when an agent becomes idle', async () => { const { connectPanePty } = await import('./pty-connection') const transport = createMockTransport() diff --git a/src/renderer/src/components/terminal-pane/pty-connection.ts b/src/renderer/src/components/terminal-pane/pty-connection.ts index ae7dbfbcf..19046aa00 100644 --- a/src/renderer/src/components/terminal-pane/pty-connection.ts +++ b/src/renderer/src/components/terminal-pane/pty-connection.ts @@ -120,7 +120,7 @@ import { resolveSshPaneConnectGate } from './ssh-pane-connect-gate' import { dispatchTerminalCommandFinishedEvent } from '@/hooks/terminal-command-finished-event' import { e2eConfig } from '@/lib/e2e-config' import { - AGENT_STATUS_STALE_AFTER_MS, + isFreshNonDoneAgentStatus, type AgentStatusEntry, type AgentType } from '../../../../shared/agent-status-types' @@ -135,12 +135,19 @@ import { type AgentInterruptInputIntent } from '../../../../shared/agent-interrupt-intent' import { createAgentCompletionCoordinator } from './agent-completion-coordinator' +import { + dispatchAgentHookTerminalLifecycle, + registerAgentHookTerminalLifecycleHandler +} from './agent-hook-terminal-lifecycle' import { createCodexAutoApprovalHookCompletionSuppressor, shouldSuppressCodexAutoApprovalSyntheticTitle, shouldSuppressCodexAutoApprovalStatus } from './codex-auto-approval-notification-suppression' -import type { AgentCompletionStatusSnapshot } from './agent-completion-coordinator-types' +import type { + AgentCompletionDispatchMeta, + AgentCompletionStatusSnapshot +} from './agent-completion-coordinator-types' import { markTerminalBracketedPasteInterrupted, observeTerminalBracketedPasteModeOutput @@ -951,6 +958,7 @@ export function connectPanePty( let agentTaskCompleteSettingsUnsubscribe: (() => void) | null = null let agentTaskCompleteNotificationGeneration = 0 let wasAgentTaskCompleteTrackingEnabled = isAgentTaskCompleteTrackingEnabled() + let requiresFreshWorkingForAgentTaskCompleteNotification = !wasAgentTaskCompleteTrackingEnabled let wasAgentTaskCompleteOsNotificationEnabled = isAgentTaskCompleteNotificationEnabled() let terminalBellNotificationTimer: ReturnType | null = null let pendingTerminalBellNotification = false @@ -1297,13 +1305,94 @@ export function connectPanePty( const isFreshActivePaneAgentEntry = ( entry: AgentStatusEntry | undefined ): entry is AgentStatusEntry => { - return ( - !!entry && - typeof entry.updatedAt === 'number' && - Date.now() - entry.updatedAt <= AGENT_STATUS_STALE_AFTER_MS && - entry.state !== 'done' - ) + return isFreshNonDoneAgentStatus(entry) } + const shouldSuppressTitleCompletionForFreshHook = ( + title: string, + activeHookStatus: AgentStatusEntry | undefined + ): boolean => { + if ( + detectAgentStatusFromTitle(title) === 'working' || + !isFreshNonDoneAgentStatus(activeHookStatus) + ) { + return false + } + const explicitTitleAgentType = resolveCommittedTitleAgentType(title) + const activeHookAgentForTitle = resolveCompatibleAgentTypeForOwner( + activeHookStatus?.agentType, + explicitTitleAgentType + ) + const titleNamesDifferentKnownAgent = + explicitTitleAgentType && + activeHookStatus?.agentType && + activeHookStatus.agentType !== 'unknown' && + activeHookAgentForTitle !== explicitTitleAgentType + return !titleNamesDifferentKnownAgent + } + let pendingSuppressedTitleSideEffects: { + title: string + agentType: AgentType | undefined + } | null = null + const clearSuppressedTitleSideEffects = (): void => { + pendingSuppressedTitleSideEffects = null + } + const applyAgentCompletionSideEffects = ( + title: string, + agentType: AgentType | undefined + ): void => { + const settings = useAppStore.getState().settings + if ( + (agentType === 'claude' || isClaudeAgent(title)) && + (settings === null || settings.promptCacheTimerEnabled) + ) { + deps.setCacheTimerStartedAt(cacheKey, Date.now()) + } + queueAgentIdleTerminalModeReset() + } + const preserveSuppressedTitleSideEffects = ( + title: string, + activeHookStatus: AgentStatusEntry + ): void => { + pendingSuppressedTitleSideEffects = { + title, + agentType: activeHookStatus.agentType + } + if (activeHookStatus.state === 'waiting' || activeHookStatus.state === 'blocked') { + queueAgentIdleTerminalModeReset() + } + } + const handleAgentHookTerminalLifecycle = (payload: AgentCompletionStatusSnapshot): void => { + const pending = pendingSuppressedTitleSideEffects + if (!pending) { + return + } + const payloadAgentForPending = resolveCompatibleAgentTypeForOwner( + payload.agentType, + pending.agentType + ) + const belongsToPendingAgent = + !pending.agentType || + pending.agentType === 'unknown' || + !payload.agentType || + payload.agentType === 'unknown' || + payloadAgentForPending === pending.agentType + if (!belongsToPendingAgent || payload.state === 'working') { + clearSuppressedTitleSideEffects() + return + } + if (payload.state === 'done') { + applyAgentCompletionSideEffects(pending.title, payload.agentType ?? pending.agentType) + clearSuppressedTitleSideEffects() + return + } + if (payload.state === 'waiting' || payload.state === 'blocked') { + queueAgentIdleTerminalModeReset() + } + } + const unregisterAgentHookTerminalLifecycle = registerAgentHookTerminalLifecycleHandler( + cacheKey, + handleAgentHookTerminalLifecycle + ) const hasFreshPaneAgentSurface = (): boolean => { const entry = useAppStore.getState().agentStatusByPaneKey[cacheKey] if (isFreshActivePaneAgentEntry(entry)) { @@ -1934,11 +2023,47 @@ export function connectPanePty( getPtyId: () => transport.getPtyId(), getSettings: () => useAppStore.getState().settings, inspectProcess: inspectRuntimeTerminalProcess, - dispatchCompletion: (title, meta) => + dispatchHookLifecycle: (payload) => dispatchAgentHookTerminalLifecycle(cacheKey, payload), + shouldSuppressProcessReplacementCompletion: (_exited, replacement) => { + const currentStatus = useAppStore.getState().agentStatusByPaneKey[cacheKey] + const currentAgentForReplacement = resolveCompatibleAgentTypeForOwner( + currentStatus?.agentType, + replacement.agent + ) + return ( + isFreshNonDoneAgentStatus(currentStatus) && currentAgentForReplacement === replacement.agent + ) + }, + shouldSuppressConfirmedProcessExitCompletion: (exited) => { + const currentStatus = useAppStore.getState().agentStatusByPaneKey[cacheKey] + const currentAgentForExited = resolveCompatibleAgentTypeForOwner( + currentStatus?.agentType, + exited.agent + ) + // Why: a replacement hook can lead process visibility by one cadence; + // only a different known active owner can veto confirmed old-process exit. + return Boolean( + isFreshNonDoneAgentStatus(currentStatus) && + currentStatus.agentType && + currentStatus.agentType !== 'unknown' && + currentAgentForExited !== exited.agent + ) + }, + dispatchCompletion: (title, meta) => { + if (meta?.source === 'process-exit') { + clearSuppressedTitleSideEffects() + } + if (meta?.terminalIdleConfirmed === true) { + // Why: an agent can crash before its done hook; confirmed process death + // must still restore cursor and native Windows Kitty keyboard modes. + queueAgentIdleTerminalModeReset() + } scheduleAgentTaskCompleteNotification(title, { allowDoneDetailAfterGrace: meta?.quietedHookDone, + ...(meta?.source === 'process-exit' ? { agentCompletionSource: meta.source } : {}), ...(meta?.agentStatus ? { agentStatusSnapshot: meta.agentStatus } : {}) - }), + }) + }, dispatchAttention: (title, meta) => scheduleAgentTaskCompleteNotification(title, { agentStatusSnapshot: meta.agentStatus @@ -2254,7 +2379,12 @@ export function connectPanePty( // feed completion tracking — observeTitle would classify the cleared // title as idle and mint a task-complete for a merely-paused agent. if (!meta?.staleWorkingTitleClear && syncAgentTaskCompleteTrackingEnabled()) { - agentCompletionCoordinator.observeTitle(decision.rawTitle) + const activeHookStatus = useAppStore.getState().agentStatusByPaneKey[cacheKey] + if (!shouldSuppressTitleCompletionForFreshHook(decision.rawTitle, activeHookStatus)) { + // Why: display titles still update while hooks are active, but a stale + // idle frame must not complete the coordinator turn before hook `done`. + agentCompletionCoordinator.observeTitle(decision.rawTitle) + } } // Why: only the focused pane should drive the tab title — otherwise two // agents in split panes cause rapid title flickering as each emits OSC @@ -2521,17 +2651,17 @@ export function connectPanePty( } if (!enabled && wasAgentTaskCompleteTrackingEnabled) { // Why: disabling every completion consumer is an event-time boundary. - // Drop pending timers and coordinator state so old work cannot replay. + // Drop pending alerts while preserving accepted-hook lifecycle state. agentTaskCompleteNotificationGeneration += 1 + requiresFreshWorkingForAgentTaskCompleteNotification = true clearPendingAgentTaskCompleteNotification() - agentCompletionCoordinator.resetCompletionState({ requireFreshWorking: true }) if (pendingTerminalBellNotification) { scheduleTerminalBellNotification() } } else if (enabled && !wasAgentTaskCompleteTrackingEnabled) { // Why: a pane may have observed work while all completion consumers were // disabled. Re-enabling should not let the next idle event report old work. - agentCompletionCoordinator.resetCompletionState({ requireFreshWorking: true }) + requiresFreshWorkingForAgentTaskCompleteNotification = true } wasAgentTaskCompleteTrackingEnabled = enabled wasAgentTaskCompleteOsNotificationEnabled = osNotificationsEnabled @@ -2543,20 +2673,49 @@ export function connectPanePty( options: { allowDoneDetailAfterGrace?: boolean agentStatusSnapshot?: AgentCompletionStatusSnapshot + agentCompletionSource?: AgentCompletionDispatchMeta['source'] } = {} ): void => { - if (!syncAgentTaskCompleteTrackingEnabled()) { + if ( + !syncAgentTaskCompleteTrackingEnabled() || + requiresFreshWorkingForAgentTaskCompleteNotification + ) { return } clearPendingAgentTaskCompleteNotification() let graceElapsed = false const generationAtSchedule = agentTaskCompleteNotificationGeneration + const agentStatusAtSchedule = useAppStore.getState().agentStatusByPaneKey[cacheKey] + const hasNewerActiveHookStatus = (): boolean => { + const currentStatus = useAppStore.getState().agentStatusByPaneKey[cacheKey] + const scheduledAgentType = agentStatusAtSchedule?.agentType + const currentAgentForScheduledTurn = resolveCompatibleAgentTypeForOwner( + currentStatus?.agentType, + scheduledAgentType + ) + const hasDifferentKnownAgent = Boolean( + currentStatus?.agentType && + scheduledAgentType && + currentStatus.agentType !== 'unknown' && + scheduledAgentType !== 'unknown' && + currentAgentForScheduledTurn !== scheduledAgentType + ) + return ( + options.agentCompletionSource === 'process-exit' && + isFreshNonDoneAgentStatus(currentStatus) && + (!agentStatusAtSchedule || + currentStatus.state !== agentStatusAtSchedule.state || + currentStatus.stateStartedAt !== agentStatusAtSchedule.stateStartedAt || + hasDifferentKnownAgent) + ) + } const dispatch = (): void => { clearPendingAgentTaskCompleteNotification() if ( generationAtSchedule !== agentTaskCompleteNotificationGeneration || - !syncAgentTaskCompleteTrackingEnabled() + !syncAgentTaskCompleteTrackingEnabled() || + hasNewerActiveHookStatus() ) { return } @@ -2573,12 +2732,21 @@ export function connectPanePty( source: 'agent-task-complete', terminalTitle: title, paneKey: cacheKey, + ...(options.agentCompletionSource + ? { agentCompletionSource: options.agentCompletionSource } + : {}), ...(shouldDispatchOsNotification ? {} : { suppressOsNotification: true }), ...(options.agentStatusSnapshot ? { agentStatusSnapshot: options.agentStatusSnapshot } : {}) }) } const dispatchIfDetailed = (): void => { + if (hasNewerActiveHookStatus()) { + // Why: the confirmed exit belongs to the row captured above; a replaced + // active row means a newer turn started during the notification delay. + clearPendingAgentTaskCompleteNotification() + return + } if (!graceElapsed) { return } @@ -2629,6 +2797,16 @@ export function connectPanePty( deps.setCacheTimerStartedAt(cacheKey, null) return } + const currentState = useAppStore.getState() + const activeHookStatus = currentState.agentStatusByPaneKey[cacheKey] + if (shouldSuppressTitleCompletionForFreshHook(title, activeHookStatus)) { + // Why: agent CLIs can briefly publish an idle title while hook status + // still says the same agent turn is active (e.g. during tool output). + if (activeHookStatus) { + preserveSuppressedTitleSideEffects(title, activeHookStatus) + } + return + } // Why: only start the prompt-cache countdown for Claude agents — other // agents have different (or no) prompt-caching semantics and showing a // timer for them would be misleading. @@ -2639,7 +2817,7 @@ export function connectPanePty( // tab silently drops the timer. Writing a timestamp is cheap and the // CacheTimer component gates rendering on the enabled flag, so a // spurious write when the feature turns out to be disabled is harmless. - const settings = useAppStore.getState().settings + const settings = currentState.settings if (isClaudeAgent(title) && (settings === null || settings.promptCacheTimerEnabled)) { deps.setCacheTimerStartedAt(cacheKey, Date.now()) } @@ -2651,7 +2829,9 @@ export function connectPanePty( queueAgentIdleTerminalModeReset() } const onAgentBecameWorking = (): void => { + clearSuppressedTitleSideEffects() if (syncAgentTaskCompleteTrackingEnabled()) { + requiresFreshWorkingForAgentTaskCompleteNotification = false agentCompletionCoordinator.observeTitleWorking() } // Why: a new API call refreshes the prompt-cache TTL, so clear any running @@ -2663,6 +2843,7 @@ export function connectPanePty( } } const onAgentExited = (): void => { + clearSuppressedTitleSideEffects() clearCommandInferredPaneAgent() // Why: when the terminal title reverts to a plain shell (e.g., "bash", "zsh"), // the agent has exited. Clear any running cache timer so the sidebar doesn't @@ -2917,14 +3098,18 @@ export function connectPanePty( } else { currentState.setAgentStatus(cacheKey, statusPayload, statusTitle) } - if (syncAgentTaskCompleteTrackingEnabled()) { - const storedStatus = useAppStore.getState().agentStatusByPaneKey[cacheKey] - const notificationPayload = - typeof storedStatus?.stateStartedAt === 'number' - ? { ...statusPayload, stateStartedAt: storedStatus.stateStartedAt } - : statusPayload - agentCompletionCoordinator.observeHookStatus(notificationPayload) + const trackingEnabled = syncAgentTaskCompleteTrackingEnabled() + if (payload.state === 'working' && trackingEnabled) { + requiresFreshWorkingForAgentTaskCompleteNotification = false } + const storedStatus = useAppStore.getState().agentStatusByPaneKey[cacheKey] + const notificationPayload = + typeof storedStatus?.stateStartedAt === 'number' + ? { ...statusPayload, stateStartedAt: storedStatus.stateStartedAt } + : statusPayload + // Why: hook lifecycle owns deferred terminal side effects even when + // every outward completion alert consumer is disabled. + agentCompletionCoordinator.observeHookStatus(notificationPayload) if (payload.state === 'working' && pendingTerminalBellNotification) { scheduleTerminalBellNotification() } @@ -6985,6 +7170,8 @@ export function connectPanePty( } cleanupStartupDraftPasteTimers() releaseUnattemptedStartupDraftPasteDelivery() + unregisterAgentHookTerminalLifecycle() + clearSuppressedTitleSideEffects() clearPendingAgentTaskCompleteNotification() pendingTerminalBellNotification = false clearTerminalBellNotificationTimer() diff --git a/src/renderer/src/components/terminal-pane/use-notification-dispatch.test.ts b/src/renderer/src/components/terminal-pane/use-notification-dispatch.test.ts index c97252353..1ca3490ef 100644 --- a/src/renderer/src/components/terminal-pane/use-notification-dispatch.test.ts +++ b/src/renderer/src/components/terminal-pane/use-notification-dispatch.test.ts @@ -1,6 +1,9 @@ import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' import { dispatchTerminalNotification } from './use-notification-dispatch' -import type { AgentStatusEntry } from '../../../../shared/agent-status-types' +import { + AGENT_STATUS_STALE_AFTER_MS, + type AgentStatusEntry +} from '../../../../shared/agent-status-types' import type { TerminalLayoutSnapshot } from '../../../../shared/types' import { buildAgentNotificationId } from '../../../../shared/agent-notification-id' @@ -449,11 +452,12 @@ describe('dispatchTerminalNotification', () => { expect(mockState.markTerminalPaneUnread).toHaveBeenCalledWith(paneKey) }) - it('does not reuse a fresh stale agent snapshot when the terminal title names another agent', () => { + it('does not let fresh active status suppress a completion from another named agent', () => { mockState.agentStatusByPaneKey[paneKey] = makeAgentStatus(paneKey, { + state: 'working', agentType: 'codex', terminalTitle: 'Codex', - lastAssistantMessage: 'Codex done.' + lastAssistantMessage: undefined }) dispatchTerminalNotification('wt-primary', { @@ -554,6 +558,125 @@ describe('dispatchTerminalNotification', () => { ) }) + it('drops a title-only completion when fresh hook state is still active', () => { + mockState.agentStatusByPaneKey[paneKey] = makeAgentStatus(paneKey, { + state: 'working', + prompt: 'still running', + updatedAt: Date.now() - 60_000, + stateStartedAt: Date.now() - 60_000, + lastAssistantMessage: undefined + }) + + dispatchTerminalNotification('wt-primary', { + source: 'agent-task-complete', + terminalTitle: '✳ Launch UI and thumbnail generator', + paneKey + }) + + expect(window.api.notifications.dispatch).not.toHaveBeenCalled() + expect(mockState.markWorktreeUnread).not.toHaveBeenCalled() + expect(mockState.markAgentCompletionPaneUnread).not.toHaveBeenCalled() + expect(mockState.markTerminalTabUnread).not.toHaveBeenCalled() + expect(mockState.markTerminalPaneUnread).not.toHaveBeenCalled() + }) + + it('allows confirmed process-exit completion while fresh hook state is still active', () => { + mockState.agentStatusByPaneKey[paneKey] = makeAgentStatus(paneKey, { + state: 'working', + prompt: 'agent crashed before its done hook', + updatedAt: Date.now() - 60_000, + stateStartedAt: Date.now() - 60_000, + lastAssistantMessage: undefined + }) + + dispatchTerminalNotification('wt-primary', { + source: 'agent-task-complete', + terminalTitle: 'codex', + paneKey, + agentCompletionSource: 'process-exit' + }) + + expect(window.api.notifications.dispatch).toHaveBeenCalledWith( + expect.objectContaining({ + source: 'agent-task-complete', + worktreeId: 'wt-primary', + paneKey, + terminalTitle: 'codex' + }) + ) + expect(mockState.markWorktreeUnread).toHaveBeenCalledWith('wt-primary') + expect(mockState.markTerminalTabUnread).toHaveBeenCalledWith('tab-1') + expect(mockState.markTerminalPaneUnread).toHaveBeenCalledWith(paneKey) + const dispatchArgs = getLastNotificationDispatchArg() + expect(dispatchArgs?.agentState).toBeUndefined() + expect(dispatchArgs?.agentPrompt).toBeUndefined() + }) + + it.each([undefined, 'unknown'] as const)( + 'drops an explicitly named title completion when fresh hook identity is %s', + (agentType) => { + mockState.agentStatusByPaneKey[paneKey] = makeAgentStatus(paneKey, { + state: 'working', + agentType, + updatedAt: Date.now() - 60_000, + stateStartedAt: Date.now() - 60_000, + lastAssistantMessage: undefined + }) + + dispatchTerminalNotification('wt-primary', { + source: 'agent-task-complete', + terminalTitle: 'Claude Code done', + paneKey + }) + + expect(window.api.notifications.dispatch).not.toHaveBeenCalled() + expect(mockState.markWorktreeUnread).not.toHaveBeenCalled() + expect(mockState.markAgentCompletionPaneUnread).not.toHaveBeenCalled() + expect(mockState.markTerminalTabUnread).not.toHaveBeenCalled() + expect(mockState.markTerminalPaneUnread).not.toHaveBeenCalled() + } + ) + + it('drops a Pi title completion while compatible OMP hook status is active', () => { + mockState.agentStatusByPaneKey[paneKey] = makeAgentStatus(paneKey, { + state: 'working', + agentType: 'omp', + updatedAt: Date.now() - 60_000, + stateStartedAt: Date.now() - 60_000, + lastAssistantMessage: undefined + }) + + dispatchTerminalNotification('wt-primary', { + source: 'agent-task-complete', + terminalTitle: 'Pi ready', + paneKey + }) + + expect(window.api.notifications.dispatch).not.toHaveBeenCalled() + expect(mockState.markWorktreeUnread).not.toHaveBeenCalled() + expect(mockState.markAgentCompletionPaneUnread).not.toHaveBeenCalled() + expect(mockState.markTerminalTabUnread).not.toHaveBeenCalled() + expect(mockState.markTerminalPaneUnread).not.toHaveBeenCalled() + }) + + it('allows title-only completion after active hook status becomes stale', () => { + mockState.agentStatusByPaneKey[paneKey] = makeAgentStatus(paneKey, { + state: 'working', + updatedAt: Date.now() - AGENT_STATUS_STALE_AFTER_MS - 1, + stateStartedAt: Date.now() - AGENT_STATUS_STALE_AFTER_MS - 1, + lastAssistantMessage: undefined + }) + + dispatchTerminalNotification('wt-primary', { + source: 'agent-task-complete', + terminalTitle: '/workspace/orca', + paneKey + }) + + expect(window.api.notifications.dispatch).toHaveBeenCalled() + expect(mockState.markWorktreeUnread).toHaveBeenCalledWith('wt-primary') + }) + it('drops a delayed completion snapshot when the pane has already started a newer turn', () => { const previousDoneStartedAt = Date.now() - 10_000 mockState.agentStatusByPaneKey[paneKey] = makeAgentStatus(paneKey, { diff --git a/src/renderer/src/components/terminal-pane/use-notification-dispatch.ts b/src/renderer/src/components/terminal-pane/use-notification-dispatch.ts index 4a39cef7d..20a0ddadf 100644 --- a/src/renderer/src/components/terminal-pane/use-notification-dispatch.ts +++ b/src/renderer/src/components/terminal-pane/use-notification-dispatch.ts @@ -5,8 +5,16 @@ import { getRepoMapFromState, getWorktreeMapFromState } from '@/store/selectors' import { playDesktopNotificationSound } from '@/lib/desktop-notification-sound' import { showBlockedNotificationFallbackToast } from '@/lib/blocked-notification-fallback' import { buildAgentNotificationId } from '../../../../shared/agent-notification-id' +import { resolveCompatibleAgentTypeForOwner } from '../../../../shared/agent-title-owner' +import { + isFreshNonDoneAgentStatus, + type AgentStatusEntry +} from '../../../../shared/agent-status-types' import { isSupersededAgentCompletionSnapshot } from './agent-completion-snapshot-staleness' -import type { AgentCompletionStatusSnapshot } from './agent-completion-coordinator-types' +import type { + AgentCompletionDispatchMeta, + AgentCompletionStatusSnapshot +} from './agent-completion-coordinator-types' import { countReposNeedingNotificationDisambiguation, getPaneKeyTabId, @@ -28,11 +36,28 @@ function agentSnapshotMatchesExplicitTitle( return !snapshot || !explicitTitleAgentType || snapshot.agentType === explicitTitleAgentType } +function hasFreshActiveHookStatus( + snapshot: Pick | undefined, + explicitTitleAgentType: string | null +): boolean { + const activeHookAgentForTitle = resolveCompatibleAgentTypeForOwner( + snapshot?.agentType, + explicitTitleAgentType + ) + const titleNamesDifferentKnownAgent = + explicitTitleAgentType && + snapshot?.agentType && + snapshot.agentType !== 'unknown' && + activeHookAgentForTitle !== explicitTitleAgentType + return Boolean(isFreshNonDoneAgentStatus(snapshot) && !titleNamesDifferentKnownAgent) +} + export type TerminalNotificationEvent = { source: 'terminal-bell' | 'agent-task-complete' terminalTitle?: string paneKey?: string agentStatusSnapshot?: AgentCompletionStatusSnapshot + agentCompletionSource?: AgentCompletionDispatchMeta['source'] suppressOsNotification?: boolean } @@ -70,9 +95,24 @@ export function dispatchTerminalNotification( agentSnapshotMatchesExplicitTitle(storedAgentStatus, explicitTitleAgentType) ? storedAgentStatus : undefined + if ( + event.source === 'agent-task-complete' && + event.agentCompletionSource !== 'process-exit' && + !eventAgentStatusSnapshot && + hasFreshActiveHookStatus(storedAgentStatus, explicitTitleAgentType) + ) { + // Why: a title-only idle signal can race behind active hook state; a + // confirmed process exit is independent authority that the turn ended. + return + } + // Why: a process can die before its hook emits done; do not label the + // resulting completion notification with that stale active state or prompt. const agentStatus = event.source === 'agent-task-complete' - ? (eventAgentStatusSnapshot ?? freshStoredAgentStatus) + ? (eventAgentStatusSnapshot ?? + (event.agentCompletionSource === 'process-exit' && freshStoredAgentStatus?.state !== 'done' + ? undefined + : freshStoredAgentStatus)) : undefined if ( event.source === 'agent-task-complete' && 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 d39e2aab2..3fa220ffa 100644 --- a/src/renderer/src/hooks/agent-hook-completion-notifications.test.ts +++ b/src/renderer/src/hooks/agent-hook-completion-notifications.test.ts @@ -4,6 +4,7 @@ import { YOLO_TUI_AGENT_ARGS } from '../../../shared/tui-agent-permissions' import { createHookListenerState, normalizeHookPayload } from '../../../shared/agent-hook-listener' const dispatchTerminalNotification = vi.fn() +const dispatchAgentHookTerminalLifecycle = vi.fn() type MockStoreState = { settings: { @@ -66,6 +67,10 @@ vi.mock('@/components/terminal-pane/use-notification-dispatch', () => ({ dispatchTerminalNotification })) +vi.mock('@/components/terminal-pane/agent-hook-terminal-lifecycle', () => ({ + dispatchAgentHookTerminalLifecycle +})) + function hookStatus(state: ParsedAgentStatusPayload['state']): ParsedAgentStatusPayload { return { state, @@ -105,6 +110,7 @@ describe('agent hook completion notifications', () => { vi.resetModules() vi.useFakeTimers() dispatchTerminalNotification.mockClear() + dispatchAgentHookTerminalLifecycle.mockClear() mockStoreState = { settings: { experimentalTerminalAttention: false, @@ -183,6 +189,39 @@ describe('agent hook completion notifications', () => { ) }, 15_000) + it('accepts hook lifecycle while every completion alert consumer is disabled', async () => { + mockStoreState.settings.notifications.agentTaskComplete = false + mockStoreState.settings.experimentalTerminalAttention = false + const { + observeAgentHookCompletionForNotification, + syncAgentHookCompletionNotificationSettings + } = await import('./agent-hook-completion-notifications') + + observeAgentHookCompletionForNotification({ + paneKey, + worktreeId: 'wt-1', + payload: hookStatus('working') + }) + observeAgentHookCompletionForNotification({ + paneKey, + worktreeId: 'wt-1', + payload: hookStatus('done') + }) + vi.advanceTimersByTime(HOOK_DONE_QUIET_MS) + + expect(dispatchAgentHookTerminalLifecycle).toHaveBeenCalledWith( + paneKey, + expect.objectContaining({ state: 'done', agentType: 'codex' }) + ) + expect(dispatchTerminalNotification).not.toHaveBeenCalled() + + mockStoreState.settings.notifications.agentTaskComplete = true + syncAgentHookCompletionNotificationSettings() + vi.advanceTimersByTime(HOOK_DONE_QUIET_MS) + + expect(dispatchTerminalNotification).not.toHaveBeenCalled() + }) + it('tracks hook completion for terminal attention when OS completion notifications are disabled', async () => { mockStoreState.settings.experimentalTerminalAttention = true mockStoreState.settings.notifications.agentTaskComplete = false @@ -695,6 +734,11 @@ describe('agent hook completion notifications', () => { }) vi.advanceTimersByTime(HOOK_DONE_QUIET_MS - 1) expect(dispatchTerminalNotification).not.toHaveBeenCalled() + expect( + dispatchAgentHookTerminalLifecycle.mock.calls.filter( + ([, payload]) => payload.state === 'done' + ) + ).toHaveLength(0) observeAgentHookCompletionForNotification({ paneKey, @@ -703,6 +747,11 @@ describe('agent hook completion notifications', () => { }) vi.advanceTimersByTime(HOOK_DONE_QUIET_MS) expect(dispatchTerminalNotification).not.toHaveBeenCalled() + expect( + dispatchAgentHookTerminalLifecycle.mock.calls.filter( + ([, payload]) => payload.state === 'done' + ) + ).toHaveLength(0) observeAgentHookCompletionForNotification({ paneKey, @@ -712,6 +761,10 @@ describe('agent hook completion notifications', () => { vi.advanceTimersByTime(HOOK_DONE_QUIET_MS) expect(dispatchTerminalNotification).toHaveBeenCalledTimes(1) + expect(dispatchAgentHookTerminalLifecycle).toHaveBeenCalledWith( + paneKey, + expect.objectContaining({ state: 'done', agentType: 'codex' }) + ) expect(dispatchTerminalNotification).toHaveBeenCalledWith( 'wt-1', expect.objectContaining({ @@ -771,7 +824,7 @@ describe('agent hook completion notifications', () => { expect(_getAgentHookCompletionNotificationCoordinatorCountForTest()).toBe(3) }) - it('reads tabsByWorktree once per prune pass regardless of coordinator count', async () => { + it('skips tab scans until a pane-liveness slice changes', async () => { seedManyLivePanes() const { observeAgentHookCompletionForNotification, @@ -786,21 +839,27 @@ describe('agent hook completion notifications', () => { }) } - // Count tabsByWorktree reads during a single prune pass. Pre-fix this was - // O(coordinators) because each pane re-flattened tabsByWorktree; the index - // makes it exactly one read for the whole pass. + // Count full tab-map enumerations rather than cheap reference reads. const realTabs = mockStoreState.tabsByWorktree - let tabsReadCount = 0 - Object.defineProperty(mockStoreState, 'tabsByWorktree', { - configurable: true, - get() { - tabsReadCount += 1 - return realTabs + let tabEnumerationCount = 0 + mockStoreState.tabsByWorktree = new Proxy(realTabs, { + ownKeys(target) { + tabEnumerationCount += 1 + return Reflect.ownKeys(target) } }) syncAgentHookCompletionNotificationSettings() - expect(tabsReadCount).toBe(1) + expect(tabEnumerationCount).toBe(1) + + syncAgentHookCompletionNotificationSettings() + + expect(tabEnumerationCount).toBe(1) + + mockStoreState.ptyIdsByTabId = { ...mockStoreState.ptyIdsByTabId } + syncAgentHookCompletionNotificationSettings() + + expect(tabEnumerationCount).toBe(2) }) }) diff --git a/src/renderer/src/hooks/agent-hook-completion-notifications.ts b/src/renderer/src/hooks/agent-hook-completion-notifications.ts index 00575397e..0154d5d0d 100644 --- a/src/renderer/src/hooks/agent-hook-completion-notifications.ts +++ b/src/renderer/src/hooks/agent-hook-completion-notifications.ts @@ -9,6 +9,7 @@ import type { RuntimeTerminalProcessInspection } from '@/runtime/runtime-termina import { dispatchTerminalNotification } from '@/components/terminal-pane/use-notification-dispatch' import { collectLeafIdsInOrder } from '@/components/terminal-pane/layout-serialization' import { createCodexAutoApprovalHookCompletionSuppressor } from '@/components/terminal-pane/codex-auto-approval-notification-suppression' +import { dispatchAgentHookTerminalLifecycle } from '@/components/terminal-pane/agent-hook-terminal-lifecycle' type CoordinatorEntry = { worktreeId: string @@ -18,14 +19,18 @@ type CoordinatorEntry = { type StoreSnapshot = ReturnType type WorktreeTab = NonNullable[string][number] // Why: a paneKey resolves to a tab by id. Prebuilding this index once per prune -// pass avoids re-flattening tabsByWorktree per coordinator (O(coordinators x -// tabs)) — the prune runs on every store notify, including every OSC title frame. +// pass avoids re-flattening tabsByWorktree per coordinator (O(coordinators x tabs)). type TabIndex = ReadonlyMap +type PaneCoordinatorLivenessSnapshot = Pick< + StoreSnapshot, + 'tabsByWorktree' | 'ptyIdsByTabId' | 'terminalLayoutsByTabId' | 'suppressedPtyExitIds' +> const coordinatorsByPaneKey = new Map() const paneKeysRequiringFreshWorking = new Set() let wasAgentTaskCompleteTrackingEnabled = isAgentTaskCompleteTrackingEnabled() let requireFreshWorkingForNewTrackingCoordinators = !wasAgentTaskCompleteTrackingEnabled +let lastPrunedLivenessSnapshot: PaneCoordinatorLivenessSnapshot | null = null function disposeCoordinatorForPaneKey(paneKey: string): void { coordinatorsByPaneKey.get(paneKey)?.coordinator.dispose() @@ -33,9 +38,9 @@ function disposeCoordinatorForPaneKey(paneKey: string): void { paneKeysRequiringFreshWorking.delete(paneKey) } -function buildTabIndex(state: StoreSnapshot): TabIndex { +function buildTabIndex(tabsByWorktree: StoreSnapshot['tabsByWorktree']): TabIndex { const index = new Map() - for (const tabs of Object.values(state.tabsByWorktree ?? {})) { + for (const tabs of Object.values(tabsByWorktree ?? {})) { for (const tab of tabs) { // Why: first-wins to match the previous Array.flat().find() semantics // exactly, even in the degenerate case of a tab id shared across worktrees. @@ -51,11 +56,28 @@ function pruneClosedPaneCoordinators(): void { // Why: hook-completion coordinators are module-scoped and may outlive a pane // unless liveness changes from close/sleep paths evict them here. if (coordinatorsByPaneKey.size === 0 && paneKeysRequiringFreshWorking.size === 0) { + lastPrunedLivenessSnapshot = null return } + const state = useAppStore.getState() + const livenessSnapshot: PaneCoordinatorLivenessSnapshot = { + tabsByWorktree: state.tabsByWorktree, + ptyIdsByTabId: state.ptyIdsByTabId, + terminalLayoutsByTabId: state.terminalLayoutsByTabId, + suppressedPtyExitIds: state.suppressedPtyExitIds + } + if ( + lastPrunedLivenessSnapshot?.tabsByWorktree === livenessSnapshot.tabsByWorktree && + lastPrunedLivenessSnapshot.ptyIdsByTabId === livenessSnapshot.ptyIdsByTabId && + lastPrunedLivenessSnapshot.terminalLayoutsByTabId === livenessSnapshot.terminalLayoutsByTabId && + lastPrunedLivenessSnapshot.suppressedPtyExitIds === livenessSnapshot.suppressedPtyExitIds + ) { + return + } + lastPrunedLivenessSnapshot = livenessSnapshot // Why: build the paneKey->tab index once for the whole pass instead of // re-flattening tabsByWorktree inside paneCanReceiveHookCompletion per entry. - const tabIndex = buildTabIndex(useAppStore.getState()) + const tabIndex = buildTabIndex(livenessSnapshot.tabsByWorktree) for (const paneKey of coordinatorsByPaneKey.keys()) { if (!paneCanReceiveHookCompletion(paneKey, tabIndex)) { disposeCoordinatorForPaneKey(paneKey) @@ -66,6 +88,9 @@ function pruneClosedPaneCoordinators(): void { paneKeysRequiringFreshWorking.delete(paneKey) } } + if (coordinatorsByPaneKey.size === 0 && paneKeysRequiringFreshWorking.size === 0) { + lastPrunedLivenessSnapshot = null + } } function isAgentTaskCompleteNotificationEnabled(): boolean { @@ -84,11 +109,10 @@ function isAgentTaskCompleteTrackingEnabled(): boolean { export function syncAgentHookCompletionNotificationSettings(): boolean { pruneClosedPaneCoordinators() const enabled = isAgentTaskCompleteTrackingEnabled() - if (!enabled || (!wasAgentTaskCompleteTrackingEnabled && enabled)) { + if (enabled !== wasAgentTaskCompleteTrackingEnabled) { requireFreshWorkingForNewTrackingCoordinators = true - for (const [paneKey, entry] of coordinatorsByPaneKey) { + for (const paneKey of coordinatorsByPaneKey.keys()) { paneKeysRequiringFreshWorking.add(paneKey) - entry.coordinator.resetCompletionState({ requireFreshWorking: true }) } } wasAgentTaskCompleteTrackingEnabled = enabled @@ -195,7 +219,11 @@ function createCoordinator(paneKey: string, worktreeId: string): AgentCompletion foregroundProcess: null, hasChildProcesses: false }), + dispatchHookLifecycle: (payload) => dispatchAgentHookTerminalLifecycle(paneKey, payload), dispatchCompletion: (title, meta) => { + if (!isAgentTaskCompleteTrackingEnabled() || paneKeysRequiringFreshWorking.has(paneKey)) { + return + } dispatchTerminalNotification(worktreeId, { source: 'agent-task-complete', terminalTitle: title, @@ -205,6 +233,9 @@ function createCoordinator(paneKey: string, worktreeId: string): AgentCompletion }) }, dispatchAttention: (title, meta) => { + if (!isAgentTaskCompleteTrackingEnabled() || paneKeysRequiringFreshWorking.has(paneKey)) { + return + } // Why: native notification settings still label this channel as "agent // task complete"; the snapshot state makes the banner read "needs input". dispatchTerminalNotification(worktreeId, { @@ -234,13 +265,7 @@ export function observeAgentHookCompletionForNotification({ return } - if (!syncAgentHookCompletionNotificationSettings()) { - paneKeysRequiringFreshWorking.add(paneKey) - coordinatorsByPaneKey - .get(paneKey) - ?.coordinator.resetCompletionState({ requireFreshWorking: true }) - return - } + const trackingEnabled = syncAgentHookCompletionNotificationSettings() let entry = coordinatorsByPaneKey.get(paneKey) if (!entry || entry.worktreeId !== worktreeId) { @@ -254,14 +279,12 @@ export function observeAgentHookCompletionForNotification({ paneKeysRequiringFreshWorking.add(paneKey) } } - if (paneKeysRequiringFreshWorking.has(paneKey)) { - entry.coordinator.resetCompletionState({ requireFreshWorking: true }) - } - - entry.coordinator.observeHookStatus(payload) - if (payload.state === 'working') { + // Why: notification preferences may suppress alerts, but accepted hooks must + // still release pane-owned cursor/cache effects after the quiet window. + if (payload.state === 'working' && trackingEnabled) { paneKeysRequiringFreshWorking.delete(paneKey) } + entry.coordinator.observeHookStatus(payload) } export function resetAgentHookCompletionNotificationCoordinators(): void { @@ -270,6 +293,7 @@ export function resetAgentHookCompletionNotificationCoordinators(): void { } coordinatorsByPaneKey.clear() paneKeysRequiringFreshWorking.clear() + lastPrunedLivenessSnapshot = null wasAgentTaskCompleteTrackingEnabled = isAgentTaskCompleteTrackingEnabled() requireFreshWorkingForNewTrackingCoordinators = !wasAgentTaskCompleteTrackingEnabled } diff --git a/src/renderer/src/hooks/useIpcEvents.test.ts b/src/renderer/src/hooks/useIpcEvents.test.ts index bef556028..7abf34a6d 100644 --- a/src/renderer/src/hooks/useIpcEvents.test.ts +++ b/src/renderer/src/hooks/useIpcEvents.test.ts @@ -5064,6 +5064,78 @@ describe('useIpcEvents agent status snapshot integration', () => { expect(observeAgentHookCompletionForNotification).toHaveBeenCalledTimes(1) }) + it('does not send an out-of-order hook event to completion lifecycle tracking', async () => { + const setAgentStatus = vi.fn() + const updateTabTitle = vi.fn() + const observeAgentHookCompletionForNotification = vi.fn() + const onSetListenerRef: { current: ((data: AgentStatusSetData) => void) | null } = { + current: null + } + const storeState: StoreLike = buildStoreState({ + setAgentStatus, + updateTabTitle, + workspaceSessionReady: true, + settings: { terminalFontSize: 13, notifications: { enabled: true, agentTaskComplete: true } }, + tabsByWorktree: { + 'wt-1': [{ id: 'tab-future', ptyId: 'pty-1', worktreeId: 'wt-1', title: 'Claude' }] + }, + agentStatusByPaneKey: { + [FUTURE_PANE_KEY]: { + state: 'working', + prompt: 'newer turn', + agentType: 'claude', + updatedAt: 1_700_000_000_500, + stateStartedAt: 1_700_000_000_400 + } + } + }) + + stubReactSyncEffect() + vi.doMock('../store', () => ({ + useAppStore: { + subscribe: vi.fn(() => () => {}), + getState: () => storeState + } + })) + vi.doMock('./agent-hook-completion-notifications', () => ({ + observeAgentHookCompletionForNotification, + resetAgentHookCompletionNotificationCoordinators: vi.fn(), + syncAgentHookCompletionNotificationSettings: vi.fn() + })) + stubAuxiliaryModules() + vi.stubGlobal( + 'window', + buildWindowApi({ + onSet: (cb) => { + onSetListenerRef.current = cb + return () => {} + } + }) + ) + + const { useIpcEvents } = await import('./useIpcEvents') + useIpcEvents() + await Promise.resolve() + + if (typeof onSetListenerRef.current !== 'function') { + throw new Error('Expected agentStatus.onSet listener to be registered') + } + onSetListenerRef.current({ + paneKey: FUTURE_PANE_KEY, + tabId: 'tab-future', + worktreeId: 'wt-1', + state: 'done', + prompt: 'older turn', + agentType: 'claude', + receivedAt: 1_700_000_000_300, + stateStartedAt: 1_700_000_000_200 + }) + + expect(setAgentStatus).not.toHaveBeenCalled() + expect(updateTabTitle).not.toHaveBeenCalled() + expect(observeAgentHookCompletionForNotification).not.toHaveBeenCalled() + }) + it('keeps auto-approved Codex done statuses on the completion path', async () => { const setAgentStatus = vi.fn() const observeAgentHookCompletionForNotification = vi.fn() diff --git a/src/renderer/src/hooks/useIpcEvents.ts b/src/renderer/src/hooks/useIpcEvents.ts index 49867868e..219f8dca7 100644 --- a/src/renderer/src/hooks/useIpcEvents.ts +++ b/src/renderer/src/hooks/useIpcEvents.ts @@ -3027,6 +3027,11 @@ export function useIpcEvents(): void { ? { ...resolvedPayload, orchestration: data.orchestration } : resolvedPayload const existingStatus = store.agentStatusByPaneKey[data.paneKey] + if (existingStatus && data.receivedAt < existingStatus.updatedAt) { + // Why: the store rejects out-of-order status rows; keep notification and + // terminal lifecycle effects on the same accepted event boundary. + return 'dropped' + } const identity = resolveAgentStatusIdentity({ existing: existingStatus ? { diff --git a/src/shared/agent-status-identity.ts b/src/shared/agent-status-identity.ts index a796e89ce..dc4f3cf55 100644 --- a/src/shared/agent-status-identity.ts +++ b/src/shared/agent-status-identity.ts @@ -1,5 +1,6 @@ import { AGENT_STATUS_STALE_AFTER_MS, + isFreshNonDoneAgentStatus, type AgentStatusState, type AgentType } from './agent-status-types' @@ -36,7 +37,7 @@ function isActiveExistingIdentity( now: number, staleAfterMs: number ): boolean { - return existing.state !== 'done' && now - existing.updatedAt <= staleAfterMs + return isFreshNonDoneAgentStatus(existing, now, staleAfterMs) } export function resolveAgentStatusIdentity(args: { diff --git a/src/shared/agent-status-types.ts b/src/shared/agent-status-types.ts index dc1c64d59..a2987649e 100644 --- a/src/shared/agent-status-types.ts +++ b/src/shared/agent-status-types.ts @@ -223,6 +223,15 @@ export const AGENT_STATUS_INTERACTIVE_PROMPT_MAX_LENGTH = 16000 * dashboard + hover only display hook-reported data as-is. */ export const AGENT_STATUS_STALE_AFTER_MS = 30 * 60 * 1000 + +export function isFreshNonDoneAgentStatus( + entry: Pick | undefined, + now = Date.now(), + staleAfterMs = AGENT_STATUS_STALE_AFTER_MS +): boolean { + return Boolean(entry && entry.state !== 'done' && now - entry.updatedAt <= staleAfterMs) +} + const SINGLE_LINE_FIELD_SCAN_OVERHEAD = 64 const SINGLE_LINE_FIELD_SCAN_MULTIPLIER = 8