From 3881fbcda9dc7f2e42d9f8a58b343dfd83cc620e Mon Sep 17 00:00:00 2001 From: Brennan Benson <79079362+brennanb2025@users.noreply.github.com> Date: Fri, 30 Oct 2026 10:46:10 -0700 Subject: [PATCH] Keep pending questions through agent notices and undelivered replies (#36306) * Keep pending blocking questions through non-answer rows * Use a canonical delivered human row in blocking question tests * Remove redundant live-status question test --- ...mobile-native-chat-blocking-notice.test.ts | 79 +++++++++++++ .../NativeChatBlockingNotice.test.tsx | 106 +++++++++++++++++ .../native-chat/input/native-chat-ask.test.ts | 111 +++++++++++++++++- .../native-chat/input/native-chat-ask.ts | 14 +-- 4 files changed, 299 insertions(+), 11 deletions(-) create mode 100644 mobile/src/session/native-chat/input/use-mobile-native-chat-blocking-notice.test.ts create mode 100644 src/renderer/src/components/native-chat/NativeChatBlockingNotice.test.tsx diff --git a/mobile/src/session/native-chat/input/use-mobile-native-chat-blocking-notice.test.ts b/mobile/src/session/native-chat/input/use-mobile-native-chat-blocking-notice.test.ts new file mode 100644 index 00000000000..02d67d5b141 --- /dev/null +++ b/mobile/src/session/native-chat/input/use-mobile-native-chat-blocking-notice.test.ts @@ -0,0 +1,79 @@ +import { createElement } from 'react' +import TestRenderer from 'react-test-renderer' +import { expect, it } from 'vitest' +import type { NativeChatMessage } from '../../../../../src/shared/native-chat-types' +import { projectStructuredItemToNativeChat } from '../../../../../src/shared/structured-agent-session-projection' +import { useMobileNativeChatPrompts } from './use-mobile-native-chat-prompts' + +it('retains a terminal blocking ask across a projected notice until its result', () => { + // Mixed-source hook contract, not a captured shipping delivery: Claude-shaped call plus projected journal row. + const asked: NativeChatMessage = { + id: 'ask', + role: 'assistant', + timestamp: 1, + source: 'transcript', + blocks: [ + { + type: 'tool-call', + name: 'AskUserQuestion', + input: { questions: [{ question: 'Which path?', options: ['fast', 'safe'] }] } + } + ] + } + const notice = projectStructuredItemToNativeChat({ + itemId: 'notice', + sequence: 2, + revision: 1, + observedAt: 2, + body: { + kind: 'message', + role: 'user', + blocks: [{ type: 'text', text: 'Agent update' }], + from: { kind: 'agent', senders: [], orchestration: null } + } + })! + let prompts: ReturnType | undefined + function Probe({ messages }: { messages: NativeChatMessage[] }): null { + prompts = useMobileNativeChatPrompts({ + enabled: true, + status: null, + messages, + transcriptLoading: false, + asyncSourceScope: 'host/session', + hiddenAsyncIds: new Set() + }) + return null + } + let view!: TestRenderer.ReactTestRenderer + try { + TestRenderer.act(() => { + view = TestRenderer.create(createElement(Probe, { messages: [asked] })) + }) + const pending = prompts?.ask + expect(pending?.questions[0]?.question).toBe('Which path?') + TestRenderer.act(() => { + view.update(createElement(Probe, { messages: [asked, notice] })) + }) + expect(prompts?.ask).toEqual(pending) + TestRenderer.act(() => { + view.update( + createElement(Probe, { + messages: [ + asked, + notice, + { + id: 'result', + role: 'tool', + timestamp: 3, + source: 'transcript', + blocks: [{ type: 'tool-result', output: 'fast' }] + } + ] + }) + ) + }) + expect(prompts?.ask).toBeNull() + } finally { + TestRenderer.act(() => view?.unmount()) + } +}) diff --git a/src/renderer/src/components/native-chat/NativeChatBlockingNotice.test.tsx b/src/renderer/src/components/native-chat/NativeChatBlockingNotice.test.tsx new file mode 100644 index 00000000000..a300ec4418f --- /dev/null +++ b/src/renderer/src/components/native-chat/NativeChatBlockingNotice.test.tsx @@ -0,0 +1,106 @@ +// @vitest-environment happy-dom +import '@testing-library/jest-dom/vitest' +import { cleanup, fireEvent, render, screen } from '@testing-library/react' +import { afterEach, expect, it, vi } from 'vitest' +import type { NativeChatMessage } from '../../../../shared/native-chat-types' +import { projectStructuredItemToNativeChat } from '../../../../shared/structured-agent-session-projection' +import { TooltipProvider } from '@/components/ui/tooltip' +import { NativeChatInteractiveCard } from './NativeChatInteractiveCard' +import { useNativeChatTerminalPromptCard } from './use-native-chat-interactive-prompt-card' +import type { NativeChatInteractiveSend } from './use-native-chat-interactive-send' + +vi.mock('../../store', () => ({ + useAppStore: (selector: (state: { agentStatusByPaneKey: Record }) => unknown) => + selector({ agentStatusByPaneKey: {} }) +})) +const send: NativeChatInteractiveSend = { + sendAnswer: vi.fn(), + sendRaw: vi.fn(), + sendRawVerified: vi.fn(), + cancelPending: vi.fn(), + cancel: vi.fn(), + cancelAsk: vi.fn() +} +function Card({ messages }: { messages: NativeChatMessage[] }) { + const { presentation, actionKey } = useNativeChatTerminalPromptCard({ + paneKey: 'blocking-notice:leaf', + targetPtyId: 'pty', + runtimeEnvironmentId: null, + agent: 'claude', + sessionId: 'session', + transcriptPath: null, + messages, + transcriptSettled: true, + canSend: true + }) + return ( + + {presentation.card && ( + + )} + + ) +} +afterEach(cleanup) + +it('retains selected and typed blocking answers across a projected notice until its result', () => { + // Mixed-source contract fixture: Claude-shaped transcript plus journal projection; no live delivery claim. + const asked: NativeChatMessage = { + id: 'ask', + role: 'assistant', + timestamp: 1, + source: 'transcript', + blocks: [ + { + type: 'tool-call', + name: 'AskUserQuestion', + input: { + questions: [ + { question: 'Tabs or spaces?', options: [{ label: 'Tabs' }, { label: 'Spaces' }] } + ] + } + } + ] + } + const notice = projectStructuredItemToNativeChat({ + itemId: 'notice', + sequence: 2, + revision: 1, + observedAt: 2, + body: { + kind: 'message', + role: 'user', + blocks: [{ type: 'text', text: 'Agent update' }], + from: { kind: 'agent', senders: [], orchestration: null } + } + })! + const view = render() + fireEvent.change(screen.getByRole('textbox'), { target: { value: 'Four spaces' } }) + fireEvent.click(screen.getByRole('button', { name: /Spaces/ })) + view.rerender() + expect(screen.getByText('Tabs or spaces?')).toBeInTheDocument() + expect(screen.getByRole('button', { name: /Spaces/ })).toHaveAttribute('aria-pressed', 'true') + expect(screen.getByRole('textbox')).toHaveValue('Four spaces') + view.rerender( + + ) + expect(screen.queryByText('Tabs or spaces?')).not.toBeInTheDocument() + expect(send.sendAnswer).not.toHaveBeenCalled() +}) diff --git a/src/shared/native-chat/input/native-chat-ask.test.ts b/src/shared/native-chat/input/native-chat-ask.test.ts index 4611ac6fd00..d086ca74163 100644 --- a/src/shared/native-chat/input/native-chat-ask.test.ts +++ b/src/shared/native-chat/input/native-chat-ask.test.ts @@ -9,6 +9,7 @@ import { type AgentMessageSource } from '../../agent-session/agent-session-message-source' import { nativeChatAskDismissKey } from '../../native-chat-ask-dismiss-key' +import { projectStructuredItemToNativeChat } from '../../structured-agent-session-projection' import { extractPendingAsk, extractPendingMessageReplyAsk, @@ -57,6 +58,114 @@ const QUESTIONS_INPUT = { questions: [{ question: 'Deploy?', options: [{ label: 'Yes' }, { label: 'No' }] }] } +describe('blocking abandonment boundary', () => { + // Mixed-source scanner contract: Claude-shaped calls/results plus projected journal notices, not a live capture. + const notice = projectStructuredItemToNativeChat({ + itemId: 'notice', + sequence: 2, + revision: 1, + observedAt: 2, + body: { + kind: 'message', + role: 'user', + blocks: [{ type: 'text', text: 'You have 1 orchestration message.' }], + from: { + kind: 'agent', + senders: [], + orchestration: { + message: 'mail-notice', + mailbox: 'term_b', + dispatchId: null, + messages: [{ messageId: 'mail-1', runId: 'run-1', from: 'term_a' }] + } + } + } + })! + const boundaries = [ + notice, + { + ...notice, + from: readAgentMessageSource({ kind: 'agent', senders: null, orchestration: null }) + }, + { + ...notice, + from: readAgentMessageSource({ + kind: 'agent', + senders: [], + orchestration: { message: 'future-notice' } + }) + }, + ...( + ['queued', 'unsent', 'stoppedBeforeStart', 'awaitsRetry', 'sentWhileStopping'] as const + ).map((flag) => ({ ...notice, from: undefined, [flag]: true })) + ] + it.each(boundaries)('preserves blocking FIFO across non-answer row %#', (boundary) => { + const rows = [ + message('ordinary', [call('Bash', { command: 'ls' })]), + message('ask', [call('AskUserQuestion', QUESTIONS_INPUT)]), + boundary, + toolTurn('ordinary-result'), + toolTurn('ask-result') + ] + const scanner = createPendingAskScanner() + const replay: NativeChatMessage[] = [] + for (const row of rows) { + replay.push(row) + scanner.append(row) + expect(scanner.current()).toEqual(scanPendingAsks(replay)) + if (replay.length >= 2 && replay.length < rows.length) { + expect(scanner.current().blocking?.questions[0]?.question).toBe('Deploy?') + } + } + expect(scanner.current().blocking).toBeNull() + expect( + scanPendingAsks([rows[0]!, rows[1]!, userTurn('human', 'never mind, do this instead')]) + .blocking + ).toBeNull() + }) + + // Normalized source-shaped inputs: OMP toolCall.arguments and OpenCode pending state.input; no decoder imports. + // Grok question definition copied from src/main/acp/fixtures/s5-plan-approved.jsonl:6 (recorded ACP, not terminal delivery). + it.each([ + ['OMP/OpenCode canonical shape', 'ask', QUESTIONS_INPUT], + [ + 'Grok', + 'ask_user_question', + { + questions: [ + { + question: 'Which test approach do you want for the subtract function?', + options: [ + { + label: 'node:test (Recommended)', + description: + "A math.test.js file using Node's built-in test runner. Run it with node --test. No new dependencies or package.json." + }, + { + label: 'Plain assert script', + description: + 'A small math.test.js that uses node:assert and exits non-zero on failure. Run it with node math.test.js. No test framework.' + }, + { + label: 'Add a package.json and a test script', + description: + 'Add package.json with a test script that runs the same Node built-in tests, so npm test works.' + } + ], + multiSelect: null + } + ] + } + ] + ])('retains %s normalized blocking input across a projected notice', (_agent, name, input) => { + const asked = message('ask', [call(String(name), input)]) + const pending = scanPendingAsks([asked]).blocking + expect(pending).not.toBeNull() + expect(scanPendingAsks([asked, notice]).blocking).toEqual(pending) + expect(scanPendingAsks([asked, notice, toolTurn('answered')]).blocking).toBeNull() + }) +}) + describe('extractPendingAsk', () => { it('recognizes an unregistered tool whose input matches the canonical questions shape', () => { // Match live status’s arbitrary-tool canonical-shape fallback during transcript replay. @@ -335,7 +444,7 @@ describe('async questions', () => { expect(isDeliveredNativeChatUserMessage(notice)).toBe(false) scanner.append(notice) expect(scanner.current().contributors).toBe(pending) - expect(scanner.current().blocking).toBeNull() + expect(scanner.current().blocking).not.toBeNull() expect(scanner.append(message('replay', [asyncCall('same-call', 'duplicate')]))).toEqual([]) scanner.append({ ...notice, id: 'legacy-human', from: undefined }) expect(scanner.current().contributors).toEqual([]) diff --git a/src/shared/native-chat/input/native-chat-ask.ts b/src/shared/native-chat/input/native-chat-ask.ts index ab9a015fb60..9c821b7a6e5 100644 --- a/src/shared/native-chat/input/native-chat-ask.ts +++ b/src/shared/native-chat/input/native-chat-ask.ts @@ -141,7 +141,7 @@ export function extractPendingMessageReplyAsk( return aggregateAsyncQuestions(scanPendingAsks(messages).contributors) } -/** Delivered human input only; agent notices and pending sends answer no async question. */ +/** Delivered human input only; agent notices and pending sends answer no question. */ export function isDeliveredNativeChatUserMessage(message: NativeChatMessage): boolean { return ( message.role === 'user' && @@ -257,15 +257,9 @@ export function createPendingAskScanner(): { } const append = (message: NativeChatMessage): readonly AsyncQuestionContributor[] => { const accepted: AsyncQuestionContributor[] = [] - // New user turns/interrupts abandon unresolved calls. Decoding drops tool_use_id, so orphan results cannot match by id; - // reset the FIFO or one orphan strands later answered asks forever (#11761). Claude tool results decode as role tool. - const interrupted = isInterruptedStatusMessage(message) - if (message.role === 'user' || interrupted) { - if (interrupted || isDeliveredNativeChatUserMessage(message)) { - interrupt() - } else { - resetBlocking() - } + // Delivered human turns/interrupts abandon the uncorrelated FIFO, or orphan calls strand later asks (#11761). + if (isDeliveredNativeChatUserMessage(message) || isInterruptedStatusMessage(message)) { + interrupt() } for (const block of message.blocks) { if (block.type === 'tool-call') {