diff --git a/mobile/src/session/use-mobile-diff-review-send-actions.test.ts b/mobile/src/session/use-mobile-diff-review-send-actions.test.ts index 1188c44ecb4..74c018abdc7 100644 --- a/mobile/src/session/use-mobile-diff-review-send-actions.test.ts +++ b/mobile/src/session/use-mobile-diff-review-send-actions.test.ts @@ -3,6 +3,7 @@ import { act, create, type ReactTestRenderer } from 'react-test-renderer' import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' import type { DiffComment } from '../../../src/shared/diff-comment-types' import type { RpcClient } from '../transport/rpc-client' +import type { RpcResponse } from '../transport/types' import type { ReviewScreenState } from './mobile-diff-review-screen-model' import { isMobileNativeChatInputStale, @@ -59,30 +60,35 @@ const LAUNCH_CAPABILITIES = [ 'agent.launch.replay-required.v1' ] -// Answers the agent loader's reads and the launch, by method. -function launchClient(promptOutcome: 'handed-to-terminal' | 'not-delivered') { - const reply = (result: unknown) => ({ - id: 'rpc', - ok: true as const, - result, - _meta: { runtimeId: 'r' } +function rpcReply(result: unknown) { + return { id: 'rpc', ok: true as const, result, _meta: { runtimeId: 'r' } } +} + +function launchedReply(promptOutcome: 'handed-to-terminal' | 'not-delivered') { + return rpcReply({ + outcome: { kind: 'terminal', handle: 'term-1' }, + worktreeId: 'wt-1', + receipt: { mode: 'terminal', preferred: 'terminal', reason: 'user_default', detail: 'd' }, + prompt: { delivery: 'submit', outcome: promptOutcome } }) - const sendRequest = vi.fn(async (method: string, _params?: unknown) => { +} + +// Answers the agent loader's reads and the launch, by method. +function launchClient( + promptOutcome: 'handed-to-terminal' | 'not-delivered', + launchReply: () => Promise = async () => launchedReply(promptOutcome) +) { + const sendRequest = vi.fn(async (method: string, _params?: unknown): Promise => { if (method === 'repo.list') { - return reply({ repos: [{ id: 'wt-1' }] }) + return rpcReply({ repos: [{ id: 'wt-1' }] }) } if (method === 'settings.get') { - return reply({ settings: { defaultTuiAgent: 'codex' } }) + return rpcReply({ settings: { defaultTuiAgent: 'codex' } }) } if (method === 'preflight.detectAgents') { - return reply(['codex']) + return rpcReply(['codex']) } - return reply({ - outcome: { kind: 'terminal', handle: 'term-1' }, - worktreeId: 'wt-1', - receipt: { mode: 'terminal', preferred: 'terminal', reason: 'user_default', detail: 'd' }, - prompt: { delivery: 'submit', outcome: promptOutcome } - }) + return launchReply() }) return { client: requestPortRpcClient(sendRequest), sendRequest } } @@ -102,8 +108,10 @@ describe('useMobileDiffReviewSendActions', () => { let setActionError: ReturnType let setSendSheet: ReturnType let saveCommentsAndReviewState: ReturnType + let screenState: ReviewScreenState = READY beforeEach(() => { + screenState = READY clipboardMock.setStringAsync.mockReset().mockResolvedValue(true) resetMobileNativeChatStaleInputForTests() setActionError = vi.fn() @@ -124,7 +132,7 @@ describe('useMobileDiffReviewSendActions', () => { connState: 'connected', hostCapabilities: LAUNCH_CAPABILITIES, worktreeId: 'wt-1', - screenState: READY, + screenState, setActionError, setSendSheet, saveCommentsAndReviewState @@ -318,4 +326,60 @@ describe('useMobileDiffReviewSendActions', () => { "The agent started, but the notes weren't sent. Use Copy Notes to paste them." ) }) + + it('shows a launch that did not start on the review screen instead of rejecting', async () => { + await mount( + launchClient('handed-to-terminal', async () => ({ + id: 'rpc', + ok: false, + error: { code: 'selector_not_found', message: 'Workspace not found' }, + _meta: { runtimeId: 'r' } + })).client + ) + await act(async () => { + await actions?.createTerminalAndSend([COMMENT]) + }) + expect(setSendSheet).toHaveBeenCalledWith(null) + expect(setActionError).toHaveBeenLastCalledWith('Workspace not found') + expect(saveCommentsAndReviewState).not.toHaveBeenCalled() + }) + + it('starts one agent for a double tap and keeps notes written while it starts', async () => { + let answerLaunch: (reply: RpcResponse) => void = () => {} + const { client, sendRequest } = launchClient( + 'handed-to-terminal', + () => + new Promise((resolve) => { + answerLaunch = resolve + }) + ) + await mount(client) + let first: Promise | undefined + await act(async () => { + first = actions?.createTerminalAndSend([COMMENT]) + await actions?.createTerminalAndSend([COMMENT]) + }) + expect(setActionError).toHaveBeenLastCalledWith('Starting an agent...') + const written: DiffComment = { ...COMMENT, id: 'comment-2', body: 'written meanwhile' } + screenState = { ...READY, comments: [COMMENT, written] } + await act(async () => { + renderer?.update(createElement(Harness)) + }) + await act(async () => { + answerLaunch(launchedReply('handed-to-terminal')) + await first + }) + expect( + sendRequest.mock.calls.filter(([method]) => method === 'agent.launchReplay') + ).toHaveLength(1) + expect(saveCommentsAndReviewState).toHaveBeenCalledWith( + [ + expect.objectContaining({ id: 'comment-1', sentAt: expect.any(Number) }), + expect.not.objectContaining({ sentAt: expect.anything() }) + ], + READY.reviewState + ) + expect(saveCommentsAndReviewState.mock.calls[0]?.[0]?.[1]?.id).toBe('comment-2') + expect(setActionError).toHaveBeenLastCalledWith('Review notes sent') + }) }) diff --git a/mobile/src/session/use-mobile-diff-review-send-actions.ts b/mobile/src/session/use-mobile-diff-review-send-actions.ts index fb77ae386bd..42b0688eb05 100644 --- a/mobile/src/session/use-mobile-diff-review-send-actions.ts +++ b/mobile/src/session/use-mobile-diff-review-send-actions.ts @@ -1,4 +1,4 @@ -import { useCallback, type Dispatch, type SetStateAction } from 'react' +import { useCallback, useEffect, useRef, type Dispatch, type SetStateAction } from 'react' import type { DiffComment, MobileDiffReviewState } from '../../../src/shared/diff-comment-types' import type { ConnectionState } from '../transport/types' import type { RpcClient } from '../transport/rpc-client' @@ -65,19 +65,29 @@ export function useMobileDiffReviewSendActions(input: SendActionsInput) { await saveCommentsAndReviewState(nextComments, screenState.reviewState) }, [saveCommentsAndReviewState, screenState]) + // Read when a send settles, not when it was tapped: an agent launch can take a minute, and notes + // written meanwhile must survive the whole-list save below. + const latestScreenStateRef = useRef(screenState) + useEffect(() => { + latestScreenStateRef.current = screenState + }, [screenState]) + // One launch at a time: each tap is a new operation, so a second tap would start a second agent. + const agentLaunchInFlightRef = useRef(false) + const markNotesSent = useCallback( async (comments: readonly DiffComment[]) => { - if (screenState.kind !== 'ready') { + const current = latestScreenStateRef.current + if (current.kind !== 'ready') { return } const next = markMobileDiffCommentsSent( - screenState.comments, + current.comments, new Set(comments.map((comment) => comment.id)), Date.now() ) - await saveCommentsAndReviewState(next, screenState.reviewState) + await saveCommentsAndReviewState(next, current.reviewState) }, - [saveCommentsAndReviewState, screenState] + [saveCommentsAndReviewState] ) const sendPromptToTerminal = useCallback( @@ -116,20 +126,32 @@ export function useMobileDiffReviewSendActions(input: SendActionsInput) { if (!client || connState !== 'connected') { throw new Error('Waiting for desktop...') } - // The desktop asks which agent to use; the review screen has no picker, so this takes the - // desktop's own default resolution with no saved recipe. - const result = await launchAgentWithPrompt({ - client, - hostCapabilities, - worktreeId, - prompt: formatMobileDiffReviewPrompt(comments), - actionId: null, - launchSource: 'notes_send' - }) - if (result.kind === 'not-started' || result.kind === 'unconfirmed') { - throw new Error(result.message) + if (agentLaunchInFlightRef.current) { + return } + agentLaunchInFlightRef.current = true + // Closed up front so the wait (up to a minute for a terminal agent) shows its progress here. setSendSheet(null) + setActionError('Starting an agent...') + let result + try { + // The desktop asks which agent to use; the review screen has no picker, so this takes the + // desktop's own default resolution with no saved recipe. + result = await launchAgentWithPrompt({ + client, + hostCapabilities, + worktreeId, + prompt: formatMobileDiffReviewPrompt(comments), + actionId: null, + launchSource: 'notes_send' + }) + } finally { + agentLaunchInFlightRef.current = false + } + if (result.kind === 'not-started' || result.kind === 'unconfirmed') { + setActionError(result.message) + return + } if (result.kind === 'prompt-not-sent') { // Notes stay unsent so Copy Notes and a later send still carry them. setActionError(