diff --git a/src/main/agent-hooks/server.claude-interactive-question.test.ts b/src/main/agent-hooks/server.claude-interactive-question.test.ts index a2c31a8f1..cc450a3c3 100644 --- a/src/main/agent-hooks/server.claude-interactive-question.test.ts +++ b/src/main/agent-hooks/server.claude-interactive-question.test.ts @@ -57,6 +57,34 @@ describe('Claude interactive-question status transitions', () => { ]) }) + it('clears a PermissionRequest-shaped AskUserQuestion wait when later tool work starts', () => { + // Why: newer Claude reports the AskUserQuestion wait as PermissionRequest; + // the question must not inherit real-permission stickiness from that shape. + const server = new AgentHookServer() + + ingestClaudeStatus(server, { + state: 'waiting', + hookEventName: 'PermissionRequest', + toolName: 'AskUserQuestion', + toolUseId: 'tool-question' + }) + ingestClaudeStatus(server, { + state: 'working', + hookEventName: 'PreToolUse', + toolName: 'Read', + toolUseId: 'tool-after-answer' + }) + + expect(server.getStatusSnapshot()).toEqual([ + expect.objectContaining({ + paneKey: PANE_KEY, + state: 'working', + agentType: 'claude', + toolName: 'Read' + }) + ]) + }) + it('keeps an actual permission request sticky during unrelated tool work', () => { const server = new AgentHookServer() @@ -83,3 +111,102 @@ describe('Claude interactive-question status transitions', () => { ]) }) }) + +function answeredRequestFromSnapshot( + server: AgentHookServer +): Parameters[0] { + const [entry] = server.getStatusSnapshot() + return { + paneKey: entry.paneKey, + baselineUpdatedAt: entry.receivedAt, + baselineStateStartedAt: entry.stateStartedAt, + // Why: mirror the renderer, which echoes the entry's prompt verbatim — + // these ingests carry none, and the server must strict-match that. + baselinePrompt: entry.prompt as string, + baselineAgentType: entry.agentType + } +} + +describe('inferQuestionAnswered', () => { + it('clears an AskUserQuestion wait when the submit keystroke is reported', () => { + const server = new AgentHookServer() + ingestClaudeStatus(server, { + state: 'waiting', + hookEventName: 'PreToolUse', + toolName: 'AskUserQuestion', + toolUseId: 'tool-question' + }) + + expect(server.inferQuestionAnswered(answeredRequestFromSnapshot(server))).toBe(true) + // Why: the answered question must also drop the tool identity so the + // question card cannot linger on the working row. + const [entry] = server.getStatusSnapshot() + expect(entry).toMatchObject({ paneKey: PANE_KEY, state: 'working', agentType: 'claude' }) + expect(entry.toolName).toBeUndefined() + expect(entry.interactivePrompt).toBeUndefined() + }) + + it('clears a PermissionRequest-shaped AskUserQuestion wait (newer Claude)', () => { + const server = new AgentHookServer() + ingestClaudeStatus(server, { + state: 'waiting', + hookEventName: 'PermissionRequest', + toolName: 'AskUserQuestion', + toolUseId: 'tool-question' + }) + + expect(server.inferQuestionAnswered(answeredRequestFromSnapshot(server))).toBe(true) + expect(server.getStatusSnapshot()).toEqual([ + expect.objectContaining({ paneKey: PANE_KEY, state: 'working', agentType: 'claude' }) + ]) + }) + + it('refuses when the cached status changed since the baseline was captured', () => { + const server = new AgentHookServer() + ingestClaudeStatus(server, { + state: 'waiting', + hookEventName: 'PreToolUse', + toolName: 'AskUserQuestion', + toolUseId: 'tool-question' + }) + const staleRequest = { + ...answeredRequestFromSnapshot(server), + baselineUpdatedAt: 1 + } + + expect(server.inferQuestionAnswered(staleRequest)).toBe(false) + expect(server.getStatusSnapshot()).toEqual([ + expect.objectContaining({ state: 'waiting', toolName: 'AskUserQuestion' }) + ]) + }) + + it('never clears a real permission request', () => { + const server = new AgentHookServer() + ingestClaudeStatus(server, { + state: 'waiting', + hookEventName: 'PermissionRequest', + toolName: 'Bash', + toolUseId: 'tool-needs-permission' + }) + + expect(server.inferQuestionAnswered(answeredRequestFromSnapshot(server))).toBe(false) + expect(server.getStatusSnapshot()).toEqual([ + expect.objectContaining({ state: 'waiting', toolName: 'Bash' }) + ]) + }) + + it('ignores panes that are not waiting on a question', () => { + const server = new AgentHookServer() + ingestClaudeStatus(server, { + state: 'working', + hookEventName: 'PreToolUse', + toolName: 'Read', + toolUseId: 'tool-working' + }) + + expect(server.inferQuestionAnswered(answeredRequestFromSnapshot(server))).toBe(false) + expect(server.getStatusSnapshot()).toEqual([ + expect.objectContaining({ state: 'working', toolName: 'Read' }) + ]) + }) +}) diff --git a/src/main/agent-hooks/server.ts b/src/main/agent-hooks/server.ts index 4011fdabd..3480378d3 100644 --- a/src/main/agent-hooks/server.ts +++ b/src/main/agent-hooks/server.ts @@ -22,6 +22,7 @@ import { ORCA_HOOK_PROTOCOL_VERSION } from '../../shared/agent-hook-types' import { clearAllListenerCaches, clearPaneCacheState, + clearClaudeAnsweredQuestionWait, createHookListenerState, getEndpointFileName, hasPendingAgentResultText, @@ -57,6 +58,10 @@ import { isAgentInterruptInputIntent, type AgentInterruptInferenceRequest } from '../../shared/agent-interrupt-intent' +import { + isAskUserQuestionTool, + type AgentQuestionAnsweredInferenceRequest +} from '../../shared/agent-question-answered-intent' import { parseLegacyNumericPaneKey, parsePaneKey } from '../../shared/stable-pane-id' import type { LegacyPaneKeyAliasEntry } from '../../shared/types' import { normalizeAgentProviderSession } from '../../shared/agent-session-resume' @@ -353,7 +358,12 @@ function shouldKeepClaudePermissionVisible( return false } // Why: only real permission requests stay sticky across concurrent subagent - // activity; interactive questions clear on the next working hook. + // activity; interactive questions clear on the next working hook. Newer + // Claude reports the AskUserQuestion wait AS a PermissionRequest, so the + // tool name — not the event name — decides which rule applies. + if (isAskUserQuestionTool(previous.payload.toolName)) { + return false + } return true } @@ -626,6 +636,69 @@ export class AgentHookServer { return true } + /** Guarded fallback for a hook Claude never sends: answering AskUserQuestion + * produces no event, so the amber wait would otherwise linger until the + * agent's next tool or turn end. The renderer reports the submit keystroke; + * this re-validates its baseline against the cached status (a racing real + * hook wins) and synthesizes the post-answer state. */ + inferQuestionAnswered(request: AgentQuestionAnsweredInferenceRequest): boolean { + if (!isValidPaneKey(request.paneKey)) { + return false + } + const existing = this.state.lastStatusByPaneKey.get(request.paneKey) as + | EnrichedAgentHookEventPayload + | undefined + if (!existing) { + return false + } + const payload = existing.payload + // Why: only Claude's interactive question may clear on typed input. The + // tool name is the discriminator, not the hook event — Claude versions + // differ on whether the AskUserQuestion wait arrives as PreToolUse or + // PermissionRequest. Real permission waits (other tools) stay sticky until + // the approved tool resumes — a denied or ignored permission must keep + // demanding attention even though approving is also a keystroke. + if ( + payload.agentType !== 'claude' || + payload.state !== 'waiting' || + !isAskUserQuestionTool(payload.toolName) + ) { + return false + } + if ( + payload.agentType !== request.baselineAgentType || + payload.prompt !== request.baselinePrompt || + existing.receivedAt !== request.baselineUpdatedAt || + existing.stateStartedAt !== request.baselineStateStartedAt || + Date.now() - existing.receivedAt > AGENT_STATUS_STALE_AFTER_MS + ) { + return false + } + // Why: sync the listener's lead-turn record too — a later child lifecycle + // event would otherwise re-emit the stale waiting state and resurrect the + // dismissed question card. + const restored = clearClaudeAnsweredQuestionWait(this.state, existing.paneKey) + const inferred = this.applyNormalizedStatus({ + paneKey: existing.paneKey, + tabId: existing.tabId, + worktreeId: existing.worktreeId, + connectionId: existing.connectionId, + providerSession: existing.providerSession, + payload: { + state: restored.state, + prompt: payload.prompt, + agentType: payload.agentType, + ...(restored.state === 'done' && restored.interrupted ? { interrupted: true } : {}), + ...(payload.subagents ? { subagents: payload.subagents } : {}) + } + }) + console.debug('[agent-hooks] inferred answered question status', { + paneKey: inferred.paneKey, + state: inferred.payload.state + }) + return true + } + getStatusChangeSnapshot(): AgentHookStatusChangeEntry[] { return Array.from(this.state.lastStatusByPaneKey.entries(), ([paneKey, entry]) => { const enriched = entry as EnrichedAgentHookEventPayload diff --git a/src/main/ipc/agent-hooks.ts b/src/main/ipc/agent-hooks.ts index 76b98ccb8..5a8e3f69a 100644 --- a/src/main/ipc/agent-hooks.ts +++ b/src/main/ipc/agent-hooks.ts @@ -5,6 +5,7 @@ import type { MigrationUnsupportedPtyEntry } from '../../shared/agent-status-types' import type { AgentInterruptInferenceRequest } from '../../shared/agent-interrupt-intent' +import type { AgentQuestionAnsweredInferenceRequest } from '../../shared/agent-question-answered-intent' import { agentHookServer, isValidPaneKey } from '../agent-hooks/server' import { ampHookService } from '../amp/hook-service' import { @@ -67,6 +68,7 @@ export function registerAgentHookHandlers( ipcMain.removeHandler('agentHooks:kimiStatus') ipcMain.removeHandler('agentStatus:getSnapshot') ipcMain.removeHandler('agentStatus:inferInterrupt') + ipcMain.removeHandler('agentStatus:inferQuestionAnswered') ipcMain.removeHandler('agentStatus:getMigrationUnsupportedSnapshot') // Why: agentStatus:drop is sent fire-and-forget from the renderer via // ipcRenderer.send(); we listen with ipcMain.on (not handle) so we don't @@ -121,6 +123,12 @@ export function registerAgentHookHandlers( } return agentHookServer.inferInterrupt(request as AgentInterruptInferenceRequest) }) + ipcMain.handle('agentStatus:inferQuestionAnswered', (_event, request: unknown): boolean => { + if (typeof request !== 'object' || request === null) { + return false + } + return agentHookServer.inferQuestionAnswered(request as AgentQuestionAnsweredInferenceRequest) + }) ipcMain.handle( 'agentStatus:getMigrationUnsupportedSnapshot', (): MigrationUnsupportedPtyEntry[] => getMigrationUnsupportedPtySnapshot() diff --git a/src/preload/api-types.ts b/src/preload/api-types.ts index 7c03b6f88..0b2cfd982 100644 --- a/src/preload/api-types.ts +++ b/src/preload/api-types.ts @@ -304,6 +304,7 @@ import type { MigrationUnsupportedPtyEntry } from '../shared/agent-status-types' import type { AgentInterruptInferenceRequest } from '../shared/agent-interrupt-intent' +import type { AgentQuestionAnsweredInferenceRequest } from '../shared/agent-question-answered-intent' import type { TerminalSideEffectBatch } from '../shared/terminal-side-effect-facts' import type { RuntimeBrowserDriverState, @@ -3181,6 +3182,9 @@ export type PreloadApi = { /** Return the current main-process hook cache after renderer hydration. */ getSnapshot: () => Promise inferInterrupt: (request: AgentInterruptInferenceRequest) => Promise + /** Guarded clear for an answered AskUserQuestion wait — the CLI emits no + * hook at answer time, so the renderer reports the submit keystroke. */ + inferQuestionAnswered: (request: AgentQuestionAnsweredInferenceRequest) => Promise /** Listen for PTYs that still use a legacy numeric pane key but have * registry-backed UUID pane proof. */ onMigrationUnsupported: (callback: (entry: MigrationUnsupportedPtyEntry) => void) => () => void diff --git a/src/preload/index.ts b/src/preload/index.ts index 9f0a30529..8084b0b1f 100644 --- a/src/preload/index.ts +++ b/src/preload/index.ts @@ -145,6 +145,7 @@ import type { MigrationUnsupportedPtyEntry } from '../shared/agent-status-types' import type { AgentInterruptInferenceRequest } from '../shared/agent-interrupt-intent' +import type { AgentQuestionAnsweredInferenceRequest } from '../shared/agent-question-answered-intent' import type { TerminalSideEffectBatch } from '../shared/terminal-side-effect-facts' import type { SpeechErrorEvent, @@ -4404,6 +4405,8 @@ const api = { ipcRenderer.invoke('agentStatus:getSnapshot'), inferInterrupt: (request: AgentInterruptInferenceRequest): Promise => ipcRenderer.invoke('agentStatus:inferInterrupt', request), + inferQuestionAnswered: (request: AgentQuestionAnsweredInferenceRequest): Promise => + ipcRenderer.invoke('agentStatus:inferQuestionAnswered', request), onMigrationUnsupported: ( callback: (entry: MigrationUnsupportedPtyEntry) => void ): (() => void) => { diff --git a/src/renderer/src/components/native-chat/NativeChatInteractiveCard.test.tsx b/src/renderer/src/components/native-chat/NativeChatInteractiveCard.test.tsx index 37f5e21db..26aea01d0 100644 --- a/src/renderer/src/components/native-chat/NativeChatInteractiveCard.test.tsx +++ b/src/renderer/src/components/native-chat/NativeChatInteractiveCard.test.tsx @@ -2,7 +2,7 @@ import '@testing-library/jest-dom/vitest' -import { cleanup, fireEvent, render, screen } from '@testing-library/react' +import { act, cleanup, fireEvent, render, screen } from '@testing-library/react' import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' import type { NativeChatInteractiveSend } from './use-native-chat-interactive-send' @@ -73,7 +73,7 @@ describe('NativeChatInteractiveCard answer lifecycle', () => { }) it('keeps the card retryable when no PTY answer was sent', () => { - mocks.sendAnswer.mockReturnValue(0) + mocks.sendAnswer.mockReturnValue({ settleAfterMs: 0, waitsForVerifiedDelivery: false }) renderCard() chooseSpacesAndSubmit() @@ -84,7 +84,7 @@ describe('NativeChatInteractiveCard answer lifecycle', () => { }) it('cancels delayed PTY writes when the owning card unmounts', () => { - mocks.sendAnswer.mockReturnValue(5_000) + mocks.sendAnswer.mockReturnValue({ settleAfterMs: 5_000, waitsForVerifiedDelivery: false }) const rendered = renderCard() chooseSpacesAndSubmit() @@ -95,7 +95,7 @@ describe('NativeChatInteractiveCard answer lifecycle', () => { }) it('cancels delayed PTY writes when desktop send authority is lost', () => { - mocks.sendAnswer.mockReturnValue(5_000) + mocks.sendAnswer.mockReturnValue({ settleAfterMs: 5_000, waitsForVerifiedDelivery: false }) const rendered = renderCard() chooseSpacesAndSubmit() @@ -105,7 +105,7 @@ describe('NativeChatInteractiveCard answer lifecycle', () => { }) it('shows the paced send as busy and freezes the snapshotted answer', () => { - mocks.sendAnswer.mockReturnValue(5_000) + mocks.sendAnswer.mockReturnValue({ settleAfterMs: 5_000, waitsForVerifiedDelivery: false }) renderCard() chooseSpacesAndSubmit() @@ -117,7 +117,7 @@ describe('NativeChatInteractiveCard answer lifecycle', () => { }) it('cancels the old answer sequence when a replacement prompt arrives', () => { - mocks.sendAnswer.mockReturnValue(5_000) + mocks.sendAnswer.mockReturnValue({ settleAfterMs: 5_000, waitsForVerifiedDelivery: false }) const rendered = renderCard() chooseSpacesAndSubmit() @@ -135,4 +135,34 @@ describe('NativeChatInteractiveCard answer lifecycle', () => { expect(mocks.cancelPending).toHaveBeenCalledOnce() expect(screen.getByText('Choose a shell?')).toBeInTheDocument() }) + + it('keeps a verified send visible until delivery succeeds', () => { + let settleDelivery: ((delivered: boolean) => void) | undefined + mocks.sendAnswer.mockImplementation((_prompt, _selections, onDeliverySettled) => { + settleDelivery = onDeliverySettled + return { settleAfterMs: 500, waitsForVerifiedDelivery: true } + }) + renderCard() + + chooseSpacesAndSubmit() + expect(screen.getByRole('button', { name: 'Sending…' })).toBeDisabled() + + act(() => settleDelivery?.(true)) + expect(screen.queryByText('Tabs or spaces?')).not.toBeInTheDocument() + }) + + it('restores a verified send for retry when delivery is rejected', () => { + let settleDelivery: ((delivered: boolean) => void) | undefined + mocks.sendAnswer.mockImplementation((_prompt, _selections, onDeliverySettled) => { + settleDelivery = onDeliverySettled + return { settleAfterMs: 500, waitsForVerifiedDelivery: true } + }) + renderCard() + + chooseSpacesAndSubmit() + act(() => settleDelivery?.(false)) + + expect(screen.getByRole('button', { name: 'Send answer' })).toBeEnabled() + expect(screen.getByText('Tabs or spaces?')).toBeInTheDocument() + }) }) diff --git a/src/renderer/src/components/native-chat/NativeChatInteractiveCard.tsx b/src/renderer/src/components/native-chat/NativeChatInteractiveCard.tsx index de2d1aa1a..3c8973e6d 100644 --- a/src/renderer/src/components/native-chat/NativeChatInteractiveCard.tsx +++ b/src/renderer/src/components/native-chat/NativeChatInteractiveCard.tsx @@ -107,23 +107,42 @@ export function NativeChatInteractiveCard({ return } submittingRef.current = true - const settleMs = sendAnswer(card.prompt, selections) - if (settleMs <= 0) { - // Keep the actionable card visible when its PTY disappeared between - // render and submit; the next live target update can make it retryable. - submittingRef.current = false - return - } - setSubmitting(true) - // Hold the card until the paced write finishes, then mark it answered - // (which hides it and restores the composer). - dismissTimerRef.current = setTimeout(() => { - cancelPending() + const dismissAnsweredCard = (): void => { setDismissedKey(cardKey) submittingRef.current = false setSubmitting(false) dismissTimerRef.current = null - }, settleMs) + } + const keepRejectedAnswerVisible = (): void => { + submittingRef.current = false + setSubmitting(false) + } + const result = sendAnswer(card.prompt, selections, (delivered) => { + if (delivered) { + dismissAnsweredCard() + } else { + keepRejectedAnswerVisible() + } + }) + if (result.settleAfterMs <= 0) { + // Keep the actionable card visible when its PTY disappeared between + // render and submit; the next live target update can make it retryable. + keepRejectedAnswerVisible() + return + } + setSubmitting(true) + if (result.waitsForVerifiedDelivery) { + // Why: remote acceptance can outlive the keystroke pacing window. + // Keep the card until delivery is proven instead of cancelling the + // inference callback at the old fixed dismissal deadline. + return + } + // Hold the card until the paced write finishes, then mark it answered + // (which hides it and restores the composer). + dismissTimerRef.current = setTimeout(() => { + cancelPending() + dismissAnsweredCard() + }, result.settleAfterMs) }} onCancel={() => { clearDismissTimer() diff --git a/src/renderer/src/components/native-chat/NativeChatView.tsx b/src/renderer/src/components/native-chat/NativeChatView.tsx index d2267281e..984a49191 100644 --- a/src/renderer/src/components/native-chat/NativeChatView.tsx +++ b/src/renderer/src/components/native-chat/NativeChatView.tsx @@ -166,7 +166,7 @@ function NativeChatResolvedView({ const canSend = useNativeChatCanSend(targetPtyId) // Reuse the verified composer send path for interactive cards and composer // stop (Stop sends ESC, the agent-TUI interrupt key). - const interactiveSend = useNativeChatInteractiveSend(terminalTabId, targetPtyId, agent) + const interactiveSend = useNativeChatInteractiveSend(terminalTabId, paneKey, targetPtyId, agent) const [workingInterrupted, setWorkingInterrupted] = useState(false) // True while a question card owns the input region, so the composer is hidden. const [questionActive, setQuestionActive] = useState(false) diff --git a/src/renderer/src/components/native-chat/native-chat-runtime-send.test.ts b/src/renderer/src/components/native-chat/native-chat-runtime-send.test.ts index 730ad84f1..4982bbda2 100644 --- a/src/renderer/src/components/native-chat/native-chat-runtime-send.test.ts +++ b/src/renderer/src/components/native-chat/native-chat-runtime-send.test.ts @@ -33,6 +33,7 @@ describe('sendNativeChatMessage', () => { beforeEach(() => { vi.useFakeTimers() sendRuntimePtyInput.mockClear() + sendRuntimePtyInput.mockReturnValue(true) }) afterEach(() => { vi.useRealTimers() @@ -205,6 +206,8 @@ describe('sendNativeChatAskAnswer', () => { beforeEach(() => { vi.useFakeTimers() sendRuntimePtyInput.mockClear() + sendRuntimePtyInput.mockReturnValue(true) + sendRuntimePtyInputVerified.mockReset().mockResolvedValue(true) }) afterEach(() => { vi.useRealTimers() @@ -278,4 +281,39 @@ describe('sendNativeChatAskAnswer', () => { // Only the first keystroke landed; the rest were cancelled. expect(sendRuntimePtyInput).toHaveBeenCalledTimes(1) }) + + it('reports verified delivery only after settling and suppresses it after cancellation', async () => { + const onSettled = vi.fn() + sendRuntimePtyInputVerified.mockResolvedValueOnce(true).mockResolvedValueOnce(false) + const handle = sendNativeChatAskAnswer(SETTINGS, PTY, [{ raw: '1' }, { raw: '\r' }], onSettled) + + await vi.advanceTimersByTimeAsync(handle.settleAfterMs) + expect(onSettled).toHaveBeenCalledExactlyOnceWith(false) + expect(sendRuntimePtyInput).not.toHaveBeenCalled() + + const canceledSettled = vi.fn() + const canceled = sendNativeChatAskAnswer(SETTINGS, PTY, [{ raw: '1' }], canceledSettled) + canceled.cancel() + await vi.runAllTimersAsync() + expect(canceledSettled).not.toHaveBeenCalled() + }) + + it('waits for remote acceptance before reporting delivery', async () => { + const onSettled = vi.fn() + let resolveAccepted!: (accepted: boolean) => void + sendRuntimePtyInputVerified.mockReturnValueOnce( + new Promise((resolve) => { + resolveAccepted = resolve + }) + ) + + const handle = sendNativeChatAskAnswer(SETTINGS, PTY, [{ raw: '2' }], onSettled) + await vi.advanceTimersByTimeAsync(handle.settleAfterMs) + + expect(sendRuntimePtyInputVerified).toHaveBeenCalledWith(SETTINGS, PTY, '2') + expect(onSettled).not.toHaveBeenCalled() + + resolveAccepted(true) + await vi.waitFor(() => expect(onSettled).toHaveBeenCalledExactlyOnceWith(true)) + }) }) diff --git a/src/renderer/src/components/native-chat/native-chat-runtime-send.ts b/src/renderer/src/components/native-chat/native-chat-runtime-send.ts index a1dca86b6..f067064e0 100644 --- a/src/renderer/src/components/native-chat/native-chat-runtime-send.ts +++ b/src/renderer/src/components/native-chat/native-chat-runtime-send.ts @@ -155,27 +155,54 @@ export function submitNativeChatPrompt( export function sendNativeChatAskAnswer( settings: ReturnType, ptyId: string, - groups: AskAnswerKeyGroup[] + groups: AskAnswerKeyGroup[], + onSettled?: (delivered: boolean) => void ): NativeChatSendHandle { if (groups.length === 0) { return { cancel: () => {}, settleAfterMs: 0 } } const timers: ReturnType[] = [] + const verifiedWrites: Promise[] = [] + let cancelled = false groups.forEach((group, index) => { timers.push( setTimeout(() => { const bytes = 'raw' in group ? group.raw : buildNativeChatPasteBytes(group.text) - sendRuntimePtyInput(settings, ptyId, bytes) + if (onSettled) { + // Why: inference must use the remote host's acceptance result, not + // the fire-and-forget renderer dispatch result. + verifiedWrites.push( + sendRuntimePtyInputVerified(settings, ptyId, bytes).catch(() => false) + ) + } else { + sendRuntimePtyInput(settings, ptyId, bytes) + } }, index * NATIVE_CHAT_QUESTION_STEP_MS) ) }) + const settleAfterMs = + (groups.length - 1) * NATIVE_CHAT_QUESTION_STEP_MS + NATIVE_CHAT_SUBMIT_DELAY_MS + if (onSettled) { + // Why: status inference must wait for every paced write and must not run + // after cancellation or a rejected runtime write. + timers.push( + setTimeout(() => { + void Promise.all(verifiedWrites).then((results) => { + if (!cancelled) { + onSettled(results.every(Boolean)) + } + }) + }, settleAfterMs) + ) + } return { cancel: () => { + cancelled = true for (const timer of timers) { clearTimeout(timer) } }, // Hold the card until the last keystroke has fired and its submit gap passed. - settleAfterMs: (groups.length - 1) * NATIVE_CHAT_QUESTION_STEP_MS + NATIVE_CHAT_SUBMIT_DELAY_MS + settleAfterMs } } diff --git a/src/renderer/src/components/native-chat/use-native-chat-interactive-send.test.tsx b/src/renderer/src/components/native-chat/use-native-chat-interactive-send.test.tsx index 5a336009c..9b886bbbb 100644 --- a/src/renderer/src/components/native-chat/use-native-chat-interactive-send.test.tsx +++ b/src/renderer/src/components/native-chat/use-native-chat-interactive-send.test.tsx @@ -5,9 +5,30 @@ import { beforeEach, describe, expect, it, vi } from 'vitest' const mocks = vi.hoisted(() => ({ cancel: vi.fn(), + inferQuestionAnswered: vi.fn(() => Promise.resolve(true)), sendRuntimePtyInput: vi.fn(), sendNativeChatAskAnswer: vi.fn(), - sendNativeChatMessage: vi.fn() + sendNativeChatMessage: vi.fn(), + // Mutable so a test can swap the live status between sendAnswer and settle. + storeState: { agentStatusByPaneKey: {} as Record } +})) + +const PANE_KEY = 'tab-1:11111111-1111-4111-8111-111111111111' +const waitingQuestion = { + state: 'waiting' as const, + prompt: 'pick one', + updatedAt: Date.now(), + stateStartedAt: Date.now(), + agentType: 'claude' as const, + paneKey: PANE_KEY, + stateHistory: [], + toolName: 'AskUserQuestion' +} + +vi.mock('../../store', () => ({ + useAppStore: { + getState: () => mocks.storeState + } })) vi.mock('@/runtime/runtime-terminal-inspection', () => ({ @@ -33,13 +54,20 @@ const PROMPT: AskPrompt = { describe('useNativeChatInteractiveSend', () => { beforeEach(() => { vi.clearAllMocks() + mocks.storeState = { agentStatusByPaneKey: { [PANE_KEY]: waitingQuestion } } + Object.defineProperty(window, 'api', { + configurable: true, + value: { agentStatus: { inferQuestionAnswered: mocks.inferQuestionAnswered } } + }) const handle = { cancel: mocks.cancel, settleAfterMs: 500 } mocks.sendNativeChatAskAnswer.mockReturnValue(handle) mocks.sendNativeChatMessage.mockReturnValue(handle) }) it('routes a non-Claude answer through the pasted-text send path', () => { - const { result } = renderHook(() => useNativeChatInteractiveSend('tab-1', 'pty-1', 'codex')) + const { result } = renderHook(() => + useNativeChatInteractiveSend('tab-1', PANE_KEY, 'pty-1', 'codex') + ) act(() => result.current.sendAnswer(PROMPT, [{ indices: [1] }])) @@ -53,7 +81,9 @@ describe('useNativeChatInteractiveSend', () => { }) it('routes a Claude answer through the option-number keystroke path', () => { - const { result } = renderHook(() => useNativeChatInteractiveSend('tab-1', 'pty-1', 'claude')) + const { result } = renderHook(() => + useNativeChatInteractiveSend('tab-1', PANE_KEY, 'pty-1', 'claude') + ) act(() => result.current.sendAnswer(PROMPT, [{ indices: [1] }])) @@ -61,27 +91,43 @@ describe('useNativeChatInteractiveSend', () => { expect(mocks.sendNativeChatAskAnswer).toHaveBeenCalledWith( { terminalTabId: 'tab-1' }, 'pty-1', - [{ raw: '2' }] + [{ raw: '2' }], + expect.any(Function) ) expect(mocks.sendNativeChatMessage).not.toHaveBeenCalled() }) - it('does nothing when no option is answered', () => { - const { result } = renderHook(() => useNativeChatInteractiveSend('tab-1', 'pty-1', 'claude')) + it('infers OpenClaude answers through its Claude-compatible selector path', () => { + const { result } = renderHook(() => + useNativeChatInteractiveSend('tab-1', PANE_KEY, 'pty-1', 'openclaude') + ) - let settleMs = -1 + act(() => result.current.sendAnswer(PROMPT, [{ indices: [1] }])) + + const onSettled = mocks.sendNativeChatAskAnswer.mock.calls[0]?.[3] + expect(onSettled).toBeTypeOf('function') + onSettled?.(true) + expect(mocks.inferQuestionAnswered).toHaveBeenCalledOnce() + }) + + it('does nothing when no option is answered', () => { + const { result } = renderHook(() => + useNativeChatInteractiveSend('tab-1', PANE_KEY, 'pty-1', 'claude') + ) + + let resultValue: ReturnType | undefined act(() => { - settleMs = result.current.sendAnswer(PROMPT, [{ indices: [] }]) + resultValue = result.current.sendAnswer(PROMPT, [{ indices: [] }]) }) - expect(settleMs).toBe(0) + expect(resultValue).toEqual({ settleAfterMs: 0, waitsForVerifiedDelivery: false }) expect(mocks.sendNativeChatAskAnswer).not.toHaveBeenCalled() expect(mocks.sendNativeChatMessage).not.toHaveBeenCalled() }) it('cancels delayed answer writes when the PTY target changes', () => { const { result, rerender } = renderHook( - ({ targetPtyId }) => useNativeChatInteractiveSend('tab-1', targetPtyId, 'codex'), + ({ targetPtyId }) => useNativeChatInteractiveSend('tab-1', PANE_KEY, targetPtyId, 'codex'), { initialProps: { targetPtyId: 'pty-1' as string | null } } ) @@ -91,8 +137,22 @@ describe('useNativeChatInteractiveSend', () => { expect(mocks.cancel).toHaveBeenCalledOnce() }) + it('cancels delayed answer writes when the pane identity changes', () => { + const { result, rerender } = renderHook( + ({ paneKey }) => useNativeChatInteractiveSend('tab-1', paneKey, 'pty-1', 'claude'), + { initialProps: { paneKey: PANE_KEY } } + ) + + act(() => result.current.sendAnswer(PROMPT, [{ indices: [0] }])) + rerender({ paneKey: 'tab-1:22222222-2222-4222-8222-222222222222' }) + + expect(mocks.cancel).toHaveBeenCalledOnce() + }) + it('cancels delayed answer writes before interrupting the active PTY', () => { - const { result } = renderHook(() => useNativeChatInteractiveSend('tab-1', 'pty-1', 'claude')) + const { result } = renderHook(() => + useNativeChatInteractiveSend('tab-1', PANE_KEY, 'pty-1', 'claude') + ) act(() => result.current.sendAnswer(PROMPT, [{ indices: [1] }])) act(() => result.current.cancel()) @@ -106,7 +166,9 @@ describe('useNativeChatInteractiveSend', () => { }) it('can cancel delayed writes without interrupting the replacement prompt', () => { - const { result } = renderHook(() => useNativeChatInteractiveSend('tab-1', 'pty-1', 'claude')) + const { result } = renderHook(() => + useNativeChatInteractiveSend('tab-1', PANE_KEY, 'pty-1', 'claude') + ) act(() => result.current.sendAnswer(PROMPT, [{ indices: [1] }])) act(() => result.current.cancelPending()) @@ -114,4 +176,81 @@ describe('useNativeChatInteractiveSend', () => { expect(mocks.cancel).toHaveBeenCalledOnce() expect(mocks.sendRuntimePtyInput).not.toHaveBeenCalled() }) + + it('infers a Claude question answer only after every runtime write was delivered', () => { + const { result } = renderHook(() => + useNativeChatInteractiveSend('tab-1', PANE_KEY, 'pty-1', 'claude') + ) + + act(() => result.current.sendAnswer(PROMPT, [{ indices: [1] }])) + expect(mocks.inferQuestionAnswered).not.toHaveBeenCalled() + + const onSettled = mocks.sendNativeChatAskAnswer.mock.calls[0]?.[3] + expect(onSettled).toBeTypeOf('function') + onSettled?.(false) + expect(mocks.inferQuestionAnswered).not.toHaveBeenCalled() + onSettled?.(true) + + expect(mocks.inferQuestionAnswered).toHaveBeenCalledExactlyOnceWith({ + paneKey: PANE_KEY, + baselineUpdatedAt: waitingQuestion.updatedAt, + baselineStateStartedAt: waitingQuestion.stateStartedAt, + baselinePrompt: 'pick one', + baselineAgentType: 'claude' + }) + }) + + it('infers the answered question baseline, not a replacement that became current mid-send', () => { + const { result } = renderHook(() => + useNativeChatInteractiveSend('tab-1', PANE_KEY, 'pty-1', 'claude') + ) + + // Answer question A while it is the current waiting question. + act(() => result.current.sendAnswer(PROMPT, [{ indices: [1] }])) + + // A different AskUserQuestion becomes current before the paced send settles. + mocks.storeState = { + agentStatusByPaneKey: { + [PANE_KEY]: { + ...waitingQuestion, + prompt: 'second question', + updatedAt: waitingQuestion.updatedAt + 5000, + stateStartedAt: waitingQuestion.stateStartedAt + 5000 + } + } + } + + const onSettled = mocks.sendNativeChatAskAnswer.mock.calls[0]?.[3] + onSettled?.(true) + + // The baseline is question A's (captured before delivery), so the server can + // reject it against the now-current question B instead of clearing B's wait. + expect(mocks.inferQuestionAnswered).toHaveBeenCalledExactlyOnceWith({ + paneKey: PANE_KEY, + baselineUpdatedAt: waitingQuestion.updatedAt, + baselineStateStartedAt: waitingQuestion.stateStartedAt, + baselinePrompt: 'pick one', + baselineAgentType: 'claude' + }) + }) + + it('reports verified delivery settlement to the question card', () => { + const onDeliverySettled = vi.fn() + const { result } = renderHook(() => + useNativeChatInteractiveSend('tab-1', PANE_KEY, 'pty-1', 'claude') + ) + + let sendResult: ReturnType | undefined + act(() => { + sendResult = result.current.sendAnswer(PROMPT, [{ indices: [1] }], onDeliverySettled) + }) + expect(sendResult).toEqual({ settleAfterMs: 500, waitsForVerifiedDelivery: true }) + + const onSettled = mocks.sendNativeChatAskAnswer.mock.calls[0]?.[3] + onSettled?.(false) + expect(onDeliverySettled).toHaveBeenCalledExactlyOnceWith(false) + + act(() => result.current.cancelPending()) + expect(mocks.cancel).not.toHaveBeenCalled() + }) }) diff --git a/src/renderer/src/components/native-chat/use-native-chat-interactive-send.ts b/src/renderer/src/components/native-chat/use-native-chat-interactive-send.ts index cb49cd9c5..9b2225392 100644 --- a/src/renderer/src/components/native-chat/use-native-chat-interactive-send.ts +++ b/src/renderer/src/components/native-chat/use-native-chat-interactive-send.ts @@ -1,4 +1,5 @@ import { useCallback, useLayoutEffect, useRef } from 'react' +import { useAppStore } from '../../store' import { sendRuntimePtyInput } from '@/runtime/runtime-terminal-inspection' import { getSettingsForAgentTabRuntimeOwner } from '@/lib/agent-paste-draft' import type { AgentType } from '../../../../shared/native-chat-types' @@ -15,16 +16,20 @@ import { sendNativeChatMessage, type NativeChatSendHandle } from './native-chat-runtime-send' +import { inferQuestionAnsweredFromCurrentStatus } from '../terminal-pane/agent-question-answered-inference' // ESC is the agent-TUI interrupt/cancel key over the PTY (matches how the // composer forwards Escape). Used to cancel a question or deny an approval. const ESC = '\x1b' export type NativeChatInteractiveSend = { - /** Deliver the answer to an AskUserQuestion prompt. Returns the ms after which - * every scheduled write has fired (0 if nothing was sent) so the caller can - * keep the card up until the send settles. */ - sendAnswer: (prompt: AskPrompt, selections: AskAnswerSelection[]) => number + /** Deliver the answer to an AskUserQuestion prompt. Claude-format selectors + * verify every runtime write before reporting settlement. */ + sendAnswer: ( + prompt: AskPrompt, + selections: AskAnswerSelection[], + onDeliverySettled?: (delivered: boolean) => void + ) => { settleAfterMs: number; waitsForVerifiedDelivery: boolean } /** Send a raw control string (e.g. an approval option number or ESC) as-is. */ sendRaw: (raw: string) => void /** Stop delayed writes without interrupting the agent. */ @@ -44,6 +49,7 @@ export type NativeChatInteractiveSend = { */ export function useNativeChatInteractiveSend( terminalTabId: string, + paneKey: string, targetPtyId: string | null, agent: AgentType ): NativeChatInteractiveSend { @@ -57,7 +63,10 @@ export function useNativeChatInteractiveSend( }, []) // Why: a split can be rebound without unmounting this view. Cancel during // commit so no delayed answer write can race the replacement PTY. - useLayoutEffect(() => cancelInFlight, [cancelInFlight, targetPtyId, terminalTabId]) + useLayoutEffect( + () => cancelInFlight, + [agent, cancelInFlight, paneKey, targetPtyId, terminalTabId] + ) const sendRaw = useCallback( (raw: string) => { @@ -70,9 +79,13 @@ export function useNativeChatInteractiveSend( ) const sendAnswer = useCallback( - (prompt: AskPrompt, selections: AskAnswerSelection[]): number => { + ( + prompt: AskPrompt, + selections: AskAnswerSelection[], + onDeliverySettled?: (delivered: boolean) => void + ): { settleAfterMs: number; waitsForVerifiedDelivery: boolean } => { if (!targetPtyId || !hasAskAnswer(prompt, selections)) { - return 0 + return { settleAfterMs: 0, waitsForVerifiedDelivery: false } } // Cancel any prior in-flight answer before starting a new one. cancelInFlight() @@ -83,13 +96,57 @@ export function useNativeChatInteractiveSend( // Other agents' question tools commit a pasted answer, so send label text. // Gate on the transcript agent (not `=== 'claude'`) so OpenClaude — which // runs the same selector — takes the keystroke path too. - const handle: NativeChatSendHandle = shouldStepNativeChatAskAnswer(agent) - ? sendNativeChatAskAnswer(settings, targetPtyId, buildAskAnswerKeys(prompt, selections)) + const stepsAnswer = shouldStepNativeChatAskAnswer(agent) + // Why: pin the answered question's baseline BEFORE delivery. A late settle + // callback (paced writes + remote acceptance can span seconds on SSH) must + // not read the live status and mint a fresh baseline for a replacement + // question that became current meanwhile — that would clear the new + // question's wait. The server re-validates this captured baseline and + // rejects a changed status, matching the terminal keystroke path. + const questionStatusBaseline = stepsAnswer + ? useAppStore.getState().agentStatusByPaneKey[paneKey] + : undefined + let settledHandle: NativeChatSendHandle | null = null + const onSettled = stepsAnswer + ? (delivered: boolean): void => { + if (settledHandle && inFlightRef.current === settledHandle) { + // Why: a completed verified send otherwise retains its timers, + // promises, and prompt callback until the next send or unmount. + inFlightRef.current = null + } + if (delivered) { + inferQuestionAnsweredFromCurrentStatus({ + paneKey, + getStatusEntry: () => questionStatusBaseline, + inferQuestionAnswered: (request) => + window.api.agentStatus.inferQuestionAnswered(request).catch((err) => { + console.warn('[agent-question] native-chat inference failed:', err) + return false + }) + }) + } + onDeliverySettled?.(delivered) + } + : undefined + const handle: NativeChatSendHandle = stepsAnswer + ? sendNativeChatAskAnswer( + settings, + targetPtyId, + buildAskAnswerKeys(prompt, selections), + onSettled + ) : sendNativeChatMessage(settings, targetPtyId, formatAskAnswer(prompt, selections)) + // Why: native-chat answer writes bypass xterm.onData. Infer only after + // every paced selector write has fired, so an early digit in a multi-step + // answer cannot dismiss the wait or cancel the remaining writes. + settledHandle = handle inFlightRef.current = handle - return handle.settleAfterMs + return { + settleAfterMs: handle.settleAfterMs, + waitsForVerifiedDelivery: onSettled !== undefined + } }, - [terminalTabId, targetPtyId, agent, cancelInFlight] + [terminalTabId, paneKey, targetPtyId, agent, cancelInFlight] ) // Stop/cancel: drop any pending answer writes, then send ESC to interrupt. diff --git a/src/renderer/src/components/terminal-pane/agent-question-answered-inference.test.ts b/src/renderer/src/components/terminal-pane/agent-question-answered-inference.test.ts new file mode 100644 index 000000000..1460ef5b5 --- /dev/null +++ b/src/renderer/src/components/terminal-pane/agent-question-answered-inference.test.ts @@ -0,0 +1,175 @@ +import { describe, expect, it, vi } from 'vitest' +import type { AgentStatusEntry } from '../../../../shared/agent-status-types' +import { createAgentQuestionAnsweredInference } from './agent-question-answered-inference' + +const PANE_KEY = 'tab-1:11111111-1111-4111-8111-111111111111' +const SINGLE_SELECT_PROMPT = JSON.stringify({ + questions: [ + { + question: 'pick a color', + multiSelect: false, + options: [{ label: 'red' }, { label: 'blue' }] + } + ] +}) + +function makeWaitingQuestionEntry(overrides: Partial = {}): AgentStatusEntry { + return { + state: 'waiting', + prompt: 'pick a color', + updatedAt: 1_000, + stateStartedAt: 900, + agentType: 'claude', + paneKey: PANE_KEY, + stateHistory: [], + toolName: 'AskUserQuestion', + interactivePrompt: SINGLE_SELECT_PROMPT, + ...overrides + } +} + +function makeInference(entry: AgentStatusEntry | undefined) { + const inferQuestionAnswered = vi.fn() + const inference = createAgentQuestionAnsweredInference({ + paneKey: PANE_KEY, + getStatusEntry: () => entry, + inferQuestionAnswered, + now: () => 2_000 + }) + return { inference, inferQuestionAnswered } +} + +describe('agent question-answered inference', () => { + it('reports the baseline when Enter is sent to a waiting question pane', () => { + const { inference, inferQuestionAnswered } = makeInference(makeWaitingQuestionEntry()) + + inference.observeSentTerminalInput('\r') + + expect(inferQuestionAnswered).toHaveBeenCalledExactlyOnceWith({ + paneKey: PANE_KEY, + baselineUpdatedAt: 1_000, + baselineStateStartedAt: 900, + baselinePrompt: 'pick a color', + baselineAgentType: 'claude' + }) + }) + + it('accepts kitty-keyboard Enter encodings', () => { + const { inference, inferQuestionAnswered } = makeInference(makeWaitingQuestionEntry()) + + inference.observeSentTerminalInput('\x1b[13u') + + expect(inferQuestionAnswered).toHaveBeenCalledOnce() + }) + + it('accepts a bare digit quick-select, which submits without Enter', () => { + const { inference, inferQuestionAnswered } = makeInference(makeWaitingQuestionEntry()) + + inference.observeSentTerminalInput('2') + + expect(inferQuestionAnswered).toHaveBeenCalledOnce() + }) + + it('keeps waiting when a digit only advances or toggles a partial answer', () => { + const prompts = [ + JSON.stringify({ + questions: [ + { question: 'first?', multiSelect: false, options: [{ label: 'A' }] }, + { question: 'second?', multiSelect: false, options: [{ label: 'B' }] } + ] + }), + JSON.stringify({ + questions: [ + { + question: 'pick several', + multiSelect: true, + options: [{ label: 'A' }, { label: 'B' }] + } + ] + }) + ] + + for (const interactivePrompt of prompts) { + const { inference, inferQuestionAnswered } = makeInference( + makeWaitingQuestionEntry({ interactivePrompt }) + ) + inference.observeSentTerminalInput('1') + inference.observeSentTerminalInput('\r') + expect(inferQuestionAnswered).not.toHaveBeenCalled() + } + }) + + it('keeps waiting when the synthetic free-text row is selected', () => { + const { inference, inferQuestionAnswered } = makeInference(makeWaitingQuestionEntry()) + + // Two declared options means 3 opens Claude's synthetic "Type something" row. + inference.observeSentTerminalInput('3') + + expect(inferQuestionAnswered).not.toHaveBeenCalled() + }) + + it('keeps waiting when capped or malformed prompt JSON hides the question shape', () => { + const { inference, inferQuestionAnswered } = makeInference( + makeWaitingQuestionEntry({ interactivePrompt: '{"questions":[' }) + ) + + inference.observeSentTerminalInput('\r') + + expect(inferQuestionAnswered).not.toHaveBeenCalled() + }) + + it('keeps the legacy Enter fallback when the hook omitted tool input', () => { + const { inference, inferQuestionAnswered } = makeInference( + makeWaitingQuestionEntry({ interactivePrompt: undefined }) + ) + + inference.observeSentTerminalInput('\r') + + expect(inferQuestionAnswered).toHaveBeenCalledOnce() + }) + + it('does not read status for ordinary terminal input', () => { + const getStatusEntry = vi.fn(() => makeWaitingQuestionEntry()) + const inferQuestionAnswered = vi.fn() + const inference = createAgentQuestionAnsweredInference({ + paneKey: PANE_KEY, + getStatusEntry, + inferQuestionAnswered + }) + + inference.observeSentTerminalInput('ordinary typing') + + expect(getStatusEntry).not.toHaveBeenCalled() + expect(inferQuestionAnswered).not.toHaveBeenCalled() + }) + + it('ignores non-submit input, batched keystrokes, and pastes', () => { + const { inference, inferQuestionAnswered } = makeInference(makeWaitingQuestionEntry()) + + inference.observeSentTerminalInput('\x1b') + inference.observeSentTerminalInput('a') + inference.observeSentTerminalInput('0') + inference.observeSentTerminalInput('12') + inference.observeSentTerminalInput('yes\r') + inference.observeSentTerminalInput('\x1b[200~line one\nline two\x1b[201~') + + expect(inferQuestionAnswered).not.toHaveBeenCalled() + }) + + it('ignores panes without a fresh waiting AskUserQuestion status', () => { + const cases: (AgentStatusEntry | undefined)[] = [ + undefined, + makeWaitingQuestionEntry({ state: 'working' }), + makeWaitingQuestionEntry({ toolName: 'Bash' }), + makeWaitingQuestionEntry({ agentType: 'codex' }), + // Why: a stale wait past the freshness horizon no longer renders amber, + // so a keystroke must not synthesize activity for it. + makeWaitingQuestionEntry({ updatedAt: -100_000_000, stateStartedAt: -100_000_000 }) + ] + for (const entry of cases) { + const { inference, inferQuestionAnswered } = makeInference(entry) + inference.observeSentTerminalInput('\r') + expect(inferQuestionAnswered).not.toHaveBeenCalled() + } + }) +}) diff --git a/src/renderer/src/components/terminal-pane/agent-question-answered-inference.ts b/src/renderer/src/components/terminal-pane/agent-question-answered-inference.ts new file mode 100644 index 000000000..1469648be --- /dev/null +++ b/src/renderer/src/components/terminal-pane/agent-question-answered-inference.ts @@ -0,0 +1,85 @@ +import { + AGENT_STATUS_STALE_AFTER_MS, + type AgentStatusEntry +} from '../../../../shared/agent-status-types' +import { + isAskUserQuestionTool, + isPotentialQuestionAnsweredSubmitInput, + isQuestionAnsweredSubmitInput, + type AgentQuestionAnsweredInferenceRequest +} from '../../../../shared/agent-question-answered-intent' +import { isExplicitAgentStatusFresh } from '@/lib/agent-status' + +export type AgentQuestionAnsweredInference = { + observeSentTerminalInput(data: string): void +} + +type AgentQuestionAnsweredInferenceDeps = { + paneKey: string + getStatusEntry: () => AgentStatusEntry | undefined + inferQuestionAnswered: ( + request: AgentQuestionAnsweredInferenceRequest + ) => boolean | Promise | void + now?: () => number +} + +function inferQuestionAnsweredFromEntry( + deps: AgentQuestionAnsweredInferenceDeps, + entry: AgentStatusEntry | undefined +): boolean { + const now = deps.now ?? Date.now + if ( + !entry || + entry.state !== 'waiting' || + entry.agentType !== 'claude' || + !isAskUserQuestionTool(entry.toolName) || + !isExplicitAgentStatusFresh(entry, now(), AGENT_STATUS_STALE_AFTER_MS) + ) { + return false + } + void deps.inferQuestionAnswered({ + paneKey: deps.paneKey, + baselineUpdatedAt: entry.updatedAt, + baselineStateStartedAt: entry.stateStartedAt, + baselinePrompt: entry.prompt, + baselineAgentType: entry.agentType + }) + return true +} + +/** Completion signal for answer surfaces that write directly to the runtime + * instead of xterm (notably native chat). The same fresh-status baseline is + * used, so a real hook that wins the race prevents the fallback IPC. */ +export function inferQuestionAnsweredFromCurrentStatus( + deps: AgentQuestionAnsweredInferenceDeps +): boolean { + return inferQuestionAnsweredFromEntry(deps, deps.getStatusEntry()) +} + +/** Sibling of the interrupt inference for a hook Claude never sends: answering + * an AskUserQuestion emits no event, so the submit keystroke into the waiting + * pane is the only "question dealt with" signal. Unlike interrupts there is + * no expected real hook to settle for, so the inference fires immediately — + * the main process re-validates the baseline, so a racing hook always wins. */ +export function createAgentQuestionAnsweredInference({ + paneKey, + getStatusEntry, + inferQuestionAnswered, + now = () => Date.now() +}: AgentQuestionAnsweredInferenceDeps): AgentQuestionAnsweredInference { + return { + observeSentTerminalInput(data) { + // Why: ordinary terminal input is the hot path. Reject it with one + // constant-time membership check before reading Zustand or parsing the + // bounded interactive-prompt JSON. + if (!isPotentialQuestionAnsweredSubmitInput(data)) { + return + } + const entry = getStatusEntry() + if (!entry || !isQuestionAnsweredSubmitInput(data, entry.interactivePrompt)) { + return + } + inferQuestionAnsweredFromEntry({ paneKey, getStatusEntry, inferQuestionAnswered, now }, entry) + } + } +} diff --git a/src/renderer/src/components/terminal-pane/pty-connection.ts b/src/renderer/src/components/terminal-pane/pty-connection.ts index 859a71bba..5716e535d 100644 --- a/src/renderer/src/components/terminal-pane/pty-connection.ts +++ b/src/renderer/src/components/terminal-pane/pty-connection.ts @@ -162,6 +162,7 @@ import { isCtrlCKeyEvent, isPlainEscapeKeyEvent } from './agent-interrupt-inference' +import { createAgentQuestionAnsweredInference } from './agent-question-answered-inference' import { AGENT_INTERRUPT_SETTLE_MS, type AgentInterruptInputIntent @@ -1771,6 +1772,15 @@ export function connectPanePty( }) } }) + const questionAnsweredInference = createAgentQuestionAnsweredInference({ + paneKey: cacheKey, + getStatusEntry: () => useAppStore.getState().agentStatusByPaneKey[cacheKey], + inferQuestionAnswered: (request) => + window.api.agentStatus.inferQuestionAnswered(request).catch((err) => { + console.warn('[agent-question] inferQuestionAnswered failed:', err) + return false + }) + }) const dropCommandFinishedStatusIfSameTurn = ( entry: AgentStatusEntry | undefined, options?: { allowInferredInterrupt?: boolean } @@ -1856,6 +1866,10 @@ export function connectPanePty( if (intent === 'ctrl-c' || data === '\x03') { markTerminalBracketedPasteInterrupted(pane.terminal) } + // Why: every delivered-input path funnels through here, so this is where a + // submit keystroke into a waiting AskUserQuestion pane becomes the + // "question answered" signal no hook will ever deliver. + questionAnsweredInference.observeSentTerminalInput(data) } let pendingTerminalInputWrite: Promise | null = null const setPendingTerminalInputWrite = (promise: Promise): void => { diff --git a/src/renderer/src/web/web-preload-api.ts b/src/renderer/src/web/web-preload-api.ts index 29268cee1..cddcfae93 100644 --- a/src/renderer/src/web/web-preload-api.ts +++ b/src/renderer/src/web/web-preload-api.ts @@ -745,6 +745,7 @@ function createWebPreloadApi(): Partial { onClear: () => noopUnsubscribe, getSnapshot: () => Promise.resolve([]), inferInterrupt: () => Promise.resolve(false), + inferQuestionAnswered: () => Promise.resolve(false), onMigrationUnsupported: () => noopUnsubscribe, onMigrationUnsupportedClear: () => noopUnsubscribe, getMigrationUnsupportedSnapshot: () => Promise.resolve([]), diff --git a/src/shared/agent-hook-listener.test.ts b/src/shared/agent-hook-listener.test.ts index e9f9e265f..e5c739ab3 100644 --- a/src/shared/agent-hook-listener.test.ts +++ b/src/shared/agent-hook-listener.test.ts @@ -6,6 +6,7 @@ import { mkdirSync, mkdtempSync, readFileSync, rmSync, statSync, writeFileSync } import { tmpdir } from 'node:os' import { join } from 'node:path' import { + clearClaudeAnsweredQuestionWait, clearPaneCacheState, createHookListenerState, getEndpointFileName, @@ -2781,4 +2782,60 @@ describe('shared agent-hook-listener', () => { expect(ok).toBe(false) }) }) + + describe('clearClaudeAnsweredQuestionWait', () => { + const claudeEvent = ( + payload: Record + ): ReturnType => + normalizeHookPayload(state, 'claude', { paneKey: PANE_KEY, payload }, 'production') + + it('restores working for an answered lead question and drops the card', () => { + claudeEvent({ hook_event_name: 'UserPromptSubmit', prompt: 'pick a color' }) + const wait = claudeEvent({ + hook_event_name: 'PreToolUse', + tool_name: 'AskUserQuestion', + tool_input: { questions: [{ question: 'Red or Blue?' }] } + }) + expect(wait?.payload.state).toBe('waiting') + expect(wait?.payload.interactivePrompt).toBeDefined() + + expect(clearClaudeAnsweredQuestionWait(state, PANE_KEY)).toEqual({ state: 'working' }) + + // Why: a child-driven refresh re-emits the cached lead state; the linger + // bug would come back if it could resurrect the dismissed question. + const childDriven = claudeEvent({ + hook_event_name: 'SubagentStart', + agent_id: 'a1', + agent_type: 'probe' + }) + expect(childDriven?.payload.state).toBe('working') + expect(childDriven?.payload.toolName).toBeUndefined() + expect(childDriven?.payload.interactivePrompt).toBeUndefined() + }) + + it('restores the stashed lead state for an answered child question', () => { + claudeEvent({ hook_event_name: 'UserPromptSubmit', prompt: 'go' }) + claudeEvent({ hook_event_name: 'SubagentStart', agent_id: 'a1', agent_type: 'probe' }) + claudeEvent({ hook_event_name: 'Stop' }) + const wait = claudeEvent({ + hook_event_name: 'PreToolUse', + tool_name: 'AskUserQuestion', + agent_id: 'a1', + tool_input: { questions: [{ question: 'Continue?' }] } + }) + expect(wait?.payload.state).toBe('waiting') + + // Why: the lead already finished; the answer resumes the child, so the + // emitted state is gated up to working only while that child still runs. + expect(clearClaudeAnsweredQuestionWait(state, PANE_KEY)).toEqual({ state: 'working' }) + expect(state.claudeLeadStateByPaneKey.get(PANE_KEY)).toEqual({ state: 'done' }) + + const drained = claudeEvent({ hook_event_name: 'SubagentStop', agent_id: 'a1' }) + expect(drained?.payload.state).toBe('done') + }) + + it('falls back to working when no lead record exists', () => { + expect(clearClaudeAnsweredQuestionWait(state, PANE_KEY)).toEqual({ state: 'working' }) + }) + }) }) diff --git a/src/shared/agent-hook-listener.ts b/src/shared/agent-hook-listener.ts index bde6baf44..f080ce1ce 100644 --- a/src/shared/agent-hook-listener.ts +++ b/src/shared/agent-hook-listener.ts @@ -34,6 +34,7 @@ import { type AgentSubagentSnapshot, type ParsedAgentStatusPayload } from './agent-status-types' +import { isAskUserQuestionTool } from './agent-question-answered-intent' import { claudeRosterHasWorkingSubagent, claudeRosterToSnapshots, @@ -744,14 +745,6 @@ function clearActiveToolFieldsUpdate(): ToolSnapshot { ) } -/** True for the AskUserQuestion tool across the casing variants different - * agents emit (`AskUserQuestion` / `ask_user_question` / `askUserQuestion`). - * Why: this is the structured "pick an option" prompt whose full input the - * clients render as a live card. */ -function isAskUserQuestionTool(toolName: string | undefined): boolean { - return toolName?.replaceAll(/[^a-z0-9]/gi, '').toLowerCase() === 'askuserquestion' -} - /** Capture the full AskUserQuestion tool input as a JSON string when the tool * is an AskUserQuestion variant; otherwise undefined so resolveToolState * clears any prior prompt. Kept agent-generic: callers pass whatever raw @@ -2525,6 +2518,36 @@ function clearClaudePendingWaitForAgent( state.claudeLeadStateByPaneKey.set(paneKey, lead.stateBeforeWait ?? { state: 'working' }) } +/** Clear an AskUserQuestion wait after the user's answer was typed into the + * terminal. Answering emits no hook event, so the caller infers it from the + * submit keystroke. Restores the stashed pre-wait lead state (child-induced + * question) or falls back to 'working' (lead question), and drops the cached + * question card so later child-driven refreshes cannot re-emit the stale + * wait. Returns the pane state to emit, gated up to 'working' while children + * still run. */ +export function clearClaudeAnsweredQuestionWait( + state: HookListenerState, + paneKey: string +): Pick { + const lead = state.claudeLeadStateByPaneKey.get(paneKey) + const restored = + lead?.state === 'waiting' + ? (lead.stateBeforeWait ?? { state: 'working' as const }) + : { state: 'working' as const } + state.claudeLeadStateByPaneKey.set(paneKey, { ...restored }) + const previousTool = state.lastToolByPaneKey.get(paneKey) + state.lastToolByPaneKey.set( + paneKey, + previousTool?.lastAssistantMessage + ? { lastAssistantMessage: previousTool.lastAssistantMessage } + : {} + ) + const roster = state.claudeSubagentRosterByPaneKey.get(paneKey) + return restored.state === 'done' && claudeRosterHasWorkingSubagent(roster) + ? { state: 'working' } + : restored +} + /** Emit a pane status refresh driven by child activity (lifecycle events and * child-origin tool events): the lead's cached state is re-emitted — gated up * to 'working' while a child works — without touching the lead's tool/prompt diff --git a/src/shared/agent-question-answered-intent.ts b/src/shared/agent-question-answered-intent.ts new file mode 100644 index 000000000..de976291a --- /dev/null +++ b/src/shared/agent-question-answered-intent.ts @@ -0,0 +1,83 @@ +import type { AgentType } from './agent-status-types' + +/** Baseline snapshot the renderer captured when it observed the submit + * keystroke. The main process re-validates every field against its own + * cached status so a racing real hook always wins over the inference. */ +export type AgentQuestionAnsweredInferenceRequest = { + paneKey: string + baselineUpdatedAt: number + baselineStateStartedAt: number + baselinePrompt: string + baselineAgentType: AgentType | undefined +} + +/** True for the AskUserQuestion tool across the casing variants different + * agents emit (`AskUserQuestion` / `ask_user_question` / `askUserQuestion`). + * Why: this is the structured "pick an option" prompt whose full input the + * clients render as a live card. */ +export function isAskUserQuestionTool(toolName: string | undefined): boolean { + return toolName?.replaceAll(/[^a-z0-9]/gi, '').toLowerCase() === 'askuserquestion' +} + +const QUESTION_ANSWER_ENTER_INPUTS: ReadonlySet = new Set([ + '\r', + '\n', + '\r\n', + '\x1b[13u', + '\x1b[13;1u' +]) +const QUESTION_ANSWER_DIGIT_INPUTS: ReadonlySet = new Set('123456789') + +export function isPotentialQuestionAnsweredSubmitInput(data: string): boolean { + return QUESTION_ANSWER_ENTER_INPUTS.has(data) || QUESTION_ANSWER_DIGIT_INPUTS.has(data) +} + +function readSingleSelectOptionCount(interactivePrompt: string | undefined): number | null { + if (!interactivePrompt) { + return null + } + try { + const parsed = JSON.parse(interactivePrompt) as { questions?: unknown } + if (!Array.isArray(parsed.questions) || parsed.questions.length !== 1) { + return -1 + } + const [question] = parsed.questions as { multiSelect?: unknown; options?: unknown }[] + if (!question || question.multiSelect === true || !Array.isArray(question.options)) { + return -1 + } + return question.options.length + } catch { + // Why: malformed JSON can be a length-capped multi-question payload. It is + // not equivalent to an older hook omitting tool input, so fail closed. + return -1 + } +} + +/** True only when one keystroke is enough to finish the whole prompt. + * Why: digits merely advance multi-question prompts and toggle multi-selects; + * the synthetic "Type something" row also opens an editor instead of + * submitting. Without the prompt-shape gate those partial choices clear the + * waiting indicator while Claude is still blocked on more input. */ +export function isQuestionAnsweredSubmitInput( + data: string, + interactivePrompt: string | undefined +): boolean { + if (!isPotentialQuestionAnsweredSubmitInput(data)) { + return false + } + const optionCount = readSingleSelectOptionCount(interactivePrompt) + if (optionCount === -1) { + return false + } + if (QUESTION_ANSWER_ENTER_INPUTS.has(data)) { + // Older hook payloads can omit tool input; Enter remains the conservative + // fallback because it is the ordinary submit path for a question. + return true + } + if (optionCount === null) { + return false + } + // Claude adds a final "Type something" row after the declared options. + // Only declared option numbers complete a single-select immediately. + return Number(data) <= optionCount +}