diff --git a/apps/mobile/src/app/_layout.tsx b/apps/mobile/src/app/_layout.tsx index 9ecfe680e6..53145363a0 100644 --- a/apps/mobile/src/app/_layout.tsx +++ b/apps/mobile/src/app/_layout.tsx @@ -335,7 +335,10 @@ function RootLayoutNav() { accountId: userId, optionalConsent, }); - useScreenTracking(); + // Screen capture must wait for consent: analytics eligibility is decided + // only after the account's consent decision has loaded without error. + const bootstrapSettled = token != null && consentChecked && !needsConsent && !consentCheckError; + useScreenTracking(bootstrapSettled); useEffect(() => { if (shareIntentError) { diff --git a/apps/mobile/src/components/agents/mobile-session-manager.test.ts b/apps/mobile/src/components/agents/mobile-session-manager.test.ts index 9c629066db..2be65c829f 100644 --- a/apps/mobile/src/components/agents/mobile-session-manager.test.ts +++ b/apps/mobile/src/components/agents/mobile-session-manager.test.ts @@ -1,4 +1,5 @@ /* eslint-disable require-await, @typescript-eslint/require-await -- injectable query/sleep fakes settle without await */ +/* eslint-disable max-lines -- the manager suite pins retry cadence and attachment mints in one file. */ import { beforeEach, describe, expect, it, vi } from 'vitest'; import { type AgentAttachmentSubmissionPayload } from '@/lib/agent-attachments/agent-attachment-types'; @@ -41,17 +42,22 @@ vi.mock('@/components/agents/tool-card-image-cache', () => ({ })); const mutate = vi.fn(); +const prepareSessionMutate = vi.fn(); vi.mock('@/lib/trpc', () => ({ trpcClient: { cloudAgentNext: { getAttachmentDownloadUrl: { mutate }, + prepareSession: { mutate: prepareSessionMutate }, + }, + organizations: { + cloudAgentNext: { prepareSession: { mutate: prepareSessionMutate } }, }, }, })); const { buildRemoteAttachmentParts } = await import('@/components/agents/mobile-session-manager-helpers'); -const { fetchSessionWithNotFoundRetry, readFetchSessionErrorCode } = +const { fetchSessionWithNotFoundRetry, isCloudPrepareRetryableError, readFetchSessionErrorCode } = await import('@/components/agents/mobile-session-manager'); const SESSION_ID = 'ses_test_session_id_0000000001' as KiloSessionId; @@ -62,6 +68,10 @@ function notFoundError(): Error { return error; } +function withCode(code: string, message: string): Error { + return Object.assign(new Error(message), { data: { code } }); +} + describe('buildRemoteAttachmentParts', () => { beforeEach(() => { mutate.mockReset(); @@ -189,6 +199,46 @@ describe('readFetchSessionErrorCode', () => { }); }); +describe('isCloudPrepareRetryableError', () => { + it('keeps the key for creation_in_progress (CONFLICT)', () => { + expect(isCloudPrepareRetryableError(withCode('CONFLICT', 'creation_in_progress'))).toBe(true); + }); + + it('keeps the key for a network error with no tRPC code', () => { + expect(isCloudPrepareRetryableError(new Error('Network request failed'))).toBe(true); + }); + + it('keeps the key for transient 5xx-class and rate-limit codes', () => { + for (const code of [ + 'INTERNAL_SERVER_ERROR', + 'BAD_GATEWAY', + 'SERVICE_UNAVAILABLE', + 'GATEWAY_TIMEOUT', + 'TIMEOUT', + 'TOO_MANY_REQUESTS', + ]) { + expect(isCloudPrepareRetryableError(withCode(code, 'boom'))).toBe(true); + } + }); + + it('rotates the key on typed terminal rejections', () => { + for (const code of [ + 'BAD_REQUEST', + 'UNAUTHORIZED', + 'FORBIDDEN', + 'NOT_FOUND', + 'PAYMENT_REQUIRED', + 'PRECONDITION_FAILED', + ]) { + expect(isCloudPrepareRetryableError(withCode(code, 'nope'))).toBe(false); + } + }); + + it('rotates the key on a CONFLICT with any other message', () => { + expect(isCloudPrepareRetryableError(withCode('CONFLICT', 'something else'))).toBe(false); + }); +}); + describe('fetchSessionWithNotFoundRetry', () => { // Production return type is SessionWithRuntimeState; tests inject a minimal // stand-in via `query` and only assert retry/cadence behavior. diff --git a/apps/mobile/src/components/agents/mobile-session-manager.ts b/apps/mobile/src/components/agents/mobile-session-manager.ts index 357d6e3559..55880e624d 100644 --- a/apps/mobile/src/components/agents/mobile-session-manager.ts +++ b/apps/mobile/src/components/agents/mobile-session-manager.ts @@ -64,6 +64,38 @@ export function readFetchSessionErrorCode(error: unknown): string | undefined { return undefined; } +/** + * tRPC codes transient enough to keep the same cloud-prepare `operationKey` + * across a retry. Any other typed code is a terminal rejection and rotates it. + */ +const CLOUD_PREPARE_TRANSIENT_CODES = new Set([ + 'INTERNAL_SERVER_ERROR', + 'BAD_GATEWAY', + 'SERVICE_UNAVAILABLE', + 'GATEWAY_TIMEOUT', + 'TIMEOUT', + 'TOO_MANY_REQUESTS', +]); + +/** Stable message the ledger returns on a same-key in-flight duplicate (plan P1-A-08b). */ +const CLOUD_PREPARE_IN_PROGRESS_MESSAGE = 'creation_in_progress'; + +/** + * True when a `prepareSession` failure may be retried with the SAME + * `operationKey`: `creation_in_progress`, a transient 5xx, or a codeless + * transport failure (the ledger reconciles the ambiguous prior attempt). + */ +export function isCloudPrepareRetryableError(error: unknown): boolean { + const code = readFetchSessionErrorCode(error); + if (code === undefined) { + return true; + } + if (code === 'CONFLICT') { + return error instanceof Error && error.message === CLOUD_PREPARE_IN_PROGRESS_MESSAGE; + } + return CLOUD_PREPARE_TRANSIENT_CODES.has(code); +} + /* eslint-disable @typescript-eslint/promise-function-async, require-await -- thin tRPC passthrough */ async function defaultFetchSessionQuery( sessionId: KiloSessionId diff --git a/apps/mobile/src/components/agents/use-continue-cloud-create.ts b/apps/mobile/src/components/agents/use-continue-cloud-create.ts new file mode 100644 index 0000000000..c3f2771370 --- /dev/null +++ b/apps/mobile/src/components/agents/use-continue-cloud-create.ts @@ -0,0 +1,103 @@ +// The cloud-agent leg of `useContinueSession`: one `prepareSession` call, its +// hoisted operation key, and the contained post-success UI work. Split out of +// `use-continue-session.ts` (which keeps the paging drain, destination +// resolution, and the remote spawn leg) so each file stays under the +// max-lines limit. +import { useCallback } from 'react'; +import { useRouter } from 'expo-router'; +import { useQueryClient } from '@tanstack/react-query'; +import { generateMessageId } from '@kilocode/cloud-agent-sdk/message-id'; +import * as Haptics from 'expo-haptics'; + +import { normalizeAgentMode } from '@/components/agents/mode-options'; +import { isCloudPrepareRetryableError } from '@/components/agents/mobile-session-manager'; +import { getAgentSessionPath } from '@/components/agents/session-detail-routes'; +import { useHoistedOperationKey } from '@/lib/operation-key'; +import { captureEvent, SESSION_CREATED_EVENT } from '@/lib/analytics/posthog'; +import { invalidateAgentSessionQueries } from '@/lib/agent-session-cache'; +import { trpcClient, useTRPC } from '@/lib/trpc'; + +export function useContinueCloudCreate( + organizationId: string | undefined +): ( + seed: string, + dest: { repo: string; model: string; variant: string }, + mode: string +) => Promise { + const router = useRouter(); + const queryClient = useQueryClient(); + const trpc = useTRPC(); + // P1-A-08b: cloud prepares and remote spawns are different intents, so each + // destination family holds its own hoisted `operationKey`. + const cloudOperationKey = useHoistedOperationKey(); + + return useCallback( + async (seed: string, dest: { repo: string; model: string; variant: string }, mode: string) => { + const intentFingerprint = JSON.stringify({ + seed, + repo: dest.repo, + model: dest.model, + variant: dest.variant || undefined, + mode, + organizationId: organizationId ?? null, + }); + const operationKey = cloudOperationKey.getKey(intentFingerprint); + const initialMessageId = generateMessageId(); + const baseInput = { + prompt: seed, + initialMessageId, + mode: normalizeAgentMode(mode), + model: dest.model, + variant: dest.variant || undefined, + githubRepo: dest.repo, + autoCommit: true, + autoInitiate: true, + operationKey, + }; + try { + const result = organizationId + ? await trpcClient.organizations.cloudAgentNext.prepareSession.mutate({ + ...baseInput, + organizationId, + }) + : await trpcClient.cloudAgentNext.prepareSession.mutate(baseInput); + // The intent settled; the next submit is a fresh intent. Rotate + // before the post-success work so a UI failure cannot keep the + // successful key for a retry or rotate it a second time. + cloudOperationKey.rotateKey(); + + // The cloud session already exists, so no post-success UI failure may + // report the create as failed or invite a duplicate retry. Each step is + // contained on its own so one failure cannot skip the navigation. + try { + captureEvent(SESSION_CREATED_EVENT, { surface: 'cloud-agent' }); + } catch { + // Analytics is best-effort; stay silent. + } + try { + await invalidateAgentSessionQueries(queryClient, trpc); + } catch { + // A failed cache invalidation is cosmetic; navigation must still run. + } + try { + await Haptics.notificationAsync(Haptics.NotificationFeedbackType.Success); + } catch { + // A failed haptics call is cosmetic; stay silent and navigate. + } + try { + router.push(getAgentSessionPath(result.kiloSessionId, organizationId)); + } catch { + // A navigation failure is not a create failure. + } + } catch (error) { + // Only `prepareSession` errors reach here; UI failures are contained + // above. A typed terminal rejection ends the intent. + if (!isCloudPrepareRetryableError(error)) { + cloudOperationKey.rotateKey(); + } + throw error; + } + }, + [organizationId, queryClient, router, trpc, cloudOperationKey] + ); +} diff --git a/apps/mobile/src/components/agents/use-continue-session.test.ts b/apps/mobile/src/components/agents/use-continue-session.test.ts new file mode 100644 index 0000000000..ba060e373c --- /dev/null +++ b/apps/mobile/src/components/agents/use-continue-session.test.ts @@ -0,0 +1,558 @@ +/* eslint-disable import/first -- mocks must be defined before the module under test is imported */ +/* eslint-disable max-lines -- the suite pins both key families (cloud prepare + remote spawn) through one fake-dispatcher runner. */ +import * as React from 'react'; +import { atom, type createStore } from 'jotai'; +import { beforeEach, describe, expect, it, vi } from 'vitest'; +import { type KiloSessionId } from '@kilocode/cloud-agent-sdk'; +import { type SessionModelOption } from '@/lib/hooks/use-session-model-options'; + +type JotaiStore = ReturnType; + +// P1-A-08b: `useContinueSession` keeps TWO hoisted operation keys — one per +// destination family. Cloud prepares and remote spawns are different +// intents, so they never share a key; each is kept across retryable +// failures (so the ledger/relay dedupes the same-key retry) and rotated on +// success or a typed terminal rejection. This suite pins both families +// through a fake React dispatcher, mocking only the outside world. +// +// Destination resolution is deliberately mocked: this suite tests KEY +// WIRING, not `resolveContinuationDestinations` (which has its own module). + +const prepareSessionMutate = vi.hoisted(() => vi.fn()); +const remoteSpawnMock = vi.hoisted(() => + vi.fn( + // eslint-disable-next-line require-await, typescript-eslint/require-await -- mock returns a settled outcome without awaiting + async ( + _connectionId: string, + _opts?: unknown, + _options?: unknown + ): Promise => ({ + status: 'retryable', + reason: 'Connection destroyed', + cause: new Error('Connection destroyed'), + }) + ) +); +const routerPush = vi.hoisted(() => vi.fn()); +const queryClientFetchQuery = vi.hoisted(() => vi.fn()); +const showActionSheetWithOptions = vi.hoisted(() => vi.fn()); +const toastError = vi.hoisted(() => vi.fn()); +// Post-success side effects; tests reject them to pin the containment +// boundary around the successful cloud prepare. +const invalidateAgentSessionQueriesMock = vi.hoisted(() => vi.fn()); +// Not a `vi.fn()`: vitest attaches its own rejection handler to any promise a +// mock returns, which would mask a leaked haptics rejection. A plain module +// export returning a real promise keeps `unhandledRejection` detection honest. +const hapticsMock = vi.hoisted(() => ({ + calls: 0, + rejectWith: undefined as Error | undefined, +})); +// Destination list handed back by the mocked resolver; each test sets the +// single destination the continue flow should execute against. +const destinationsRef = vi.hoisted(() => ({ value: [] as unknown[] })); +// Lazy jotai store: `useStore()` returns one store for the whole suite and +// `store.get(manager.atoms.*)` reads the atoms' seeded initial values. +const storeRef = vi.hoisted(() => ({ + current: undefined as JotaiStore | undefined, +})); + +vi.mock('expo-router', () => ({ + useRouter: () => ({ push: routerPush }), +})); +vi.mock('@tanstack/react-query', () => ({ + useQueryClient: () => ({ fetchQuery: queryClientFetchQuery }), +})); +vi.mock('jotai', async importOriginal => { + // eslint-disable-next-line @typescript-eslint/consistent-type-imports -- namespace type for the real jotai module under vi.mock + const actual = await importOriginal(); + return { + ...actual, + useStore: () => { + storeRef.current ??= actual.createStore(); + return storeRef.current; + }, + }; +}); +vi.mock('@expo/react-native-action-sheet', () => ({ + useActionSheet: () => ({ showActionSheetWithOptions }), +})); +vi.mock('@kilocode/cloud-agent-sdk/message-id', () => ({ + generateMessageId: () => 'msg-1', +})); +vi.mock('expo-haptics', () => ({ + notificationAsync: async (): Promise => { + hapticsMock.calls += 1; + await Promise.resolve(); + if (hapticsMock.rejectWith !== undefined) { + throw hapticsMock.rejectWith; + } + }, + NotificationFeedbackType: { Success: 'success' }, +})); +vi.mock('sonner-native', () => ({ + toast: { error: toastError }, +})); +vi.mock('@/lib/analytics/posthog', () => ({ + captureEvent: vi.fn(), + SESSION_CREATED_EVENT: 'session_created', +})); +vi.mock('@/lib/agent-session-cache', () => ({ + invalidateAgentSessionQueries: invalidateAgentSessionQueriesMock, +})); +vi.mock('@/lib/share-payload', () => ({ + putSharePayload: () => 'share-1', +})); +vi.mock('@/lib/share-navigation', () => ({ + appendShareParams: (base: string) => base, +})); +vi.mock('@/lib/trpc', () => ({ + trpcClient: { + cloudAgentNext: { prepareSession: { mutate: prepareSessionMutate } }, + organizations: { cloudAgentNext: { prepareSession: { mutate: prepareSessionMutate } } }, + }, + useTRPC: () => ({ + cloudAgentNext: { + listGitHubRepositories: { queryOptions: () => ({ queryKey: ['repositories'] }) }, + }, + organizations: { + cloudAgentNext: { + listGitHubRepositories: { queryOptions: () => ({ queryKey: ['repositories'] }) }, + }, + }, + activeSessions: { listInstances: { queryOptions: () => ({ queryKey: ['instances'] }) } }, + }), +})); +// The real classifier lives in mobile-session-manager (covered by its own +// suite); this test only needs the retryable/non-retryable split. +vi.mock('@/components/agents/mobile-session-manager', () => ({ + isCloudPrepareRetryableError: (error: unknown) => { + const record = error as { data?: { code?: string }; message?: string }; + return record.data?.code === 'CONFLICT' && record.message === 'creation_in_progress'; + }, +})); +vi.mock('@/components/agents/mode-options', () => ({ + normalizeAgentMode: (mode: string | null | undefined) => + mode === 'code' || + mode === 'plan' || + mode === 'debug' || + mode === 'orchestrator' || + mode === 'ask' + ? mode + : 'code', +})); +vi.mock('@/components/agents/new-session-prefill', () => ({ + appendNewSessionPrefill: (base: string) => base, + buildContinuePrefillParams: () => ({}), +})); +// The real continuation-seed module pulls in mode-options -> lucide-react-native +// (RN tree); this suite pins key wiring, so the builders are test hooks. +vi.mock('@/components/agents/continuation-seed', () => ({ + buildContinuationSeed: () => 'seed-text', + buildContinueRemoteSpawnInput: () => undefined, + resolveContinuationDestinations: () => destinationsRef.value, +})); +vi.mock('@/components/agents/user-web-connection-provider', () => ({ + useUserWebConnection: () => ({}), +})); +// Keep the real input builder; only stub the RN-touching spawn hook. The +// builder is imported from the pure classifier module — the hook module +// itself pulls in react-native via `useUserWebConnection` and cannot load +// under the plain Node vitest environment (see the classifier's header). +vi.mock('@/lib/hooks/use-remote-instance-spawn', () => ({ + buildCreateRemoteSessionInput, + useRemoteInstanceSpawn: () => ({ spawn: remoteSpawnMock }), +})); +vi.mock('expo-crypto', () => { + let n = 0; + return { + randomUUID: () => { + n += 1; + return `op-key-${n}`; + }, + }; +}); + +// The pure input builder must be imported BEFORE the module under test: the +// mocked `use-remote-instance-spawn` factory reads this binding when +// `use-continue-session.ts` loads it. +import { + buildCreateRemoteSessionInput, + type CreateSessionOutcome, +} from '@/lib/hooks/remote-instance-spawn-classifier'; +import { useContinueSession } from './use-continue-session'; + +function creationInProgressError(): Error { + return Object.assign(new Error('creation_in_progress'), { data: { code: 'CONFLICT' } }); +} + +function badRequestError(): Error { + return Object.assign(new Error('session_creation_failed'), { data: { code: 'BAD_REQUEST' } }); +} + +function retryableOutcome() { + return { + status: 'retryable' as const, + reason: 'Connection destroyed', + cause: new Error('Connection destroyed'), + }; +} + +function readyOutcome(): CreateSessionOutcome { + return { + status: 'ready', + sessionID: 'ses_12345678901234567890123456' as KiloSessionId, + }; +} + +function nonRetryableOutcome() { + return { + status: 'nonRetryable' as const, + reason: 'CLI_UPGRADE_REQUIRED', + cause: new Error('CLI_UPGRADE_REQUIRED'), + }; +} + +// Fake manager over real jotai atoms; the store's `get` reads the seeded +// initial values, so `hasOlderMessages` stays false and the drain loop is a +// no-op while `messagesList` is non-empty for the seed builder. +const hasOlderMessagesAtom = atom(false); +const messagesListAtom = atom([ + { info: { role: 'user' }, parts: [{ type: 'text', text: 'hello' }] }, +]); +const manager = { + atoms: { hasOlderMessages: hasOlderMessagesAtom, messagesList: messagesListAtom }, + // eslint-disable-next-line no-empty-function -- seeded atoms keep the drain loop a no-op + loadOlderMessages: async () => {}, +}; + +type ReactInternals = { + __CLIENT_INTERNALS_DO_NOT_USE_OR_WARN_USERS_THEY_CANNOT_UPGRADE: { + H: unknown; + }; +}; + +type HookDispatcher = { + useCallback: (callback: T, _deps?: unknown) => T; + useRef: (initial: T) => { current: T }; + useState: (initialValue: T) => [T, (value: T | ((previous: T) => T)) => void]; +}; + +type ContinueSessionResult = ReturnType; + +function runContinueSession(args: { + organizationId?: string; + models?: SessionModelOption[]; +}): ContinueSessionResult { + const reactInternals = React as typeof React & ReactInternals; + const hookState: unknown[] = []; + const refs: { current: unknown }[] = []; + let hookIndex = 0; + let refIndex = 0; + + const dispatcher: HookDispatcher = { + useCallback: hookCallback => { + hookIndex += 1; + return hookCallback; + }, + useRef: initial => { + const index = refIndex; + refIndex += 1; + refs[index] ??= { current: initial }; + return refs[index] as { current: typeof initial }; + }, + useState: initialValue => { + const stateIndex = hookIndex; + hookIndex += 1; + if (hookState[stateIndex] === undefined) { + hookState[stateIndex] = initialValue; + } + const setState = ( + value: typeof initialValue | ((previous: typeof initialValue) => typeof initialValue) + ) => { + hookState[stateIndex] = + typeof value === 'function' + ? (value as (previous: typeof initialValue) => typeof initialValue)( + hookState[stateIndex] as typeof initialValue + ) + : value; + }; + return [hookState[stateIndex] as typeof initialValue, setState]; + }, + }; + + const previousDispatcher = + reactInternals.__CLIENT_INTERNALS_DO_NOT_USE_OR_WARN_USERS_THEY_CANNOT_UPGRADE.H; + reactInternals.__CLIENT_INTERNALS_DO_NOT_USE_OR_WARN_USERS_THEY_CANNOT_UPGRADE.H = dispatcher; + try { + // eslint-disable-next-line react-hooks/rules-of-hooks -- fake dispatcher drives the hook in a plain vitest run + return useContinueSession({ + organizationId: args.organizationId, + manager: manager as never, + models: args.models ?? [], + modelsLoading: false, + }); + } finally { + reactInternals.__CLIENT_INTERNALS_DO_NOT_USE_OR_WARN_USERS_THEY_CANNOT_UPGRADE.H = + previousDispatcher; + } +} + +const CLOUD_DESTINATION = { + kind: 'cloud-agent', + repo: 'owner/repo', + model: 'model-1', + variant: 'v1', +}; +const REMOTE_DESTINATION = { + kind: 'remote', + instance: { connectionId: 'conn-1', name: 'laptop', projectName: 'kilo' }, +}; +const FIELDS = { gitUrl: null, mode: 'code', model: 'model-1', variant: 'v1' }; + +function usedCloudKeys(): (string | undefined)[] { + return prepareSessionMutate.mock.calls.map( + call => (call[0] as { operationKey?: string }).operationKey + ); +} + +function usedRemoteKeys(): (string | undefined)[] { + return remoteSpawnMock.mock.calls.map( + call => (call[2] as { operationKey?: string } | undefined)?.operationKey + ); +} + +describe('useContinueSession cloud operationKey', () => { + beforeEach(() => { + prepareSessionMutate.mockReset(); + remoteSpawnMock.mockReset(); + routerPush.mockClear(); + queryClientFetchQuery.mockReset(); + toastError.mockClear(); + invalidateAgentSessionQueriesMock.mockReset(); + hapticsMock.calls = 0; + hapticsMock.rejectWith = undefined; + destinationsRef.value = [CLOUD_DESTINATION]; + // fetchQuery: first call is the repositories query, second the instances + // query (Promise.all preserves call order). Both must resolve for the + // destination resolution step to proceed. + // eslint-disable-next-line typescript-eslint/promise-function-async -- conflicting require-await rule + queryClientFetchQuery.mockImplementation((options: { queryKey?: string[] }) => { + if (options.queryKey?.[0] === 'instances') { + return Promise.resolve({ instances: [] }); + } + return Promise.resolve({ repositories: [] }); + }); + }); + + it('keeps the same cloud operationKey across retryable creation_in_progress failures', async () => { + prepareSessionMutate + .mockRejectedValueOnce(creationInProgressError()) + .mockRejectedValueOnce(creationInProgressError()) + .mockResolvedValueOnce({ kiloSessionId: 'ses_12345678901234567890123456' }); + const hook = runContinueSession({ organizationId: 'org-1' }); + + await hook.continueSession(FIELDS); + await hook.continueSession(FIELDS); + + const keys = usedCloudKeys(); + expect(keys[0]).toBeDefined(); + expect(keys[1]).toBe(keys[0]); + expect(prepareSessionMutate.mock.calls[0]?.[0]).toMatchObject({ + prompt: 'seed-text', + githubRepo: 'owner/repo', + autoInitiate: true, + operationKey: expect.any(String), + }); + }); + + it('rotates the cloud operationKey after a successful prepare', async () => { + prepareSessionMutate + .mockRejectedValueOnce(creationInProgressError()) + .mockResolvedValueOnce({ kiloSessionId: 'ses_12345678901234567890123456' }) + .mockRejectedValueOnce(creationInProgressError()); + const hook = runContinueSession({ organizationId: 'org-1' }); + + await hook.continueSession(FIELDS); + await hook.continueSession(FIELDS); + await hook.continueSession(FIELDS); + + const keys = usedCloudKeys(); + // The successful retry rides the same key as the retryable attempt. + expect(keys[1]).toBe(keys[0]); + // The submit after success is a fresh intent with a fresh key. + expect(keys[2]).not.toBe(keys[0]); + }); + + it('rotates the cloud operationKey after a typed non-retryable rejection', async () => { + prepareSessionMutate + .mockRejectedValueOnce(badRequestError()) + .mockRejectedValueOnce(creationInProgressError()); + const hook = runContinueSession({ organizationId: 'org-1' }); + + await hook.continueSession(FIELDS); + await hook.continueSession(FIELDS); + + const keys = usedCloudKeys(); + expect(keys[0]).toBeDefined(); + expect(keys[1]).not.toBe(keys[0]); + }); +}); + +describe('useContinueSession post-success failure containment', () => { + beforeEach(() => { + prepareSessionMutate.mockReset(); + remoteSpawnMock.mockReset(); + routerPush.mockClear(); + queryClientFetchQuery.mockReset(); + toastError.mockClear(); + invalidateAgentSessionQueriesMock.mockReset(); + hapticsMock.calls = 0; + hapticsMock.rejectWith = undefined; + destinationsRef.value = [CLOUD_DESTINATION]; + // eslint-disable-next-line typescript-eslint/promise-function-async -- conflicting require-await rule + queryClientFetchQuery.mockImplementation((options: { queryKey?: string[] }) => { + if (options.queryKey?.[0] === 'instances') { + return Promise.resolve({ instances: [] }); + } + return Promise.resolve({ repositories: [] }); + }); + }); + + it('still navigates and shows no create-failure toast when cache invalidation fails', async () => { + prepareSessionMutate.mockResolvedValueOnce({ + kiloSessionId: 'ses_12345678901234567890123456', + }); + invalidateAgentSessionQueriesMock.mockRejectedValueOnce(new Error('cache invalidation failed')); + const hook = runContinueSession({ organizationId: 'org-1' }); + + await hook.continueSession(FIELDS); + + // The cloud prepare succeeded; the cache failure must not block + // navigation and must not surface as a create failure. + expect(routerPush).toHaveBeenCalledTimes(1); + expect(toastError).not.toHaveBeenCalled(); + }); + + it('still navigates and shows no create-failure toast when haptics rejects', async () => { + prepareSessionMutate.mockResolvedValueOnce({ + kiloSessionId: 'ses_12345678901234567890123456', + }); + hapticsMock.rejectWith = new Error('haptics unavailable'); + const unhandledRejections: unknown[] = []; + const onUnhandledRejection = (reason: unknown) => { + unhandledRejections.push(reason); + }; + process.on('unhandledRejection', onUnhandledRejection); + + try { + const hook = runContinueSession({ organizationId: 'org-1' }); + await hook.continueSession(FIELDS); + // Give the runtime a turn to flag an unhandled rejection if the hook + // ever leaks the haptics promise's rejection. + await new Promise(resolve => { + setImmediate(resolve); + }); + + // The rejected haptics call is contained: no unhandled rejection, no + // create-failure toast, and the navigation still runs. + expect(hapticsMock.calls).toBe(1); + expect(routerPush).toHaveBeenCalledTimes(1); + expect(toastError).not.toHaveBeenCalled(); + expect(unhandledRejections).toEqual([]); + } finally { + process.off('unhandledRejection', onUnhandledRejection); + } + }); +}); + +describe('useContinueSession remote operationKey', () => { + beforeEach(() => { + prepareSessionMutate.mockReset(); + remoteSpawnMock.mockReset(); + routerPush.mockClear(); + queryClientFetchQuery.mockReset(); + toastError.mockClear(); + destinationsRef.value = [REMOTE_DESTINATION]; + // eslint-disable-next-line typescript-eslint/promise-function-async -- conflicting require-await rule + queryClientFetchQuery.mockImplementation((options: { queryKey?: string[] }) => { + if (options.queryKey?.[0] === 'instances') { + return Promise.resolve({ instances: [] }); + } + return Promise.resolve({ repositories: [] }); + }); + }); + + it('keeps the same remote operationKey across retryable spawn outcomes', async () => { + remoteSpawnMock.mockResolvedValue(retryableOutcome()); + const hook = runContinueSession({ organizationId: 'org-1' }); + + await hook.continueSession({ gitUrl: null, mode: 'code', model: '', variant: '' }); + await hook.continueSession({ gitUrl: null, mode: 'code', model: '', variant: '' }); + + const keys = usedRemoteKeys(); + expect(keys[0]).toBeDefined(); + expect(keys[1]).toBe(keys[0]); + // The operationKey rides the third spawn argument (dedupe mutationId). + expect(remoteSpawnMock.mock.calls[0]?.[0]).toBe('conn-1'); + expect(remoteSpawnMock.mock.calls[0]?.[2]).toMatchObject({ operationKey: expect.any(String) }); + }); + + it('rotates the remote operationKey after a ready spawn', async () => { + remoteSpawnMock + .mockResolvedValueOnce(retryableOutcome()) + .mockResolvedValueOnce(readyOutcome()) + .mockResolvedValueOnce(retryableOutcome()); + const hook = runContinueSession({ organizationId: 'org-1' }); + + await hook.continueSession({ gitUrl: null, mode: 'code', model: '', variant: '' }); + await hook.continueSession({ gitUrl: null, mode: 'code', model: '', variant: '' }); + await hook.continueSession({ gitUrl: null, mode: 'code', model: '', variant: '' }); + + const keys = usedRemoteKeys(); + // The ready attempt rides the key from the retryable attempt. + expect(keys[1]).toBe(keys[0]); + // The spawn after ready is a fresh intent with a fresh key. + expect(keys[2]).not.toBe(keys[0]); + }); + + it('rotates the remote operationKey after a typed non-retryable spawn rejection', async () => { + remoteSpawnMock + .mockResolvedValueOnce(nonRetryableOutcome()) + .mockResolvedValueOnce(retryableOutcome()); + const hook = runContinueSession({ organizationId: 'org-1' }); + + await hook.continueSession({ gitUrl: null, mode: 'code', model: '', variant: '' }); + await hook.continueSession({ gitUrl: null, mode: 'code', model: '', variant: '' }); + + const keys = usedRemoteKeys(); + expect(keys[0]).toBeDefined(); + expect(keys[1]).not.toBe(keys[0]); + }); +}); + +describe('useContinueSession key separation', () => { + it('never shares a key between cloud prepares and remote spawns', async () => { + prepareSessionMutate.mockResolvedValueOnce({ kiloSessionId: 'ses_12345678901234567890123456' }); + remoteSpawnMock.mockResolvedValueOnce(retryableOutcome()); + // eslint-disable-next-line typescript-eslint/promise-function-async -- conflicting require-await rule + queryClientFetchQuery.mockImplementation((options: { queryKey?: string[] }) => { + if (options.queryKey?.[0] === 'instances') { + return Promise.resolve({ instances: [] }); + } + return Promise.resolve({ repositories: [] }); + }); + const hook = runContinueSession({ organizationId: 'org-1' }); + + destinationsRef.value = [CLOUD_DESTINATION]; + await hook.continueSession(FIELDS); + + destinationsRef.value = [REMOTE_DESTINATION]; + await hook.continueSession({ gitUrl: null, mode: 'code', model: '', variant: '' }); + + const cloudKey = usedCloudKeys()[0]; + const remoteKey = usedRemoteKeys()[0]; + expect(cloudKey).toBeDefined(); + expect(remoteKey).toBeDefined(); + expect(remoteKey).not.toBe(cloudKey); + }); +}); diff --git a/apps/mobile/src/components/agents/use-continue-session.ts b/apps/mobile/src/components/agents/use-continue-session.ts index 1539579383..4bcf1f1a19 100644 --- a/apps/mobile/src/components/agents/use-continue-session.ts +++ b/apps/mobile/src/components/agents/use-continue-session.ts @@ -4,8 +4,6 @@ import { type Href, useRouter } from 'expo-router'; import { useQueryClient } from '@tanstack/react-query'; import { useStore } from 'jotai'; import { toast } from 'sonner-native'; -import { generateMessageId } from '@kilocode/cloud-agent-sdk/message-id'; -import * as Haptics from 'expo-haptics'; import { listInstanceModels } from '@kilocode/cloud-agent-sdk/instance-model-catalog'; import { @@ -15,29 +13,25 @@ import { resolveContinuationDestinations, } from '@/components/agents/continuation-seed'; import { setContinuePickerBridge } from '@/components/agents/continue-picker-bridge'; -import { normalizeAgentMode } from '@/components/agents/mode-options'; +import { getSpawnedAgentSessionPath } from '@/components/agents/session-detail-routes'; +import { useContinueCloudCreate } from '@/components/agents/use-continue-cloud-create'; import { appendNewSessionPrefill, buildContinuePrefillParams, } from '@/components/agents/new-session-prefill'; import { getNewAgentSessionPath } from '@/components/agents/session-list-routes'; -import { - getAgentSessionPath, - getSpawnedAgentSessionPath, -} from '@/components/agents/session-detail-routes'; import { type useSessionManager } from '@/components/agents/session-provider'; import { useUserWebConnection } from '@/components/agents/user-web-connection-provider'; import { type SessionModelOption } from '@/lib/hooks/use-session-model-options'; import { putSharePayload } from '@/lib/share-payload'; import { appendShareParams } from '@/lib/share-navigation'; import { useRemoteInstanceSpawn } from '@/lib/hooks/use-remote-instance-spawn'; +import { useHoistedOperationKey } from '@/lib/operation-key'; import { REMOTE_SPAWN_NON_RETRYABLE_TOAST, REMOTE_SPAWN_RETRYABLE_TOAST, } from '@/lib/remote-submit-outcome'; -import { captureEvent, SESSION_CREATED_EVENT } from '@/lib/analytics/posthog'; -import { invalidateAgentSessionQueries } from '@/lib/agent-session-cache'; -import { trpcClient, useTRPC } from '@/lib/trpc'; +import { useTRPC } from '@/lib/trpc'; type RouterOutputs = inferRouterOutputs; type RepositoriesResult = @@ -67,33 +61,11 @@ export function useContinueSession(args: { const { spawn } = useRemoteInstanceSpawn(args.organizationId ?? null); const [isContinuing, setIsContinuing] = useState(false); const busyRef = useRef(false); - - const runCloudCreate = useCallback( - async (seed: string, dest: { repo: string; model: string; variant: string }, mode: string) => { - const initialMessageId = generateMessageId(); - const baseInput = { - prompt: seed, - initialMessageId, - mode: normalizeAgentMode(mode), - model: dest.model, - variant: dest.variant || undefined, - githubRepo: dest.repo, - autoCommit: true, - autoInitiate: true, - }; - const result = args.organizationId - ? await trpcClient.organizations.cloudAgentNext.prepareSession.mutate({ - ...baseInput, - organizationId: args.organizationId, - }) - : await trpcClient.cloudAgentNext.prepareSession.mutate(baseInput); - captureEvent(SESSION_CREATED_EVENT, { surface: 'cloud-agent' }); - await invalidateAgentSessionQueries(queryClient, trpc); - void Haptics.notificationAsync(Haptics.NotificationFeedbackType.Success); - router.push(getAgentSessionPath(result.kiloSessionId, args.organizationId)); - }, - [args.organizationId, queryClient, router, trpc] - ); + // P1-A-08b: cloud prepares and remote spawns are different intents, so each + // destination family holds its own hoisted `operationKey`. The cloud key + // lives inside `useContinueCloudCreate`. + const remoteOperationKey = useHoistedOperationKey(); + const runCloudCreate = useContinueCloudCreate(args.organizationId); const execute = useCallback( async ( @@ -112,6 +84,16 @@ export function useContinueSession(args: { } return; } + const remoteOperationKeyValue = remoteOperationKey.getKey( + JSON.stringify({ + connectionId: dest.instance.connectionId, + seed, + model: fields.model, + variant: fields.variant, + mode: fields.mode, + organizationId: args.organizationId ?? null, + }) + ); const catalogResult = await listInstanceModels(connection, dest.instance.connectionId); const outcome = await spawn( dest.instance.connectionId, @@ -122,9 +104,12 @@ export function useContinueSession(args: { options: args.models, catalogResult, organizationId: args.organizationId, - }) + }), + { operationKey: remoteOperationKeyValue } ); if (outcome.status === 'ready') { + // The spawn settled; the next submit is a fresh intent. + remoteOperationKey.rotateKey(); const shareId = putSharePayload({ text: seed, files: [], failedFiles: [] }); router.push( appendShareParams( @@ -140,12 +125,25 @@ export function useContinueSession(args: { ? REMOTE_SPAWN_RETRYABLE_TOAST : REMOTE_SPAWN_NON_RETRYABLE_TOAST ); + // A non-retryable rejection ends the intent; a retryable outcome keeps + // the key so a same-key retry dedupes on the relay. + if (outcome.status === 'nonRetryable') { + remoteOperationKey.rotateKey(); + } } finally { busyRef.current = false; setIsContinuing(false); } }, - [args.organizationId, args.models, connection, router, runCloudCreate, spawn] + [ + args.organizationId, + args.models, + connection, + router, + runCloudCreate, + spawn, + remoteOperationKey, + ] ); const fallback = useCallback( diff --git a/apps/mobile/src/components/agents/use-new-session-creator.test.ts b/apps/mobile/src/components/agents/use-new-session-creator.test.ts index 37fa36103e..4e3c2d9833 100644 --- a/apps/mobile/src/components/agents/use-new-session-creator.test.ts +++ b/apps/mobile/src/components/agents/use-new-session-creator.test.ts @@ -1,7 +1,7 @@ /* eslint-disable typescript-eslint/no-deprecated -- react-test-renderer is the DOM-free renderer used to mount React/RN trees under vitest (node env, no jsdom); see src/lib/persist/cache-persistence-mount.test.ts */ /* eslint-disable require-await, @typescript-eslint/require-await -- the fake mutate factories settle without await because they resolve immediately */ -/* eslint-disable max-lines -- creator and generation-fenced draft-load suites share this file */ -import { createElement } from 'react'; +/* eslint-disable max-lines -- the creator, operation-key, and generation-fenced draft-load suites share one mock harness in this file */ +import * as React from 'react'; import TestRenderer, { act } from 'react-test-renderer'; import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; @@ -10,17 +10,23 @@ import { useNewSessionCreator } from './use-new-session-creator'; import { clearDraft, flushDraft, loadDraft } from '@/lib/persist/drafts'; import { useFencedDraftLoad, useRemoteSpawnDraftCleanup } from '@/lib/persist/use-draft-load'; -const routerPushMock = vi.hoisted(() => vi.fn()); -const navigationDispatchMock = vi.hoisted(() => vi.fn()); -const mutateMock = vi.hoisted(() => vi.fn()); -const invalidateMock = vi.hoisted(() => vi.fn(async () => undefined)); +const prepareSessionMutate = vi.hoisted(() => vi.fn()); +const routerPush = vi.hoisted(() => vi.fn()); +const navigationDispatch = vi.hoisted(() => vi.fn()); +const toastError = vi.hoisted(() => vi.fn()); const captureEventMock = vi.hoisted(() => vi.fn()); -const toastErrorMock = vi.hoisted(() => vi.fn()); -const hapticsNotificationMock = vi.hoisted(() => vi.fn(async () => undefined)); +const invalidateAgentSessionQueries = vi.hoisted(() => vi.fn(async () => undefined)); +// Not a `vi.fn()`: vitest attaches its own rejection handler to any promise a +// mock returns, which would mask a leaked haptics rejection. A plain module +// export returning a real promise keeps `unhandledRejection` detection honest. +const hapticsMock = vi.hoisted(() => ({ + calls: 0, + rejectWith: undefined as Error | undefined, +})); vi.mock('expo-router', () => ({ - useRouter: () => ({ push: routerPushMock }), - useNavigation: () => ({ dispatch: navigationDispatchMock }), + useRouter: () => ({ push: routerPush }), + useNavigation: () => ({ dispatch: navigationDispatch }), })); vi.mock('@tanstack/react-query', () => ({ @@ -28,16 +34,22 @@ vi.mock('@tanstack/react-query', () => ({ })); vi.mock('expo-haptics', () => ({ - notificationAsync: hapticsNotificationMock, + notificationAsync: async (): Promise => { + hapticsMock.calls += 1; + await Promise.resolve(); + if (hapticsMock.rejectWith !== undefined) { + throw hapticsMock.rejectWith; + } + }, NotificationFeedbackType: { Success: 'success' }, })); vi.mock('sonner-native', () => ({ - toast: { error: toastErrorMock }, + toast: { error: toastError }, })); vi.mock('@/lib/agent-session-cache', () => ({ - invalidateAgentSessionQueries: invalidateMock, + invalidateAgentSessionQueries, })); vi.mock('@/lib/analytics/posthog', () => ({ @@ -47,8 +59,8 @@ vi.mock('@/lib/analytics/posthog', () => ({ vi.mock('@/lib/trpc', () => ({ trpcClient: { - cloudAgentNext: { prepareSession: { mutate: mutateMock } }, - organizations: { cloudAgentNext: { prepareSession: { mutate: mutateMock } } }, + cloudAgentNext: { prepareSession: { mutate: prepareSessionMutate } }, + organizations: { cloudAgentNext: { prepareSession: { mutate: prepareSessionMutate } } }, }, useTRPC: () => ({ mockTrpc: true }), })); @@ -68,9 +80,52 @@ vi.mock('@/lib/persist/drafts', () => ({ clearDraft: vi.fn(async () => undefined), })); +// The real classifier lives in mobile-session-manager (covered by its own +// suite); this test mirrors its decision so hook-level retries are exercised +// for every retryable shape: no code (transport), CONFLICT + +// `creation_in_progress`, and the transient 5xx-class codes. +vi.mock('@/components/agents/mobile-session-manager', () => { + const TRANSIENT_CODES = new Set([ + 'INTERNAL_SERVER_ERROR', + 'BAD_GATEWAY', + 'SERVICE_UNAVAILABLE', + 'GATEWAY_TIMEOUT', + 'TIMEOUT', + 'TOO_MANY_REQUESTS', + ]); + return { + isCloudPrepareRetryableError: (error: unknown) => { + const record = error as { data?: { code?: string }; code?: string; message?: string }; + const code = record.data?.code ?? record.code; + if (code === undefined) { + return true; + } + if (code === 'CONFLICT') { + return record.message === 'creation_in_progress'; + } + return TRANSIENT_CODES.has(code); + }, + }; +}); + +vi.mock('expo-crypto', () => { + let n = 0; + return { + randomUUID: () => { + n += 1; + return `op-key-${n}`; + }, + }; +}); + type CreatorInput = Parameters[0]; type CreatorResult = ReturnType; +// Simulated attachment wire payload (`{path, files}`). Each test sets this +// before a submit; the fake `toWirePayload` below reads it at call time so a +// test can change attachments between two submits. +let attachmentsWire: { path: string; files: string[] } | null = null; + const FAKE_ATTACHMENTS: CreatorInput['attachments'] = { attachments: [], addCandidates: vi.fn(async () => undefined), @@ -111,7 +166,7 @@ function Harness({ function mountCreator(input: CreatorInput) { const resultRef: { current: CreatorResult | null } = { current: null }; act(() => { - TestRenderer.create(createElement(Harness, { input, resultRef })); + TestRenderer.create(React.createElement(Harness, { input, resultRef })); }); return resultRef; } @@ -158,9 +213,11 @@ async function flushMicrotasks(): Promise { beforeEach(() => { vi.clearAllMocks(); - mutateMock.mockResolvedValue({ kiloSessionId: 'sess-1' }); - invalidateMock.mockResolvedValue(undefined); - hapticsNotificationMock.mockResolvedValue(undefined); + prepareSessionMutate.mockResolvedValue({ kiloSessionId: 'sess-1' }); + invalidateAgentSessionQueries.mockResolvedValue(undefined); + hapticsMock.calls = 0; + hapticsMock.rejectWith = undefined; + attachmentsWire = null; vi.stubGlobal('requestAnimationFrame', requestAnimationFrameStub); }); @@ -168,6 +225,331 @@ afterEach(() => { vi.unstubAllGlobals(); }); +// P1-A-08b: `useNewSessionCreator` must attach one stable `operationKey` per +// submit intent to `prepareSession`, keep it across retryable failures +// (incl. `creation_in_progress`), and rotate it on success or a typed +// non-retryable rejection. Run through a fake React dispatcher so the hook's +// own refs/callbacks are exercised without mounting React Native. + +function creationInProgressError(): Error { + return Object.assign(new Error('creation_in_progress'), { data: { code: 'CONFLICT' } }); +} + +function badRequestError(): Error { + return Object.assign(new Error('session_creation_failed'), { data: { code: 'BAD_REQUEST' } }); +} + +type ReactInternals = { + __CLIENT_INTERNALS_DO_NOT_USE_OR_WARN_USERS_THEY_CANNOT_UPGRADE: { + H: unknown; + }; +}; + +type HookDispatcher = { + useCallback: (callback: T, _deps?: unknown) => T; + useRef: (initial: T) => { current: T }; +}; + +function runCreator(args: { + mode?: string; + model?: string; + variant?: string; + organizationId?: string; + selectedRepo?: string; +}): CreatorResult { + const reactInternals = React as typeof React & ReactInternals; + const refs: { current: unknown }[] = []; + let hookIndex = 0; + let refIndex = 0; + + const dispatcher: HookDispatcher = { + useCallback: hookCallback => { + hookIndex += 1; + return hookCallback; + }, + useRef: initial => { + const index = refIndex; + refIndex += 1; + refs[index] ??= { current: initial }; + return refs[index] as { current: typeof initial }; + }, + }; + + const previousDispatcher = + reactInternals.__CLIENT_INTERNALS_DO_NOT_USE_OR_WARN_USERS_THEY_CANNOT_UPGRADE.H; + reactInternals.__CLIENT_INTERNALS_DO_NOT_USE_OR_WARN_USERS_THEY_CANNOT_UPGRADE.H = dispatcher; + try { + // eslint-disable-next-line react-hooks/rules-of-hooks -- fake dispatcher drives the hook in a plain vitest run + return useNewSessionCreator({ + // eslint-disable-next-line @typescript-eslint/consistent-type-assertions -- attachment fake shape, never read by the create path + attachments: { + attachments: [], + toWirePayload: () => attachmentsWire, + } as never, + mode: (args.mode ?? 'code') as never, + model: args.model ?? 'model-1', + organizationId: args.organizationId, + selectedRepo: args.selectedRepo ?? 'owner/repo', + // eslint-disable-next-line no-empty-function -- no-op state setter + setIsCreating: () => {}, + variant: args.variant ?? 'v1', + }); + } finally { + reactInternals.__CLIENT_INTERNALS_DO_NOT_USE_OR_WARN_USERS_THEY_CANNOT_UPGRADE.H = + previousDispatcher; + } +} + +function usedOperationKeys(): (string | undefined)[] { + return prepareSessionMutate.mock.calls.map( + call => (call[0] as { operationKey?: string }).operationKey + ); +} + +function sessionResult(): { kiloSessionId: string; cloudAgentSessionId: string } { + return { kiloSessionId: 'ses_12345678901234567890123456', cloudAgentSessionId: 'c-1' }; +} + +describe('useNewSessionCreator operationKey', () => { + beforeEach(() => { + prepareSessionMutate.mockReset(); + invalidateAgentSessionQueries.mockReset(); + invalidateAgentSessionQueries.mockResolvedValue(undefined); + }); + + it('keeps the same operationKey across retryable creation_in_progress failures', async () => { + prepareSessionMutate + .mockRejectedValueOnce(creationInProgressError()) + .mockRejectedValueOnce(creationInProgressError()); + const creator = runCreator({}); + + creator.promptRef.current = 'hello'; + await creator.createSessionFromDraft(); + await creator.createSessionFromDraft(); + + const keys = usedOperationKeys(); + expect(keys[0]).toBeDefined(); + expect(keys[1]).toBe(keys[0]); + expect(prepareSessionMutate.mock.calls[0]?.[0]).toMatchObject({ + prompt: 'hello', + autoInitiate: true, + operationKey: expect.any(String), + }); + }); + + it('rotates the operationKey after a success so the next submit is a fresh intent', async () => { + prepareSessionMutate + .mockRejectedValueOnce(creationInProgressError()) + .mockResolvedValueOnce({ + kiloSessionId: 'ses_12345678901234567890123456', + cloudAgentSessionId: 'c-1', + }) + .mockRejectedValueOnce(creationInProgressError()); + const creator = runCreator({}); + + creator.promptRef.current = 'hello'; + await creator.createSessionFromDraft(); + await creator.createSessionFromDraft(); + await creator.createSessionFromDraft(); + + const keys = usedOperationKeys(); + // The successful retry rides the same key as the retryable attempt. + expect(keys[1]).toBe(keys[0]); + // The submit after success is a fresh intent with a fresh key. + expect(keys[2]).not.toBe(keys[0]); + }); + + // The transport-failure and transient-5xx branches of the retryability + // predicate are covered in mobile-session-manager.test.ts; the hook only + // needs one retryable and one terminal case. + it('rotates the operationKey after a typed non-retryable rejection', async () => { + prepareSessionMutate + .mockRejectedValueOnce(badRequestError()) + .mockRejectedValueOnce(creationInProgressError()); + const creator = runCreator({}); + + creator.promptRef.current = 'hello'; + await creator.createSessionFromDraft(); + await creator.createSessionFromDraft(); + + const keys = usedOperationKeys(); + expect(keys[0]).toBeDefined(); + expect(keys[1]).not.toBe(keys[0]); + }); + + it('treats a changed draft as a new intent with a new key', async () => { + prepareSessionMutate + .mockRejectedValueOnce(creationInProgressError()) + .mockRejectedValueOnce(creationInProgressError()); + const creator = runCreator({}); + + creator.promptRef.current = 'hello'; + await creator.createSessionFromDraft(); + creator.promptRef.current = 'hello, changed'; + await creator.createSessionFromDraft(); + + const keys = usedOperationKeys(); + expect(keys[1]).not.toBe(keys[0]); + }); + + it('keeps the same operationKey across retryable failures when attachments are unchanged', async () => { + prepareSessionMutate + .mockRejectedValueOnce(creationInProgressError()) + .mockRejectedValueOnce(creationInProgressError()); + const creator = runCreator({}); + attachmentsWire = { path: 'p-1', files: ['a-1'] }; + + creator.promptRef.current = 'hello'; + await creator.createSessionFromDraft(); + await creator.createSessionFromDraft(); + + const keys = usedOperationKeys(); + expect(keys[0]).toBeDefined(); + expect(keys[1]).toBe(keys[0]); + // The wire payload the fingerprint read is the payload the create body + // carries, so the fingerprint and the mutation agree on the intent. + expect(prepareSessionMutate.mock.calls[0]?.[0]).toMatchObject({ + attachments: { path: 'p-1', files: ['a-1'] }, + }); + }); + + it('treats changed attachments as a new intent with a new key', async () => { + prepareSessionMutate + .mockRejectedValueOnce(creationInProgressError()) + .mockRejectedValueOnce(creationInProgressError()); + const creator = runCreator({}); + attachmentsWire = { path: 'p-1', files: ['a-1'] }; + + creator.promptRef.current = 'hello'; + await creator.createSessionFromDraft(); + // The user swapped the attachment; the next submit is a fresh intent + // with a fresh key, otherwise the same-key retry would replay the + // previous intent's ledger result instead of creating with the new file. + attachmentsWire = { path: 'p-1', files: ['a-2'] }; + await creator.createSessionFromDraft(); + + const keys = usedOperationKeys(); + expect(keys[1]).not.toBe(keys[0]); + }); + + it('does not treat a post-success cache failure as a create failure and rotates the key', async () => { + prepareSessionMutate.mockResolvedValue(sessionResult()); + invalidateAgentSessionQueries.mockRejectedValueOnce(new Error('cache invalidation failed')); + const creator = runCreator({}); + + creator.promptRef.current = 'hello'; + await creator.createSessionFromDraft(); + // The create-failure toast path must not run for a post-success failure. + expect(toastError).not.toHaveBeenCalled(); + + // The next submit is a fresh intent with a fresh key, not a retry of the + // successful operation key. + creator.promptRef.current = 'hello'; + await creator.createSessionFromDraft(); + + const keys = usedOperationKeys(); + expect(keys[1]).toBeDefined(); + expect(keys[1]).not.toBe(keys[0]); + }); + + it('does not treat a post-success navigation failure as a create failure and rotates the key', async () => { + prepareSessionMutate.mockResolvedValue(sessionResult()); + routerPush.mockImplementationOnce(() => { + throw new Error('navigation failed'); + }); + const creator = runCreator({}); + + creator.promptRef.current = 'hello'; + await creator.createSessionFromDraft(); + // The create-failure toast path must not run for a post-success failure. + expect(toastError).not.toHaveBeenCalled(); + + // The next submit is a fresh intent with a fresh key, not a retry of the + // successful operation key. + creator.promptRef.current = 'hello'; + await creator.createSessionFromDraft(); + + const keys = usedOperationKeys(); + expect(keys[1]).toBeDefined(); + expect(keys[1]).not.toBe(keys[0]); + }); + + it('does not treat a rejected haptics call as a create failure and rotates the key', async () => { + prepareSessionMutate.mockResolvedValue(sessionResult()); + hapticsMock.rejectWith = new Error('haptics failed'); + const unhandledRejections: unknown[] = []; + const onUnhandledRejection = (reason: unknown) => { + unhandledRejections.push(reason); + }; + process.on('unhandledRejection', onUnhandledRejection); + + try { + const creator = runCreator({}); + creator.promptRef.current = 'hello'; + await creator.createSessionFromDraft(); + // Give the runtime a turn to flag an unhandled rejection if the hook + // ever leaks the haptics promise's rejection. + await new Promise(resolve => { + setImmediate(resolve); + }); + expect(hapticsMock.calls).toBe(1); + // A rejected haptics call must be contained: no unhandled rejection and + // no create-failure toast. + expect(unhandledRejections).toEqual([]); + expect(toastError).not.toHaveBeenCalled(); + + // The next submit is a fresh intent with a fresh key, not a retry of the + // successful operation key. + creator.promptRef.current = 'hello'; + await creator.createSessionFromDraft(); + + const keys = usedOperationKeys(); + expect(keys[1]).toBeDefined(); + expect(keys[1]).not.toBe(keys[0]); + } finally { + process.off('unhandledRejection', onUnhandledRejection); + } + }); + + it('contains a deferred stack-cleanup failure after a success and rotates the key', async () => { + prepareSessionMutate.mockResolvedValue(sessionResult()); + navigationDispatch.mockImplementationOnce(() => { + throw new Error('stack cleanup failed'); + }); + // The test environment has no requestAnimationFrame; capture the deferred + // callback so it can be run after the submit settles, like the real frame + // boundary does. + const scheduledFrames: (() => void)[] = []; + // eslint-disable-next-line promise/prefer-await-to-callbacks -- the arrow is a global stub, not an async callback + vi.stubGlobal('requestAnimationFrame', (callback: () => void) => { + scheduledFrames.push(callback); + return 1; + }); + + const creator = runCreator({}); + creator.promptRef.current = 'hello'; + await creator.createSessionFromDraft(); + expect(toastError).not.toHaveBeenCalled(); + + // The deferred stack cleanup runs after the submit returned; a failure + // there must be contained instead of surfacing as an uncaught exception. + expect(scheduledFrames).toHaveLength(1); + expect(() => { + scheduledFrames[0]?.(); + }).not.toThrow(); + expect(toastError).not.toHaveBeenCalled(); + + // The next submit is a fresh intent with a fresh key, not a retry of the + // successful operation key. + creator.promptRef.current = 'hello'; + await creator.createSessionFromDraft(); + + const keys = usedOperationKeys(); + expect(keys[1]).toBeDefined(); + expect(keys[1]).not.toBe(keys[0]); + }); +}); + describe('useNewSessionCreator onCreated', () => { it('fires onCreated once on success, before navigating', async () => { const onCreated = vi.fn(() => undefined); @@ -180,15 +562,32 @@ describe('useNewSessionCreator onCreated', () => { }); expect(onCreated).toHaveBeenCalledTimes(1); - expect(mutateMock).toHaveBeenCalledTimes(1); - expect(routerPushMock).toHaveBeenCalledWith(expect.stringContaining('agent-chat/sess-1')); + expect(prepareSessionMutate).toHaveBeenCalledTimes(1); + expect(routerPush).toHaveBeenCalledWith(expect.stringContaining('agent-chat/sess-1')); expect(captureEventMock).toHaveBeenCalledWith('session_created', expect.anything()); - expect(invalidateMock).toHaveBeenCalledTimes(1); - expect(toastErrorMock).not.toHaveBeenCalled(); + expect(invalidateAgentSessionQueries).toHaveBeenCalledTimes(1); + expect(toastError).not.toHaveBeenCalled(); + }); + + it('contains a throwing onCreated callback and still navigates', async () => { + const onCreated = vi.fn(() => { + throw new Error('host callback failed'); + }); + const resultRef = mountCreator(createInput({ organizationId: 'org-1', onCreated })); + const { createSessionFromDraft, promptRef } = requireResult(resultRef); + promptRef.current = 'Hello agent'; + + await act(async () => { + await createSessionFromDraft(); + }); + + expect(onCreated).toHaveBeenCalledTimes(1); + expect(routerPush).toHaveBeenCalledWith(expect.stringContaining('agent-chat/sess-1')); + expect(toastError).not.toHaveBeenCalled(); }); it('never fires onCreated when prepareSession rejects, and preserves the draft', async () => { - mutateMock.mockRejectedValueOnce(new Error('boom')); + prepareSessionMutate.mockRejectedValueOnce(new Error('boom')); const onCreated = vi.fn(() => undefined); const resultRef = mountCreator(createInput({ organizationId: 'org-1', onCreated })); const { createSessionFromDraft, promptRef } = requireResult(resultRef); @@ -199,8 +598,8 @@ describe('useNewSessionCreator onCreated', () => { }); expect(onCreated).not.toHaveBeenCalled(); - expect(routerPushMock).not.toHaveBeenCalled(); - expect(toastErrorMock).toHaveBeenCalledWith('boom'); + expect(routerPush).not.toHaveBeenCalled(); + expect(toastError).toHaveBeenCalledWith('boom'); }); it('does not prepare (and never fires onCreated) when the draft is empty', async () => { @@ -213,9 +612,9 @@ describe('useNewSessionCreator onCreated', () => { await createSessionFromDraft(); }); - expect(mutateMock).not.toHaveBeenCalled(); + expect(prepareSessionMutate).not.toHaveBeenCalled(); expect(onCreated).not.toHaveBeenCalled(); - expect(routerPushMock).not.toHaveBeenCalled(); + expect(routerPush).not.toHaveBeenCalled(); }); }); @@ -231,10 +630,10 @@ describe('restored new-session submit', () => { await createSessionFromDraft(); }); - expect(mutateMock).toHaveBeenCalledWith( + expect(prepareSessionMutate).toHaveBeenCalledWith( expect.objectContaining({ prompt: 'Restored draft prompt' }) ); - expect(routerPushMock).toHaveBeenCalledWith(expect.stringContaining('agent-chat/sess-1')); + expect(routerPush).toHaveBeenCalledWith(expect.stringContaining('agent-chat/sess-1')); }); }); @@ -290,7 +689,7 @@ describe('useFencedDraftLoad generation fencing', () => { let renderer: TestRenderer.ReactTestRenderer | undefined = undefined; act(() => { renderer = TestRenderer.create( - createElement(FencedDraftHarness, { + React.createElement(FencedDraftHarness, { ...first, isIdentityLoading: false, onRender: state => { @@ -303,7 +702,7 @@ describe('useFencedDraftLoad generation fencing', () => { // The generation changes while the first load is still in flight. act(() => { renderer?.update( - createElement(FencedDraftHarness, { + React.createElement(FencedDraftHarness, { ...second, isIdentityLoading: false, onRender: state => { @@ -343,7 +742,7 @@ describe('useFencedDraftLoad generation fencing', () => { let renderer: TestRenderer.ReactTestRenderer | undefined = undefined; act(() => { renderer = TestRenderer.create( - createElement(FencedDraftHarness, { + React.createElement(FencedDraftHarness, { userId: 'u1', isIdentityLoading: false, entityKey: 'agent-composer:sess-1', @@ -387,7 +786,7 @@ function mountRemoteSpawnDraftCleanup(userId: string | undefined): { let renderer: TestRenderer.ReactTestRenderer | undefined = undefined; act(() => { renderer = TestRenderer.create( - createElement(RemoteSpawnDraftCleanupHarness, { userId, resultRef }) + React.createElement(RemoteSpawnDraftCleanupHarness, { userId, resultRef }) ); }); return { renderer, resultRef }; diff --git a/apps/mobile/src/components/agents/use-new-session-creator.ts b/apps/mobile/src/components/agents/use-new-session-creator.ts index 96e333b231..009a43b306 100644 --- a/apps/mobile/src/components/agents/use-new-session-creator.ts +++ b/apps/mobile/src/components/agents/use-new-session-creator.ts @@ -7,8 +7,10 @@ import { toast } from 'sonner-native'; import { type AgentMode } from '@/components/agents/mode-selector'; import { resolveNewSessionPromptForCreate } from '@/components/agents/new-session-prompt-state'; +import { isCloudPrepareRetryableError } from '@/components/agents/mobile-session-manager'; import { invalidateAgentSessionQueries } from '@/lib/agent-session-cache'; import { captureEvent, SESSION_CREATED_EVENT } from '@/lib/analytics/posthog'; +import { useHoistedOperationKey } from '@/lib/operation-key'; import { type AgentAttachmentWire, type useAgentAttachmentUpload, @@ -54,6 +56,9 @@ export function useNewSessionCreator({ const queryClient = useQueryClient(); const trpc = useTRPC(); const promptRef = useRef(''); + // P1-A-08b: one `operationKey` per submit intent, so a retry of the same + // intent dedupes on the ledger instead of spawning a second session. + const { getKey, rotateKey } = useHoistedOperationKey(); const createSessionFromDraft = useCallback(async () => { // Read the live, post-settlement draft (see `settleVoiceInputBeforeSubmit` @@ -74,6 +79,20 @@ export function useNewSessionCreator({ setIsCreating(true); + // Computed once and reused for both the fingerprint and the create body, so + // the two cannot disagree and a swapped attachment set is a fresh intent. + const attachmentWire = attachments.toWirePayload(); + const intentFingerprint = JSON.stringify({ + prompt, + mode, + model, + variant: variant || undefined, + repo: selectedRepo, + organizationId: organizationId ?? null, + attachments: attachmentWire ?? null, + }); + const operationKey = getKey(intentFingerprint); + try { const initialMessageId = generateMessageId(); const baseInput: { @@ -85,6 +104,7 @@ export function useNewSessionCreator({ githubRepo: string; autoCommit: boolean; autoInitiate: boolean; + operationKey: string; attachments?: AgentAttachmentWire; } = { prompt, @@ -95,10 +115,10 @@ export function useNewSessionCreator({ githubRepo: selectedRepo, autoCommit: true, autoInitiate: true, + operationKey, }; - const wireAttachments = attachments.toWirePayload(); - if (wireAttachments) { - baseInput.attachments = wireAttachments; + if (attachmentWire) { + baseInput.attachments = attachmentWire; } const result = organizationId @@ -108,29 +128,66 @@ export function useNewSessionCreator({ }) : await trpcClient.cloudAgentNext.prepareSession.mutate(baseInput); - captureEvent(SESSION_CREATED_EVENT, { surface: 'cloud-agent' }); - await invalidateAgentSessionQueries(queryClient, trpc); - // Signal the host (e.g. clear the new-session draft) before navigating, - // so the draft is gone by the time the route unmounts and can never be - // flushed back by an unmount write. - onCreated?.(); - void Haptics.notificationAsync(Haptics.NotificationFeedbackType.Success); - const path = organizationId - ? `/(app)/agent-chat/${result.kiloSessionId}?organizationId=${organizationId}` - : `/(app)/agent-chat/${result.kiloSessionId}`; - router.push(path as Href); - requestAnimationFrame(() => { - navigation.dispatch(state => { - const routes = state.routes.filter((r: { name: string }) => r.name !== 'agent-chat/new'); - return { - type: 'RESET' as const, - payload: { ...state, routes, index: routes.length - 1 }, - }; + // Rotate before the post-success work so a UI failure cannot keep the + // successful key for a retry. + rotateKey(); + + // The cloud session already exists, so no post-success UI failure may + // report the create as failed or invite a duplicate retry. + try { + // Contained together so neither can skip the host signal below. + try { + captureEvent(SESSION_CREATED_EVENT, { surface: 'cloud-agent' }); + await invalidateAgentSessionQueries(queryClient, trpc); + } catch { + // Analytics and cache invalidation are cosmetic; stay silent. + } + // Signal the host (e.g. clear the new-session draft) before navigating, + // so the draft is gone by the time the route unmounts and can never be + // flushed back by an unmount write. + try { + onCreated?.(); + } catch { + // The session exists; a host callback failure must not skip navigation. + } + // Contained on its own so a rejected haptics call still navigates. + try { + await Haptics.notificationAsync(Haptics.NotificationFeedbackType.Success); + } catch { + // A failed haptics call is cosmetic; stay silent and navigate. + } + const path = organizationId + ? `/(app)/agent-chat/${result.kiloSessionId}?organizationId=${organizationId}` + : `/(app)/agent-chat/${result.kiloSessionId}`; + router.push(path as Href); + requestAnimationFrame(() => { + // Runs after the surrounding try returned, so a failure here would + // escape uncaught. The stack cleanup is cosmetic; contain it. + try { + navigation.dispatch(state => { + const routes = state.routes.filter( + (r: { name: string }) => r.name !== 'agent-chat/new' + ); + return { + type: 'RESET' as const, + payload: { ...state, routes, index: routes.length - 1 }, + }; + }); + } catch { + // The session already exists; stay silent. + } }); - }); + } catch { + // Stay silent: no create-failure toast, no duplicate-create retry. + } } catch (error) { + // Only `prepareSession` errors reach here; UI failures are swallowed. const message = error instanceof Error ? error.message : 'Failed to create session'; toast.error(message); + // A typed terminal rejection ends the intent; a retryable one keeps the key. + if (!isCloudPrepareRetryableError(error)) { + rotateKey(); + } } finally { setIsCreating(false); } @@ -146,6 +203,8 @@ export function useNewSessionCreator({ navigation, attachments, setIsCreating, + getKey, + rotateKey, onCreated, ]); diff --git a/apps/mobile/src/components/agents/use-remote-spawn-dispatch.test.ts b/apps/mobile/src/components/agents/use-remote-spawn-dispatch.test.ts index 87eae61f79..7e992a9f8f 100644 --- a/apps/mobile/src/components/agents/use-remote-spawn-dispatch.test.ts +++ b/apps/mobile/src/components/agents/use-remote-spawn-dispatch.test.ts @@ -219,6 +219,7 @@ describe('useRemoteSpawnDispatch spawn input chain', () => { model: { providerID: 'anthropic', modelID: 'claude-x', variant: 'high' }, orgId: 'org-xyz', }, + { operationKey: expect.any(String) }, ]); }); @@ -227,7 +228,11 @@ describe('useRemoteSpawnDispatch spawn input chain', () => { organizationId: 'org-xyz', }); - expect(await captureSpawnCall(onStart)).toEqual(['conn-abc', { orgId: 'org-xyz' }]); + expect(await captureSpawnCall(onStart)).toEqual([ + 'conn-abc', + { orgId: 'org-xyz' }, + { operationKey: expect.any(String) }, + ]); }); it('explicit mode and selection reach the spawn input', async () => { @@ -240,6 +245,7 @@ describe('useRemoteSpawnDispatch spawn input chain', () => { expect(await captureSpawnCall(onStart)).toEqual([ 'conn-abc', { agent: 'code', model: { providerID: 'anthropic', modelID: 'claude-sonnet-4' } }, + { operationKey: expect.any(String) }, ]); }); @@ -266,6 +272,7 @@ describe('useRemoteSpawnDispatch spawn input chain', () => { agent: 'code', model: { providerID: 'kilo', modelID: 'kilo-auto/efficient' }, }, + { operationKey: expect.any(String) }, ]); }); @@ -283,6 +290,7 @@ describe('useRemoteSpawnDispatch spawn input chain', () => { model: { providerID: 'opencode', modelID: 'opencode-model', variant: 'xhigh' }, orgId: 'org-xyz', }, + { operationKey: expect.any(String) }, ]); }); @@ -295,6 +303,7 @@ describe('useRemoteSpawnDispatch spawn input chain', () => { expect(await captureSpawnCall(onStart)).toEqual([ 'conn-abc', { agent: 'code', orgId: 'org-xyz' }, + { operationKey: expect.any(String) }, ]); }); diff --git a/apps/mobile/src/components/agents/use-remote-spawn-dispatch.ts b/apps/mobile/src/components/agents/use-remote-spawn-dispatch.ts index d956607a26..f9b72bb403 100644 --- a/apps/mobile/src/components/agents/use-remote-spawn-dispatch.ts +++ b/apps/mobile/src/components/agents/use-remote-spawn-dispatch.ts @@ -9,9 +9,11 @@ import { buildCreateRemoteSessionInput, type CreateRemoteSessionInput, type CreateSessionOutcome, + type CreateSessionSpawnOptions, type RemoteInstanceSpawnStatus, useRemoteInstanceSpawn, } from '@/lib/hooks/use-remote-instance-spawn'; +import { useHoistedOperationKey } from '@/lib/operation-key'; import { REMOTE_SPAWN_NON_RETRYABLE_TOAST, REMOTE_SPAWN_RETRYABLE_TOAST, @@ -142,9 +144,16 @@ export function useRemoteSpawnDispatch({ // inherit by calling `useRemoteInstanceSpawn()` with no arg). const remoteSpawn: { status: RemoteInstanceSpawnStatus; - spawn: (connectionId: string, opts?: CreateRemoteSessionInput) => Promise; + spawn: ( + connectionId: string, + opts?: CreateRemoteSessionInput, + options?: CreateSessionSpawnOptions + ) => Promise; } = useRemoteInstanceSpawn(organizationId ?? null); const [showInstanceDisconnectedNote, setShowInstanceDisconnectedNote] = useState(false); + // P1-A-08b: one `operationKey` per spawn intent, so a retryable failure keeps + // the key and the relay dedupes the retry. + const { getKey, rotateKey } = useHoistedOperationKey(); // kilocode_change - `onStart`'s async tail (spawn + refetch + classify) // outlives a single render; a plain closure over `runOnInstance` would @@ -196,9 +205,19 @@ export function useRemoteSpawnDispatch({ selection: fields.selection, organizationId: fields.organizationId, }); + const operationKey = getKey( + JSON.stringify({ + connectionId: selectedConnectionId, + mode: fields.mode, + selection: fields.selection, + organizationId: fields.organizationId, + }) + ); void (async () => { - const outcome = await remoteSpawn.spawn(selectedConnectionId, createInput); + const outcome = await remoteSpawn.spawn(selectedConnectionId, createInput, { operationKey }); if (outcome.status === 'ready') { + // The spawn settled; the next submit is a fresh intent. + rotateKey(); const spawnedPath = getSpawnedAgentSessionPath(outcome.sessionID, organizationId); if (submitPayload === null) { router.replace(spawnedPath); @@ -211,6 +230,8 @@ export function useRemoteSpawnDispatch({ return; } if (outcome.status === 'nonRetryable') { + // A typed non-retryable rejection ends the intent. + rotateKey(); toast.error(REMOTE_SPAWN_NON_RETRYABLE_TOAST); return; } @@ -258,6 +279,8 @@ export function useRemoteSpawnDispatch({ router, runOnInstance, setRunOnInstance, + getKey, + rotateKey, ]); const onChangeRunOnInstance = useCallback( diff --git a/apps/mobile/src/components/pr-review/discussion/reply-input.tsx b/apps/mobile/src/components/pr-review/discussion/reply-input.tsx index b6d35eed66..46208e5bf0 100644 --- a/apps/mobile/src/components/pr-review/discussion/reply-input.tsx +++ b/apps/mobile/src/components/pr-review/discussion/reply-input.tsx @@ -11,6 +11,10 @@ import { Text } from '@/components/ui/text'; import { useThemeColors } from '@/lib/hooks/use-theme-colors'; import { classifyPrReviewMutationError } from '@/lib/pr-review/classify-pr-review-query-state'; import { type useReplyToCommentMutation } from '@/lib/pr-review/discussion/use-review-discussion-mutations'; +import { + isPrOperationPersistenceFailed, + PR_OPERATION_PERSISTENCE_FAILED_MESSAGE, +} from '@/lib/pr-review/merge/pr-operation-ledger'; const REPLY_PLACEHOLDER = 'Reply…'; @@ -37,6 +41,13 @@ export function ReplyInput({ owner, repo, number, commentId, reply }: Readonly { if (reply.error) { + // The ledger persistence-failure marker is retry-blocking: the row never + // became `reconcile_pending`, so the same key must not be retried. + if (isPrOperationPersistenceFailed(reply.error)) { + setInlineError(PR_OPERATION_PERSISTENCE_FAILED_MESSAGE); + setInlineErrorKind('bad-request'); + return; + } const classification = classifyPrReviewMutationError(reply.error); if (classification.kind === 'bad-request') { setInlineError("This reply can't be posted. The thread may have changed."); diff --git a/apps/mobile/src/components/pr-review/merge/pr-merge-sheet.test.tsx b/apps/mobile/src/components/pr-review/merge/pr-merge-sheet.test.tsx index 7e5be04828..355231c585 100644 --- a/apps/mobile/src/components/pr-review/merge/pr-merge-sheet.test.tsx +++ b/apps/mobile/src/components/pr-review/merge/pr-merge-sheet.test.tsx @@ -68,6 +68,13 @@ vi.mock('expo-haptics', () => ({ NotificationFeedbackType: { Success: 'Success' }, })); +// `pr-merge-sheet` imports the ledger helpers, which import `expo-crypto` +// (and transitively expo-modules-core). Mock it so this suite stays +// node-only, same as the other ledger pure tests. +vi.mock('expo-crypto', () => ({ + randomUUID: () => 'not-used-in-pure-tests', +})); + vi.mock('sonner-native', () => ({ toast: { error: vi.fn() }, })); diff --git a/apps/mobile/src/components/pr-review/merge/pr-merge-sheet.tsx b/apps/mobile/src/components/pr-review/merge/pr-merge-sheet.tsx index 07d963e3b8..9704bacf84 100644 --- a/apps/mobile/src/components/pr-review/merge/pr-merge-sheet.tsx +++ b/apps/mobile/src/components/pr-review/merge/pr-merge-sheet.tsx @@ -27,6 +27,10 @@ import { useMergePullRequestMutation, } from '@/lib/pr-review/merge/use-pr-merge-mutations'; import { classifyPrReviewMutationError } from '@/lib/pr-review/classify-pr-review-query-state'; +import { + isPrOperationPersistenceFailed, + PR_OPERATION_PERSISTENCE_FAILED_MESSAGE, +} from '@/lib/pr-review/merge/pr-operation-ledger'; import { applyMergeSuccessEffects } from '@/lib/pr-review/merge/merge-success-effects'; import { defaultMergeMethodOptionFor, @@ -149,6 +153,13 @@ export function PrMergeSheet(props: PrMergeSheetProps) { useEffect(() => { if (lastError) { + // The ledger persistence-failure marker is retry-blocking: the row never + // became `reconcile_pending`, so the same key must not be retried. + if (isPrOperationPersistenceFailed(lastError)) { + setInlineError(PR_OPERATION_PERSISTENCE_FAILED_MESSAGE); + setInlineErrorKind('non-retryable'); + return; + } const classification = classifyPrReviewMutationError(lastError); if (classification.kind === 'bad-request' || classification.kind === 'forbidden') { setInlineError( diff --git a/apps/mobile/src/components/security-agent/dashboard-screen.mounted.test.tsx b/apps/mobile/src/components/security-agent/dashboard-screen.mounted.test.tsx new file mode 100644 index 0000000000..8ef18d7566 --- /dev/null +++ b/apps/mobile/src/components/security-agent/dashboard-screen.mounted.test.tsx @@ -0,0 +1,256 @@ +/* eslint-disable typescript-eslint/no-deprecated -- react-test-renderer is the DOM-free renderer for RN trees under vitest (node env, no jsdom); its React 19 deprecation notice points to the DOM-based Testing Library, which cannot render this app's non-DOM tree. */ + +// Dashboard sync-control terminal-state contract: a non-retryable sync outcome +// (missing-configuration rejection, persistence failure) ends the intent — the +// hook rotates the operation key, so both sync controls ("Sync now" header +// button and the "Sync findings" empty-state action) must disable and the +// inline copy must explain the state. The missing-configuration rejection shows +// the state-specific configuration copy in place of the raw server message. +// Retryable failures keep both controls enabled so the user can retry under the +// same hoisted key. + +import { createElement } from 'react'; +import TestRenderer, { act } from 'react-test-renderer'; +import { beforeEach, describe, expect, it, vi } from 'vitest'; + +import { DashboardScreen } from './dashboard-screen'; + +const PERSISTENCE_FAILED_MESSAGE = vi.hoisted( + () => 'We could not record this action. Please try again later.' +); +const IN_PROGRESS_COPY = vi.hoisted( + () => 'A security sync is already in progress. Please try again.' +); +const CONFIGURATION_ERROR_MESSAGE = vi.hoisted(() => 'Security service is not configured'); +const CONFIGURATION_COPY = vi.hoisted( + () => 'Security service is not configured. Resubmitting cannot succeed until this is fixed.' +); + +const triggerSync = vi.hoisted(() => ({ + mutate: vi.fn(), + isPending: false, + isError: false, + error: null as Error | null, +})); +const config = vi.hoisted(() => ({ + data: { slaEnabled: true }, +})); +const dashboardStats = vi.hoisted(() => ({ + isLoading: false, + isError: false, + data: { sla: { overall: { total: 0 } } }, + refetch: vi.fn(), +})); +const lastSync = vi.hoisted(() => ({ + data: undefined as { lastSyncTime?: string } | undefined, + isError: false, + refetch: vi.fn(), +})); +const repositories = vi.hoisted(() => ({ + isLoading: false, + isError: false, + data: [] as unknown[], +})); +const capability = vi.hoisted(() => ({ + canManage: false, + isLoading: false, + isError: false, + refetch: vi.fn(), +})); + +vi.mock('react-native', () => ({ + View: 'View', + Pressable: 'Pressable', + RefreshControl: 'RefreshControl', +})); +vi.mock('@/components/ui/icons', () => ({ + RefreshCw: 'RefreshCw', + Settings: 'Settings', + ShieldAlert: 'ShieldAlert', +})); +vi.mock('expo-router', () => ({ + useRouter: () => ({ push: vi.fn() }), +})); +vi.mock('@expo/react-native-action-sheet', () => ({ + useActionSheet: () => ({ showActionSheetWithOptions: vi.fn() }), +})); +vi.mock('sonner-native', () => ({ toast: { success: vi.fn(), error: vi.fn() } })); +vi.mock('@/lib/hooks/use-theme-colors', () => ({ + useThemeColors: () => ({ foreground: '#000', mutedForeground: '#666', primary: '#000' }), +})); +vi.mock('@/lib/hooks/use-security-agent', () => ({ + useSecurityAgentCapability: () => capability, + useSecurityAgentConfig: () => config, + useSecurityAgentDashboardStats: () => dashboardStats, + useSecurityAgentLastSyncTime: () => lastSync, + useSecurityAgentRepositories: () => repositories, + useTriggerSecuritySync: () => triggerSync, +})); +// Faithful-enough mirror of the real classifier (covered by its own suite): +// the persistence-failure and replay-failed markers and the +// missing-configuration rejection are non-retryable, the rest (transport, +// in-progress copy, ambiguous, settle-failed) are retryable. +vi.mock('@/lib/hooks/use-security-agent-mutations', () => ({ + isSecurityConfigurationError: (error: unknown) => + error instanceof Error && error.message === CONFIGURATION_ERROR_MESSAGE, + SECURITY_CONFIGURATION_COPY: CONFIGURATION_COPY, + isSecuritySyncRetryable: (error: unknown) => { + const message = error instanceof Error ? error.message : ''; + return !( + message === 'We could not record this action. Please try again later.' || + message === 'This action did not complete. Please try again.' || + message === 'operation_key_reuse_mismatch' || + message === CONFIGURATION_ERROR_MESSAGE + ); + }, +})); +vi.mock('@kilocode/app-shared/security-agent', () => ({ + buildSecurityDashboardMetrics: () => [], + getSecurityRepositoriesInScope: () => [], +})); +vi.mock('@/lib/security-agent', () => ({ + getSecurityAgentPath: (scope: string, section: string) => `/security/${scope}/${section}`, +})); +vi.mock('@/lib/utils', () => ({ + cn: (...args: unknown[]) => args.filter(Boolean).join(' '), + parseTimestamp: (value: string) => value, + timeAgo: () => 'recently', +})); +vi.mock('@/components/screen-header', () => ({ ScreenHeader: () => null })); +vi.mock('@/components/query-error', () => ({ QueryError: () => null })); +vi.mock('@/components/security-agent/audit-report-button', () => ({ + AuditReportButton: () => null, +})); +vi.mock('@/components/security-agent/dashboard-sections', () => ({ + DashboardSections: () => null, +})); +vi.mock('@/components/ui/skeleton', () => ({ Skeleton: () => null })); +vi.mock('@/components/ui/spinning-icon', () => ({ SpinningIcon: () => null })); +vi.mock('@/components/ui/text', () => ({ Text: 'Text' })); +vi.mock('@/components/tab-screen', () => ({ + TabScreenScrollView: (props: { children?: unknown }) => props.children, +})); + +type R = TestRenderer.ReactTestRenderer; +type I = TestRenderer.ReactTestInstance; + +function renderScreen(): R { + const ref: { current: R | undefined } = { current: undefined }; + act(() => { + ref.current = TestRenderer.create(createElement(DashboardScreen, { scope: 'personal' })); + }); + const r = ref.current; + if (!r) { + throw new Error('renderer was not created'); + } + return r; +} + +function syncButtons(root: I): I[] { + return root.findAll( + n => + typeof n.type === 'string' && + (n.type as string) === 'Pressable' && + (n.props.accessibilityLabel === 'Sync now' || n.props.accessibilityLabel === 'Sync findings') + ); +} + +function findSyncButton(root: I, label: 'Sync now' | 'Sync findings'): I { + const nodes = syncButtons(root).filter(n => n.props.accessibilityLabel === label); + const n = nodes[0]; + if (!n) { + throw new Error(`expected 1 ${label} button, got ${nodes.length}`); + } + return n; +} + +function syncDisabledStates(root: I): boolean[] { + return syncButtons(root).map(n => n.props.disabled as boolean); +} + +function renderedTexts(root: I): string[] { + return root + .findAll( + n => + typeof n.type === 'string' && + (n.type as string) === 'Text' && + typeof n.props.children === 'string' + ) + .map(n => n.props.children as string); +} + +describe('DashboardScreen sync control terminal states', () => { + beforeEach(() => { + triggerSync.mutate.mockClear(); + triggerSync.isPending = false; + triggerSync.isError = false; + triggerSync.error = null; + config.data = { slaEnabled: true }; + dashboardStats.isLoading = false; + dashboardStats.isError = false; + dashboardStats.data = { sla: { overall: { total: 0 } } }; + lastSync.data = undefined; + lastSync.isError = false; + repositories.isLoading = false; + repositories.isError = false; + repositories.data = []; + capability.canManage = false; + }); + + it('keeps both sync controls enabled in the happy state', () => { + const root = renderScreen(); + + expect(syncButtons(root.root)).toHaveLength(2); + expect(syncDisabledStates(root.root)).toEqual([false, false]); + }); + + it('keeps both sync controls enabled after a retryable error and shows its copy', () => { + triggerSync.isError = true; + triggerSync.error = new Error(IN_PROGRESS_COPY); + const root = renderScreen(); + + expect(syncDisabledStates(root.root)).toEqual([false, false]); + expect(renderedTexts(root.root)).toContain(IN_PROGRESS_COPY); + }); + + it('disables both sync controls and shows configuration-specific copy after a missing-configuration error', () => { + triggerSync.isError = true; + triggerSync.error = new Error(CONFIGURATION_ERROR_MESSAGE); + const root = renderScreen(); + + expect(syncDisabledStates(root.root)).toEqual([true, true]); + expect(renderedTexts(root.root)).toContain(CONFIGURATION_COPY); + // The raw server message is replaced by the state-specific copy. + expect(renderedTexts(root.root)).not.toContain(CONFIGURATION_ERROR_MESSAGE); + }); + + it('disables both sync controls and shows the error copy after another non-retryable error', () => { + triggerSync.isError = true; + triggerSync.error = new Error(PERSISTENCE_FAILED_MESSAGE); + const root = renderScreen(); + + expect(syncDisabledStates(root.root)).toEqual([true, true]); + expect(renderedTexts(root.root)).toContain(PERSISTENCE_FAILED_MESSAGE); + }); + + it('disables both sync controls while a sync is pending', () => { + triggerSync.isPending = true; + const root = renderScreen(); + + expect(syncDisabledStates(root.root)).toEqual([true, true]); + }); + + it('submits the sync through the header control with the current repo filter', () => { + const root = renderScreen(); + const syncNow = findSyncButton(root.root, 'Sync now'); + + act(() => { + (syncNow.props.onPress as () => void)(); + }); + + expect(triggerSync.mutate).toHaveBeenCalledWith( + { repoFullName: undefined }, + expect.objectContaining({ onSuccess: expect.any(Function) }) + ); + }); +}); diff --git a/apps/mobile/src/components/security-agent/dashboard-screen.tsx b/apps/mobile/src/components/security-agent/dashboard-screen.tsx index 1ca4a9b72e..93b315835e 100644 --- a/apps/mobile/src/components/security-agent/dashboard-screen.tsx +++ b/apps/mobile/src/components/security-agent/dashboard-screen.tsx @@ -26,6 +26,11 @@ import { useSecurityAgentRepositories, useTriggerSecuritySync, } from '@/lib/hooks/use-security-agent'; +import { + isSecurityConfigurationError, + isSecuritySyncRetryable, + SECURITY_CONFIGURATION_COPY, +} from '@/lib/hooks/use-security-agent-mutations'; import { useThemeColors } from '@/lib/hooks/use-theme-colors'; import { getSecurityAgentPath } from '@/lib/security-agent'; import { cn, parseTimestamp, timeAgo } from '@/lib/utils'; @@ -86,6 +91,11 @@ export function DashboardScreen({ scope }: Readonly<{ scope: string }>) { // only" option list. const repoFilterUnavailable = repositories.isLoading || repositories.isError; + // A non-retryable sync outcome ended the intent, so both sync controls + // disable and the inline copy below explains the state. + const syncBlocked = triggerSync.isError && !isSecuritySyncRetryable(triggerSync.error); + const syncDisabled = triggerSync.isPending || syncBlocked; + const openRepoFilter = () => { if (repoFilterUnavailable) { return; @@ -165,11 +175,14 @@ export function DashboardScreen({ scope }: Readonly<{ scope: string }>) { } ); }} - disabled={triggerSync.isPending} + disabled={syncDisabled} accessibilityRole="button" accessibilityLabel="Sync now" - accessibilityState={{ disabled: triggerSync.isPending, busy: triggerSync.isPending }} - className="size-11 items-center justify-center active:opacity-70" + accessibilityState={{ disabled: syncDisabled, busy: triggerSync.isPending }} + className={cn( + 'size-11 items-center justify-center active:opacity-70', + syncBlocked && 'opacity-50' + )} > ) { Could not refresh — showing last synced data. ) : null} + {triggerSync.isError ? ( + + {isSecurityConfigurationError(triggerSync.error) + ? SECURITY_CONFIGURATION_COPY + : triggerSync.error.message} + + ) : null} + {dashboardStats.isLoading ? ( @@ -224,11 +245,17 @@ export function DashboardScreen({ scope }: Readonly<{ scope: string }>) { } ); }} - disabled={triggerSync.isPending} + disabled={syncDisabled} accessibilityRole="button" accessibilityLabel="Sync findings" - accessibilityState={{ disabled: triggerSync.isPending, busy: triggerSync.isPending }} - className="min-h-11 flex-row items-center gap-1.5 active:opacity-70" + accessibilityState={{ + disabled: syncDisabled, + busy: triggerSync.isPending, + }} + className={cn( + 'min-h-11 flex-row items-center gap-1.5 active:opacity-70', + syncBlocked && 'opacity-50' + )} > {triggerSync.isPending && ( diff --git a/apps/mobile/src/components/security-agent/dismiss-finding-screen.mounted.test.tsx b/apps/mobile/src/components/security-agent/dismiss-finding-screen.mounted.test.tsx new file mode 100644 index 0000000000..d9ade735d1 --- /dev/null +++ b/apps/mobile/src/components/security-agent/dismiss-finding-screen.mounted.test.tsx @@ -0,0 +1,226 @@ +/* eslint-disable typescript-eslint/no-deprecated -- react-test-renderer is the DOM-free renderer for RN trees under vitest (node env, no jsdom); its React 19 deprecation notice points to the DOM-based Testing Library, which cannot render this app's non-DOM tree. */ + +// Dismiss-screen terminal-state contract: a persistence failure (the ledger +// could not record the outcome, so a same-key retry guarantee does not hold) +// and a missing-configuration rejection are non-retryable — the form must show +// its state-specific copy and disable the dismissal CTA. Retryable failures +// (in-progress, ambiguous, transport) keep the CTA so the user can retry +// under the same hoisted key. + +import { createElement } from 'react'; +import TestRenderer, { act } from 'react-test-renderer'; +import { beforeEach, describe, expect, it, vi } from 'vitest'; + +import { DismissFindingScreen } from './dismiss-finding-screen'; + +const PERSISTENCE_FAILED_MESSAGE = vi.hoisted( + () => 'We could not record this action. Please try again later.' +); +const IN_PROGRESS_COPY = vi.hoisted( + () => 'This dismissal is already in progress. Please try again.' +); +const CONFIGURATION_ERROR_MESSAGE = vi.hoisted(() => 'Security service is not configured'); +const CONFIGURATION_COPY = vi.hoisted( + () => 'Security service is not configured. Resubmitting cannot succeed until this is fixed.' +); + +const routerBack = vi.hoisted(() => vi.fn()); +const dismiss = vi.hoisted(() => ({ + mutate: vi.fn(), + isPending: false, + isError: false, + error: null as Error | null, +})); +const capability = vi.hoisted(() => ({ + canManage: true, + isLoading: false, + isError: false, + refetch: vi.fn(), +})); +const finding = vi.hoisted(() => ({ + isLoading: false, + isError: false, + error: null as unknown, + data: { status: 'open' }, + refetch: vi.fn(), +})); +const pillGroup = vi.hoisted(() => ({ + onChange: (() => undefined) as (value: string) => void, +})); + +vi.mock('react-native', () => ({ + View: 'View', + ScrollView: 'ScrollView', + TextInput: 'TextInput', + ActivityIndicator: 'ActivityIndicator', +})); +vi.mock('@/components/ui/icons', () => ({ ShieldOff: 'ShieldOff' })); +vi.mock('expo-router', () => ({ + useRouter: () => ({ back: routerBack }), +})); +vi.mock('@/lib/hooks/use-theme-colors', () => ({ + useThemeColors: () => ({ mutedForeground: '#000', primaryForeground: '#fff' }), +})); +vi.mock('@/lib/hooks/use-security-agent', () => ({ + useSecurityAgentCapability: () => capability, +})); +vi.mock('@/lib/hooks/use-security-findings', () => ({ + useSecurityFinding: () => finding, + useDismissSecurityFinding: () => dismiss, +})); +// Faithful-enough mirror of the real classifier (covered by its own suite): +// the persistence-failure and replay-failed markers and the +// missing-configuration rejection are non-retryable, the rest (transport, +// in-progress copy, ambiguous, settle-failed) are retryable. +vi.mock('@/lib/hooks/use-security-agent-mutations', () => ({ + isSecurityConfigurationError: (error: unknown) => + error instanceof Error && error.message === CONFIGURATION_ERROR_MESSAGE, + SECURITY_CONFIGURATION_COPY: CONFIGURATION_COPY, + isSecuritySyncRetryable: (error: unknown) => { + const message = error instanceof Error ? error.message : ''; + return !( + message === 'We could not record this action. Please try again later.' || + message === 'This action did not complete. Please try again.' || + message === 'operation_key_reuse_mismatch' || + message === CONFIGURATION_ERROR_MESSAGE + ); + }, +})); +vi.mock('@/components/screen-header', () => ({ ScreenHeader: () => null })); +vi.mock('@/components/empty-state', () => ({ EmptyState: () => null })); +vi.mock('@/components/query-error', () => ({ QueryError: () => null })); +vi.mock('@/components/ui/skeleton', () => ({ Skeleton: () => null })); +vi.mock('@/components/security-agent/settings-pill-group', () => ({ + PillGroup: (props: { onChange: (value: string) => void }) => { + pillGroup.onChange = props.onChange; + return null; + }, +})); +vi.mock('@/components/ui/button', () => ({ Button: 'Button' })); +vi.mock('@/components/ui/text', () => ({ Text: 'Text' })); + +type R = TestRenderer.ReactTestRenderer; +type I = TestRenderer.ReactTestInstance; + +function renderScreen(): R { + const ref: { current: R | undefined } = { current: undefined }; + act(() => { + ref.current = TestRenderer.create( + createElement(DismissFindingScreen, { scope: 'personal', findingId: 'finding-1' }) + ); + }); + const r = ref.current; + if (!r) { + throw new Error('renderer was not created'); + } + return r; +} + +function selectReason(): void { + act(() => { + pillGroup.onChange('not_used'); + }); +} + +function findDismissButton(root: I): I { + const nodes = root.findAll(n => typeof n.type === 'string' && (n.type as string) === 'Button'); + if (nodes.length !== 1) { + throw new Error(`expected 1 dismissal Button, got ${nodes.length}`); + } + const n = nodes[0]; + if (!n) { + throw new Error('dismissal Button not found'); + } + return n; +} + +function buttonDisabled(root: I): boolean | undefined { + return findDismissButton(root).props.disabled as boolean | undefined; +} + +function renderedTexts(root: I): string[] { + return root + .findAll( + n => + typeof n.type === 'string' && + (n.type as string) === 'Text' && + typeof n.props.children === 'string' + ) + .map(n => n.props.children as string); +} + +describe('DismissFindingScreen dismissal CTA states', () => { + beforeEach(() => { + dismiss.mutate.mockClear(); + dismiss.isPending = false; + dismiss.isError = false; + dismiss.error = null; + capability.canManage = true; + capability.isLoading = false; + capability.isError = false; + finding.isLoading = false; + finding.isError = false; + finding.data = { status: 'open' }; + }); + + it('keeps the dismissal CTA enabled once a reason is chosen', () => { + const root = renderScreen(); + selectReason(); + + expect(buttonDisabled(root.root)).toBe(false); + }); + + it('keeps the dismissal CTA enabled after a retryable failure and shows its copy', () => { + dismiss.isError = true; + dismiss.error = new Error(IN_PROGRESS_COPY); + const root = renderScreen(); + selectReason(); + + expect(buttonDisabled(root.root)).toBe(false); + expect(renderedTexts(root.root)).toContain(IN_PROGRESS_COPY); + }); + + it('disables the dismissal CTA and shows the persistence-failure copy after a non-retryable error', () => { + dismiss.isError = true; + dismiss.error = new Error(PERSISTENCE_FAILED_MESSAGE); + const root = renderScreen(); + selectReason(); + + expect(buttonDisabled(root.root)).toBe(true); + expect(renderedTexts(root.root)).toContain(PERSISTENCE_FAILED_MESSAGE); + }); + + it('disables the dismissal CTA and shows configuration-specific copy after a missing-configuration error', () => { + dismiss.isError = true; + dismiss.error = new Error(CONFIGURATION_ERROR_MESSAGE); + const root = renderScreen(); + selectReason(); + + expect(buttonDisabled(root.root)).toBe(true); + expect(renderedTexts(root.root)).toContain(CONFIGURATION_COPY); + // The raw server message is replaced by the state-specific copy. + expect(renderedTexts(root.root)).not.toContain(CONFIGURATION_ERROR_MESSAGE); + }); + + it('disables the dismissal CTA while the dismissal is pending', () => { + dismiss.isPending = true; + const root = renderScreen(); + selectReason(); + + expect(buttonDisabled(root.root)).toBe(true); + }); + + it('submits the dismissal and pops the screen only on success', () => { + const root = renderScreen(); + selectReason(); + + act(() => { + (findDismissButton(root.root).props.onPress as () => void)(); + }); + + expect(dismiss.mutate).toHaveBeenCalledWith( + expect.objectContaining({ findingId: 'finding-1', reason: 'not_used' }), + expect.objectContaining({ onSuccess: expect.any(Function) }) + ); + }); +}); diff --git a/apps/mobile/src/components/security-agent/dismiss-finding-screen.tsx b/apps/mobile/src/components/security-agent/dismiss-finding-screen.tsx index 5efa464b5c..bb074c386d 100644 --- a/apps/mobile/src/components/security-agent/dismiss-finding-screen.tsx +++ b/apps/mobile/src/components/security-agent/dismiss-finding-screen.tsx @@ -11,6 +11,11 @@ import { Button } from '@/components/ui/button'; import { Skeleton } from '@/components/ui/skeleton'; import { Text } from '@/components/ui/text'; import { useSecurityAgentCapability } from '@/lib/hooks/use-security-agent'; +import { + isSecurityConfigurationError, + isSecuritySyncRetryable, + SECURITY_CONFIGURATION_COPY, +} from '@/lib/hooks/use-security-agent-mutations'; import { useDismissSecurityFinding, useSecurityFinding } from '@/lib/hooks/use-security-findings'; import { useThemeColors } from '@/lib/hooks/use-theme-colors'; @@ -152,6 +157,10 @@ export function DismissFindingScreen({ scope, findingId }: Readonly @@ -187,10 +196,14 @@ export function DismissFindingScreen({ scope, findingId }: Readonly {dismissFinding.isError && ( - {dismissFinding.error.message} + + {isSecurityConfigurationError(dismissFinding.error) + ? SECURITY_CONFIGURATION_COPY + : dismissFinding.error.message} + )} -