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 7b119905d47..1c9323dd0a9 100644 --- a/src/renderer/src/components/terminal-pane/pty-connection.test.ts +++ b/src/renderer/src/components/terminal-pane/pty-connection.test.ts @@ -17733,7 +17733,26 @@ describe('connectPanePty', () => { const randomSpy = vi.spyOn(Math, 'random').mockReturnValue(0.5) try { const getForegroundProcess = vi.mocked(window.api.pty.getForegroundProcess) - getForegroundProcess.mockResolvedValue('codex') + // Why: call-count alone is flaky — one advanceTimersToNextTimerAsync can start + // multiple getForegroundProcess reads (completion cadence + pane tracker / + // confirmForegroundProcess alias). Gate the 2nd+ null sample until the + // replacement hook owner is installed so the confirming sample always sees it. + let idleMode = false + let nullSamplesStarted = 0 + let releaseConfirmingNullSample: (() => void) | undefined + const confirmingNullSampleGate = new Promise((resolve) => { + releaseConfirmingNullSample = resolve + }) + getForegroundProcess.mockImplementation(async () => { + if (!idleMode) { + return 'codex' + } + nullSamplesStarted += 1 + if (nullSamplesStarted >= 2) { + await confirmingNullSampleGate + } + return null + }) const paneKey = makePaneKey('tab-1', LEAF_1) const pane = createPane(1) const manager = createManager(1) @@ -17753,21 +17772,16 @@ describe('connectPanePty', () => { titleHandler('Codex working', 'Codex working') await vi.advanceTimersByTimeAsync(2_500) - const inspectionsBeforeIdle = getForegroundProcess.mock.calls.length - getForegroundProcess.mockResolvedValue(null) - // Why: active process polling is jittered, so a fixed window can consume - // both null samples before the replacement hook row is installed. - for ( - let attempts = 0; - getForegroundProcess.mock.calls.length === inspectionsBeforeIdle; - attempts += 1 - ) { + idleMode = true + for (let attempts = 0; nullSamplesStarted < 1; attempts += 1) { if (attempts >= 10) { throw new Error('Expected the first idle process inspection') } await vi.advanceTimersToNextTimerAsync() } - expect(getForegroundProcess).toHaveBeenCalledTimes(inspectionsBeforeIdle + 1) + // Why: let the first null sample apply pendingProcessExitAgent before the + // replacement owner is installed; later samples stay gated until then. + await flushAsyncTicks() mockStoreState.agentStatusByPaneKey[paneKey] = { state: 'working', @@ -17778,6 +17792,7 @@ describe('connectPanePty', () => { paneKey, stateHistory: [] } + releaseConfirmingNullSample?.() idleHandler('Claude done') await vi.advanceTimersByTimeAsync(800) await vi.advanceTimersByTimeAsync(AGENT_TASK_COMPLETE_NOTIFICATION_MAX_WAIT_MS) diff --git a/src/shared/agent-hook-listener.test.ts b/src/shared/agent-hook-listener.test.ts index 08c8f00ca15..cc99a807837 100644 --- a/src/shared/agent-hook-listener.test.ts +++ b/src/shared/agent-hook-listener.test.ts @@ -372,6 +372,247 @@ describe('shared agent-hook-listener', () => { expect(event?.payload.interactivePrompt).toBe(JSON.stringify(properties)) }) + it('maps Pi tool_call ask_user_question to blocked with interactivePrompt', () => { + const questions = { + questions: [ + { + question: 'What is your priority?', + options: ['A', 'B', 'C'] + } + ] + } + const blocked = normalizeHookPayload( + state, + 'pi', + { + paneKey: PANE_KEY, + tabId: 'tab-1', + worktreeId: 'wt', + env: 'production', + version: '1', + payload: { + hook_event_name: 'tool_call', + tool_name: 'ask_user_question', + tool_input: questions + } + }, + 'production' + ) + expect(blocked?.payload).toMatchObject({ + state: 'blocked', + agentType: 'pi', + toolName: 'ask_user_question' + }) + expect(blocked?.payload.interactivePrompt).toBe(JSON.stringify(questions)) + }) + + it('maps Pi tool_execution_start ask_user_question to blocked with interactivePrompt', () => { + const questions = { + questions: [ + { + question: 'Pick a path', + options: ['path-1', 'path-2'] + } + ] + } + const blocked = normalizeHookPayload( + state, + 'pi', + { + paneKey: PANE_KEY, + tabId: 'tab-1', + worktreeId: 'wt', + env: 'production', + version: '1', + payload: { + hook_event_name: 'tool_execution_start', + tool_name: 'ask_user_question', + tool_input: questions + } + }, + 'production' + ) + expect(blocked?.payload).toMatchObject({ + state: 'blocked', + agentType: 'pi', + toolName: 'ask_user_question' + }) + expect(blocked?.payload.interactivePrompt).toBe(JSON.stringify(questions)) + }) + + it('keeps Pi regular tool_call notifications as working', () => { + const working = normalizeHookPayload( + state, + 'pi', + { + paneKey: PANE_KEY, + tabId: 'tab-1', + worktreeId: 'wt', + env: 'production', + version: '1', + payload: { + hook_event_name: 'tool_call', + tool_name: 'bash', + tool_input: { command: 'git status' } + } + }, + 'production' + ) + expect(working?.payload).toMatchObject({ + state: 'working', + agentType: 'pi', + toolName: 'bash', + toolInput: 'git status' + }) + expect(working?.payload.interactivePrompt).toBeUndefined() + }) + + it('keeps Pi ask_user_question blocked when tool_input is missing', () => { + const blocked = normalizeHookPayload( + state, + 'pi', + { + paneKey: PANE_KEY, + tabId: 'tab-1', + worktreeId: 'wt', + env: 'production', + version: '1', + payload: { + hook_event_name: 'tool_call', + tool_name: 'ask_user_question' + } + }, + 'production' + ) + expect(blocked?.payload).toMatchObject({ + state: 'blocked', + agentType: 'pi', + toolName: 'ask_user_question' + }) + expect(blocked?.payload.interactivePrompt).toBeUndefined() + }) + + it('clears Pi ask_user_question blocked once the tool_execution_end arrives', () => { + const questions = { + questions: [{ question: 'Ship it?', options: ['yes', 'no'] }] + } + const base = { + paneKey: PANE_KEY, + tabId: 'tab-1', + worktreeId: 'wt', + env: 'production' as const, + version: '1' + } + const blocked = normalizeHookPayload( + state, + 'pi', + { + ...base, + payload: { + hook_event_name: 'tool_call', + tool_name: 'ask_user_question', + tool_input: questions + } + }, + 'production' + ) + expect(blocked?.payload.state).toBe('blocked') + expect(blocked?.payload.interactivePrompt).toBe(JSON.stringify(questions)) + + // Why: the answered question must leave the blocked/needs-attention state so + // the notification and attention sort clear; tool_execution_end is working. + const cleared = normalizeHookPayload( + state, + 'pi', + { + ...base, + payload: { + hook_event_name: 'tool_execution_end', + tool_name: 'ask_user_question' + } + }, + 'production' + ) + expect(cleared?.payload.state).toBe('working') + expect(cleared?.payload.interactivePrompt).toBeUndefined() + }) + + it('marks Pi done when agent_end follows an ask_user_question block', () => { + const base = { + paneKey: PANE_KEY, + tabId: 'tab-1', + worktreeId: 'wt', + env: 'production' as const, + version: '1' + } + normalizeHookPayload( + state, + 'pi', + { + ...base, + payload: { + hook_event_name: 'tool_call', + tool_name: 'ask_user_question', + tool_input: { questions: [{ question: 'Pick', options: ['a', 'b'] }] } + } + }, + 'production' + ) + const done = normalizeHookPayload( + state, + 'pi', + { ...base, payload: { hook_event_name: 'agent_end' } }, + 'production' + ) + expect(done?.payload.state).toBe('done') + expect(done?.payload.interactivePrompt).toBeUndefined() + }) + + it('clears the ask_user_question interactivePrompt when a regular Pi tool runs next', () => { + const base = { + paneKey: PANE_KEY, + tabId: 'tab-1', + worktreeId: 'wt', + env: 'production' as const, + version: '1' + } + normalizeHookPayload( + state, + 'pi', + { + ...base, + payload: { + hook_event_name: 'tool_call', + tool_name: 'ask_user_question', + tool_input: { questions: [{ question: 'Pick', options: ['a', 'b'] }] } + } + }, + 'production' + ) + // Why: a follow-up regular tool must not inherit the prior question's blocked + // state or its live interactivePrompt card. + const working = normalizeHookPayload( + state, + 'pi', + { + ...base, + payload: { + hook_event_name: 'tool_call', + tool_name: 'bash', + tool_input: { command: 'ls' } + } + }, + 'production' + ) + expect(working?.payload).toMatchObject({ + state: 'working', + agentType: 'pi', + toolName: 'bash', + toolInput: 'ls' + }) + expect(working?.payload.interactivePrompt).toBeUndefined() + }) + it('normalizes OMP Pi-compatible hooks with OMP attribution', () => { const event = normalizeHookPayload( state, @@ -419,6 +660,40 @@ describe('shared agent-hook-listener', () => { toolName: 'bash', toolInput: 'pnpm test' }) + expect(tool?.payload.interactivePrompt).toBeUndefined() + }) + + it('keeps OMP ask_user_question behavior on Pi-compatible events', () => { + const tool = normalizeHookPayload( + state, + 'omp', + { + paneKey: PANE_KEY, + tabId: 'tab-1', + worktreeId: 'wt', + env: 'production', + version: '1', + payload: { + hook_event_name: 'tool_call', + tool_name: 'ask_user_question', + tool_input: { + questions: [ + { + question: 'Choose', + options: ['x', 'y'] + } + ] + } + } + }, + 'production' + ) + expect(tool?.payload).toMatchObject({ + state: 'working', + agentType: 'omp', + toolName: 'ask_user_question' + }) + expect(tool?.payload.interactivePrompt).toBeUndefined() }) it('captures Pi session ids on Pi-compatible status events', () => { diff --git a/src/shared/agent-hook-listener.ts b/src/shared/agent-hook-listener.ts index c4df60fb28f..f983d409e79 100644 --- a/src/shared/agent-hook-listener.ts +++ b/src/shared/agent-hook-listener.ts @@ -1898,7 +1898,8 @@ function extractCopilotToolFields( function extractPiToolFields( eventName: unknown, - hookPayload: Record + hookPayload: Record, + agentKind: 'pi' | 'omp' ): ToolSnapshot { if ( eventName === 'tool_call' || @@ -1906,9 +1907,16 @@ function extractPiToolFields( eventName === 'tool_execution_end' ) { const toolName = readString(hookPayload, 'tool_name') - const toolInput = deriveToolInputPreview(toolName, hookPayload.tool_input) + const rawToolInput = hookPayload.tool_input + const toolInput = deriveToolInputPreview(toolName, rawToolInput) + // Why: OMP shares this extractor. interactivePrompt derivation should + // only apply to Pi so OMP ask_user_question metadata stays unchanged. + const interactivePrompt = + agentKind === 'pi' && (eventName === 'tool_call' || eventName === 'tool_execution_start') + ? deriveInteractivePrompt(toolName, rawToolInput, eventName) + : undefined return toolUpdate( - { toolName, toolInput }, + { toolName, toolInput, interactivePrompt }, { hasToolInputField: hasOwnField(hookPayload, 'tool_input') } ) } @@ -2386,7 +2394,7 @@ function extractToolFields( return extractCursorToolFields(eventName, hookPayload) case 'pi': case 'omp': - return extractPiToolFields(eventName, hookPayload) + return extractPiToolFields(eventName, hookPayload, source) case 'droid': return extractDroidToolFields(eventName, hookPayload) case 'command-code': @@ -3422,13 +3430,22 @@ function normalizePiCompatibleEvent( return null } - const stateName = - eventName === 'before_agent_start' || - eventName === 'agent_start' || - eventName === 'tool_call' || - eventName === 'tool_execution_start' || - eventName === 'tool_execution_end' || - eventName === 'message_end' + // Why: gate on the event's own tool_name (matching the Claude/Grok normalizers), + // not a merged snapshot, so a partial follow-up event can't inherit a stale + // ask_user_question name from the tool cache and spuriously re-enter blocked. + const isPiAskUserQuestion = + agentType === 'pi' && + isAskUserQuestionTool(readString(hookPayload, 'tool_name')) && + (eventName === 'tool_call' || eventName === 'tool_execution_start') + + const stateName = isPiAskUserQuestion + ? 'blocked' + : eventName === 'before_agent_start' || + eventName === 'agent_start' || + eventName === 'tool_call' || + eventName === 'tool_execution_start' || + eventName === 'tool_execution_end' || + eventName === 'message_end' ? 'working' : eventName === 'agent_end' ? 'done'