diff --git a/src/renderer/src/components/terminal-pane/codex-lead-stop-subagent-gate.test.ts b/src/renderer/src/components/terminal-pane/codex-lead-stop-subagent-gate.test.ts new file mode 100644 index 00000000000..9863447a72e --- /dev/null +++ b/src/renderer/src/components/terminal-pane/codex-lead-stop-subagent-gate.test.ts @@ -0,0 +1,264 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import { + createAgentCompletionCoordinator, + resetAgentCompletionCoordinatorIdentitiesForTest +} from './agent-completion-coordinator' +import { dispatchTerminalNotification } from './use-notification-dispatch' +import { + createHookListenerState, + normalizeHookPayload, + type HookListenerState +} from '../../../../shared/agent-hook-listener' +import type { AgentHookSource } from '../../../../shared/agent-hook-relay' +import type { AgentStatusEntry } from '../../../../shared/agent-status-types' +import type { AgentCompletionStatusSnapshot } from './agent-completion-coordinator-types' +import type { NotificationDispatchRequest } from '../../../../shared/types' + +const LEAF = '11111111-1111-4111-8111-111111111111' +const PANE_KEY = `tab-1:${LEAF}` +const WORKTREE_ID = 'wt-primary' + +type MockState = Record +let mockState: MockState + +vi.mock('@/store', () => ({ + useAppStore: { getState: () => mockState, subscribe: () => () => {} } +})) +vi.mock('@/lib/desktop-notification-sound', () => ({ playDesktopNotificationSound: vi.fn() })) + +function baseState(): MockState { + return { + activeWorktreeId: 'wt-other', + activeTabId: 'tab-1', + tabsByWorktree: { [WORKTREE_ID]: [{ id: 'tab-1', ptyId: 'pty-1' }] }, + ptyIdsByTabId: { 'tab-1': ['pty-1'] }, + suppressedPtyExitIds: {}, + terminalLayoutsByTabId: { + 'tab-1': { + root: { type: 'leaf', leafId: LEAF }, + activeLeafId: LEAF, + expandedLeafId: null, + ptyIdsByLeafId: { [LEAF]: 'pty-1' } + } + }, + browserTabsByWorktree: {}, + retainedAgentsByPaneKey: {}, + agentStatusByPaneKey: {} as Record, + worktreesByRepo: { + repo1: [{ id: WORKTREE_ID, repoId: 'repo1', displayName: 'main', branch: 'main' }], + repo2: [{ id: 'wt-other', repoId: 'repo2', displayName: 'other', branch: 'other' }] + }, + repos: [ + { id: 'repo1', displayName: 'danmaku_monitor', connectionId: null }, + { id: 'repo2', displayName: 'second_repo', connectionId: null } + ], + settings: { + experimentalTerminalAttention: false, + notifications: { enabled: true, agentTaskComplete: true } + }, + markWorktreeUnread: vi.fn(), + markTerminalTabUnread: vi.fn(), + markTerminalPaneUnread: vi.fn(), + markAgentCompletionPaneUnread: vi.fn() + } +} + +function applyStatusToStore(payload: AgentCompletionStatusSnapshot, now: number): void { + const store = mockState.agentStatusByPaneKey as Record + const previous = store[PANE_KEY] + const stateStartedAt = + previous && previous.state === payload.state ? previous.stateStartedAt : now + store[PANE_KEY] = { + ...previous, + state: payload.state, + prompt: payload.prompt, + agentType: payload.agentType, + toolName: payload.toolName, + toolInput: payload.toolInput, + lastAssistantMessage: payload.lastAssistantMessage, + interrupted: payload.interrupted, + subagents: payload.subagents, + updatedAt: now, + stateStartedAt, + paneKey: PANE_KEY, + terminalTitle: payload.agentType ?? 'agent', + stateHistory: [] + } as AgentStatusEntry +} + +function dispatchCalls(): NotificationDispatchRequest[] { + const d = window.api.notifications.dispatch as unknown as ReturnType + return d.mock.calls.map((c) => c[0] as NotificationDispatchRequest) +} + +function makeCoordinator(): ReturnType { + return createAgentCompletionCoordinator({ + paneKey: PANE_KEY, + getPtyId: () => 'pty-1', + getSettings: () => null, + inspectProcess: async () => ({ foregroundProcess: null, hasChildProcesses: false }), + dispatchCompletion: (title, meta) => { + dispatchTerminalNotification(WORKTREE_ID, { + source: 'agent-task-complete', + terminalTitle: title, + paneKey: PANE_KEY, + ...(meta?.agentStatus ? { agentStatusSnapshot: meta.agentStatus } : {}) + }) + }, + dispatchAttention: (title, meta) => { + dispatchTerminalNotification(WORKTREE_ID, { + source: 'agent-task-complete', + terminalTitle: title, + paneKey: PANE_KEY, + agentStatusSnapshot: meta.agentStatus + }) + }, + isLive: () => true + }) +} + +type Step = { ev: string; p?: Record; waitMs?: number } + +/** Drives real hook normalization -> store mirror -> coordinator -> dispatcher. */ +function drive(source: AgentHookSource, steps: Step[]): NotificationDispatchRequest[] { + const listenerState: HookListenerState = createHookListenerState() + const coordinator = makeCoordinator() + for (const step of steps) { + const event = normalizeHookPayload( + listenerState, + source, + { + paneKey: PANE_KEY, + tabId: 'tab-1', + worktreeId: WORKTREE_ID, + hook_event_name: step.ev, + payload: { hook_event_name: step.ev, ...step.p } + }, + 'test-env' + ) + if (event) { + const now = Date.now() + applyStatusToStore(event.payload, now) + const stored = (mockState.agentStatusByPaneKey as Record)[PANE_KEY] + coordinator.observeHookStatus({ ...event.payload, stateStartedAt: stored.stateStartedAt }) + } + vi.advanceTimersByTime(step.waitMs ?? 100) + } + vi.advanceTimersByTime(6_000) + coordinator.dispose() + return dispatchCalls() +} + +const BASH_1 = { tool_name: 'Bash', tool_input: { command: 'npm run test:submit' } } +const BASH_2 = { + tool_name: 'Bash', + tool_input: { command: 'node scripts/check-hook-registry.js' } +} + +describe('issue #4375 — non-turn-end notification spam', () => { + beforeEach(() => { + vi.useFakeTimers() + vi.setSystemTime(new Date('2026-07-25T12:00:00Z')) + vi.stubGlobal('document', { visibilityState: 'hidden', hasFocus: () => false }) + vi.stubGlobal('navigator', { userAgent: 'Mozilla/5.0 (Windows NT 10.0; Win64; x64)' }) + vi.stubGlobal('window', { + api: { notifications: { dispatch: vi.fn().mockResolvedValue({ delivered: true }) } } + }) + mockState = baseState() + // Why: completion identities are module-scoped and keyed by paneKey, so they leak between tests. + resetAgentCompletionCoordinatorIdentitiesForTest() + }) + afterEach(() => { + vi.useRealTimers() + vi.unstubAllGlobals() + }) + + it('control: a real turn end notifies once', () => { + const calls = drive('codex', [ + { ev: 'UserPromptSubmit', p: { prompt: 'run tests' } }, + { ev: 'PreToolUse', p: BASH_1 }, + { ev: 'PostToolUse', p: BASH_1 }, + { ev: 'Stop' } + ]) + expect(calls).toHaveLength(1) + expect(calls[0].agentState).toBe('done') + }) + + it('REPRO: a Codex lead Stop notifies while a subagent is still working', () => { + const calls = drive('codex', [ + { ev: 'UserPromptSubmit', p: { prompt: 'run tests' } }, + { ev: 'SubagentStart', p: { agent_id: 'child-1', agent_type: 'reviewer' } }, + { ev: 'PreToolUse', p: BASH_1 }, + // Lead finishes; the child keeps working well past the 1.5s quiet window. + { ev: 'Stop', waitMs: 5_000 }, + { ev: 'PreToolUse', p: { ...BASH_2, agent_id: 'child-1' } } + ]) + + if (calls.length > 0) { + throw new Error( + `Spurious completion while subagent child-1 is still working: ` + + `state=${calls[0].agentState} tool=${calls[0].agentToolInput}` + ) + } + expect(calls).toHaveLength(0) + }) + + it('cancels a mid-turn Stop when work resumes inside the quiet window', () => { + const calls = drive('codex', [ + { ev: 'UserPromptSubmit', p: { prompt: 'run tests' } }, + { ev: 'PreToolUse', p: BASH_1 }, + // Resumes at 1s — inside HOOK_DONE_QUIET_MS, so the provisional done is cancelled. + { ev: 'Stop', waitMs: 1_000 }, + { ev: 'PreToolUse', p: BASH_2 } + ]) + expect(calls).toHaveLength(0) + }) + + it('a new session is not gated by the roster the previous Codex process left behind', () => { + const listenerState: HookListenerState = createHookListenerState() + const send = ( + ev: string, + p?: Record + ): ReturnType => + normalizeHookPayload( + listenerState, + 'codex', + { + paneKey: PANE_KEY, + tabId: 'tab-1', + worktreeId: WORKTREE_ID, + hook_event_name: ev, + payload: { hook_event_name: ev, ...p } + }, + 'test-env' + ) + + // A child parks in 'waiting', then the Codex process dies without a child Stop hook. + send('UserPromptSubmit', { prompt: 'first' }) + send('PermissionRequest', { agent_id: 'child-1', agent_type: 'reviewer' }) + + // Why: SessionStart retires the stale roster, so it must not also be gated by it. + const restarted = send('SessionStart') + expect(restarted?.payload.state).toBe('working') + }) + + // KNOWN-OPEN residual, deliberately out of scope for the roster fix. A bare mid-turn + // Stop (no subagent) that resumes AFTER the 1.5s quiet window still notifies. There is + // no principled bound to widen the window to, and widening it delays every real Codex + // completion; the correct fix is retracting the banner via notifications:dismiss on the + // resuming 'working' hook. Pinned here so that change flips this test. + it('KNOWN-OPEN: a bare mid-turn Stop resuming after the quiet window still notifies', () => { + const calls = drive('codex', [ + { ev: 'UserPromptSubmit', p: { prompt: 'run tests' } }, + { ev: 'PreToolUse', p: BASH_1 }, + { ev: 'Stop', waitMs: 2_000 }, + { ev: 'PreToolUse', p: BASH_2 } + ]) + + const finalRow = (mockState.agentStatusByPaneKey as Record)[PANE_KEY] + expect(finalRow.state).toBe('working') + expect(calls).toHaveLength(1) + // The banner carries the *previous* tool, matching the report in #4375. + expect(calls[0].agentToolInput).toBe('npm run test:submit') + }) +}) diff --git a/src/shared/agent-hook-listener.ts b/src/shared/agent-hook-listener.ts index 3f043a0a245..36f60d6308d 100644 --- a/src/shared/agent-hook-listener.ts +++ b/src/shared/agent-hook-listener.ts @@ -3283,6 +3283,12 @@ function normalizeCodexEvent( return buildCodexChildDrivenStatusPayload(state, eventName, paneKey, hookPayload) } + // Why: a lead Stop isn't "done" while children still run, so the roster must gate it before + // being retired. SessionStart is exempt — its roster belongs to the process that just exited. + const effectiveState = + eventName === 'SessionStart' + ? stateName + : codexRosterEffectiveState(state.codexSubagentRosterByPaneKey.get(paneKey), stateName) if (eventName === 'SessionStart') { // Why: a pane can host a new Codex process after the old one exited without child Stop hooks. state.codexSubagentRosterByPaneKey.delete(paneKey) @@ -3297,10 +3303,6 @@ function normalizeCodexEvent( normalizeOptionalField(hookPayload['model'], AGENT_MODEL_MAX_LENGTH) ?? (eventName === 'SessionStart' ? undefined : previousLead?.model) }) - const effectiveState = codexRosterEffectiveState( - state.codexSubagentRosterByPaneKey.get(paneKey), - stateName - ) return buildCodexStatusPayload(state, eventName, promptText, paneKey, hookPayload, { stateName: effectiveState, updateLead: true