diff --git a/apps/mobile/src/lib/active-sessions-live-sync.test-helpers.ts b/apps/mobile/src/lib/active-sessions-live-sync.test-helpers.ts index c723792e84..3404bae60f 100644 --- a/apps/mobile/src/lib/active-sessions-live-sync.test-helpers.ts +++ b/apps/mobile/src/lib/active-sessions-live-sync.test-helpers.ts @@ -1,10 +1,10 @@ import { createElement, type ReactNode } from 'react'; import { type QueryClient, QueryClientProvider } from '@tanstack/react-query'; import { act, TestRenderer } from '@/test/renderer'; -import { afterEach, beforeEach, vi } from 'vitest'; +import { afterEach, beforeEach, expect, vi } from 'vitest'; import { createKiloAppQueryClient } from '@/lib/query-client'; import { bumpAuthEpoch } from '@/lib/auth/auth-epoch'; -import { setSignOutActive } from '@/lib/auth/sign-out-state'; +import { isSignOutActive, setSignOutActive } from '@/lib/auth/sign-out-state'; import { ActiveSessionsLiveSync, @@ -300,4 +300,17 @@ export function replaceMutationAccount(client: QueryClient, listKey: readonly un seedMutationSessions(client, listKey, 'Account B'); } +/** Assert the replaced account's caches and notification budget are untouched. */ +export function expectMutationAccountUnchanged( + client: QueryClient, + listKey: readonly unknown[], + messages: string[] +): void { + expect(mutationStoredTitles(client, listKey)).toEqual(['Account B', 'Other']); + expect(mutationActiveTitle(client)).toBe('Account B'); + expect(messages).toEqual([]); + expect(client.getQueryState(listKey)?.isInvalidated).toBe(false); + expect(isSignOutActive()).toBe(false); +} + export { ActiveSessionsLiveSync }; diff --git a/apps/mobile/src/lib/hooks/use-session-mutations.mounted.test.tsx b/apps/mobile/src/lib/hooks/use-session-mutations.mounted.test.tsx new file mode 100644 index 0000000000..967f098e08 --- /dev/null +++ b/apps/mobile/src/lib/hooks/use-session-mutations.mounted.test.tsx @@ -0,0 +1,107 @@ +import { createElement, useState } from 'react'; +import type * as ReactQuery from '@tanstack/react-query'; +import { afterEach, describe, expect, it, vi } from 'vitest'; + +import { act } from '@/test/renderer'; +import { renderWithProviders } from '@/test/render-with-providers'; +import { useSessionMutations } from './use-session-mutations'; + +type Input = { session_id: string; title?: string }; +type MutationOptions = ReactQuery.MutationOptions; + +const listKey = [['cliSessionsV2', 'list'], { type: 'infinite' }] as const; +const activeFilter = { queryKey: [['activeSessions', 'list']] }; + +// These modules reach react-native, whose Flow source the mounted (node) +// transform cannot parse. Only their render-time surface matters: no mutation +// runs in this suite, so inert stand-ins are enough. +vi.mock('@/lib/query/schedule-cache-maintenance', () => ({ scheduleCacheMaintenance: vi.fn() })); +vi.mock('@/lib/a11y/announcing-toast', () => ({ + announcingToast: { error: vi.fn(), success: vi.fn(), warning: vi.fn() }, +})); + +// Never invoked: this suite only renders the hook, it does not run a mutation. +const rpc = { rename: vi.fn(), delete: vi.fn() }; + +// Real TanStack Query drives the mutation observers for this suite, so the +// only stubbed input is the procedure surface the hook reads at render time. +vi.mock('@/lib/trpc', () => ({ + useTRPC: () => ({ + cliSessionsV2: { + list: { infiniteQueryKey: () => listKey }, + rename: { + mutationOptions: (options: MutationOptions) => ({ ...options, mutationFn: rpc.rename }), + }, + delete: { + mutationOptions: (options: MutationOptions) => ({ ...options, mutationFn: rpc.delete }), + }, + }, + activeSessions: { list: { pathFilter: () => activeFilter } }, + }), +})); + +type MutationsResult = ReturnType; + +const mounted: { unmount: () => void }[] = []; + +afterEach(() => { + for (const entry of mounted.splice(0)) { + entry.unmount(); + } +}); + +function requireRender(renders: MutationsResult[], index: number): MutationsResult { + const render = renders[index]; + if (!render) { + throw new Error(`expected a render at index ${index}`); + } + return render; +} + +/** Mounts a probe that records every render's callbacks and can force one. */ +async function mountProbe() { + const renders: MutationsResult[] = []; + + function Probe() { + const [, setTick] = useState(0); + renders.push(useSessionMutations()); + return createElement('Button', { + onPress: () => { + setTick(tick => tick + 1); + }, + }); + } + + const { renderer, unmount } = await renderWithProviders(createElement(Probe)); + mounted.push({ unmount }); + + return { + renders, + rerender: () => { + act(() => { + (renderer.root.findByType('Button').props.onPress as () => void)(); + }); + }, + }; +} + +describe('useSessionMutations callback identity', () => { + it('keeps deleteSession and renameSession stable across re-renders while observers are stable', async () => { + const { renders, rerender } = await mountProbe(); + + rerender(); + rerender(); + + // The mutation result object is rebuilt every render while the observer's + // mutateAsync stays stable for the hook's lifetime. If the callbacks + // depended on the result object (or dropped useCallback), these identities + // would change on every unrelated update and re-render every visible row. + const first = requireRender(renders, 0); + expect(renders.length).toBeGreaterThanOrEqual(3); + for (const render of renders) { + expect(render.deleteSession).toBe(first.deleteSession); + expect(render.renameSession).toBe(first.renameSession); + expect(render.renameSessionAsync).toBe(first.renameSessionAsync); + } + }); +}); diff --git a/apps/mobile/src/lib/hooks/use-session-mutations.test.ts b/apps/mobile/src/lib/hooks/use-session-mutations.test.ts index fc650704d6..eae01a09f0 100644 --- a/apps/mobile/src/lib/hooks/use-session-mutations.test.ts +++ b/apps/mobile/src/lib/hooks/use-session-mutations.test.ts @@ -1,9 +1,11 @@ import type * as ReactQuery from '@tanstack/react-query'; +import type * as React from 'react'; import { afterAll, afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; import { mutationActiveTitle as activeTitle, deferred, + expectMutationAccountUnchanged, flushQueryUpdates as flush, makeCached, makeTestQueryClient, @@ -12,7 +14,7 @@ import { seedMutationSessions, mutationStoredTitles as titles, } from '@/lib/active-sessions-live-sync.test-helpers'; -import { isSignOutActive, setSignOutActive } from '@/lib/auth/sign-out-state'; +import { setSignOutActive } from '@/lib/auth/sign-out-state'; import { setTrpcUnauthorizedHandler } from '@/lib/auth/trpc-unauthorized'; import { getActiveSessionsQueryMetadata } from '@/lib/query-client'; import { useSessionMutations } from './use-session-mutations'; @@ -27,6 +29,13 @@ const settled: (() => void)[] = []; const listKey = [['cliSessionsV2', 'list'], { type: 'infinite' }] as const; const activeFilter = { queryKey: [['activeSessions', 'list']] }; +// Directly-invoked hook: run the one real React hook it uses as identity, +// matching the react-query mocks below. +vi.mock('react', async () => { + const actual = await vi.importActual('react'); + return { ...actual, useCallback: unknown>(fn: T) => fn }; +}); + // Execute real mutations, including cancellation, context, and late rejection. vi.mock('@tanstack/react-query', async importOriginal => { const actual = await importOriginal(); @@ -78,13 +87,6 @@ vi.mock('@/lib/a11y/announcing-toast', () => { return { announcingToast: { error: record, success: record } }; }); -function expectAccountBUnchanged() { - expect(titles(client, listKey)).toEqual(['Account B', 'Other']); - expect(activeTitle(client)).toBe('Account B'); - expect(messages).toEqual([]); - expect(client.getQueryState(listKey)?.isInvalidated).toBe(false); - expect(isSignOutActive()).toBe(false); -} afterAll( setTrpcUnauthorizedHandler(() => { setSignOutActive(true); @@ -206,7 +208,7 @@ describe('useSessionMutations account publication', () => { } await Promise.all(outcomes); await flush(); - expectAccountBUnchanged(); + expectMutationAccountUnchanged(client, listKey, messages); expect(focused).toBe('account-b'); } ); @@ -237,7 +239,7 @@ describe('useSessionMutations account publication', () => { replaceMutationAccount(client, listKey); gate.resolve(undefined); await flush(); - expectAccountBUnchanged(); + expectMutationAccountUnchanged(client, listKey, messages); expect(rpc.rename.mock.calls).toEqual([]); expect(rpc.delete.mock.calls).toEqual([]); } @@ -261,7 +263,7 @@ describe('useSessionMutations account publication', () => { replaceMutationAccount(client, listKey); request.resolve(undefined); await flush(); - expectAccountBUnchanged(); + expectMutationAccountUnchanged(client, listKey, messages); expect(rpc.rename.mock.calls.map(call => call[0])).toEqual([ { session_id: 's1', title: 'New' }, ]); @@ -283,7 +285,7 @@ describe('useSessionMutations account publication', () => { 'inactive account' ); unsubscribe(); - expectAccountBUnchanged(); + expectMutationAccountUnchanged(client, listKey, messages); expect(rpc.rename.mock.calls).toEqual([]); }); diff --git a/apps/mobile/src/lib/hooks/use-session-mutations.ts b/apps/mobile/src/lib/hooks/use-session-mutations.ts index 7608afce6e..f4cb1b9bb2 100644 --- a/apps/mobile/src/lib/hooks/use-session-mutations.ts +++ b/apps/mobile/src/lib/hooks/use-session-mutations.ts @@ -1,4 +1,5 @@ import { hashKey, type QueryKey, useMutation, useQueryClient } from '@tanstack/react-query'; +import { useCallback } from 'react'; import { rememberUserSessionTitle } from '@/components/agents/session-detail-rename-state'; import { i18n } from '@/i18n'; @@ -185,24 +186,34 @@ export function useSessionMutations() { }, }); + // The mutation result object is rebuilt every render, so depend on the + // observer-bound `mutateAsync` (stable for the hook's lifetime) instead. + // Callers list these callbacks in their own dependency arrays; an unstable + // identity there would re-render every visible row on unrelated updates. + const deleteSessionAsync = deleteSessionMutation.mutateAsync; + const renameSessionMutationAsync = renameSessionMutation.mutateAsync; + // Preserve per-session sequencing and the detail caller's rejection contract. - const renameSessionAsync = async (sessionId: string, title: string) => { - const epoch = currentAuthEpoch(); - const input = { session_id: sessionId, title }; - // Record the user's own title before the write so the render paths never - // hide it as the backend's unnamed placeholder (the rename API accepts any - // nonblank title, including one that looks like the placeholder). - rememberUserSessionTitle(sessionId, title); - operationEpochs.set(input, epoch); - await chainSave(sessionId, async () => { - assertCurrentOperation(epoch); - await renameSessionMutation.mutateAsync(input); - assertCurrentOperation(epoch); - }); - }; + const renameSessionAsync = useCallback( + async (sessionId: string, title: string) => { + const epoch = currentAuthEpoch(); + const input = { session_id: sessionId, title }; + // Record the user's own title before the write so the render paths never + // hide it as the backend's unnamed placeholder (the rename API accepts any + // nonblank title, including one that looks like the placeholder). + rememberUserSessionTitle(sessionId, title); + operationEpochs.set(input, epoch); + await chainSave(sessionId, async () => { + assertCurrentOperation(epoch); + await renameSessionMutationAsync(input); + assertCurrentOperation(epoch); + }); + }, + [renameSessionMutationAsync] + ); - return { - deleteSession: (sessionId: string, onDeleted?: () => void) => { + const deleteSession = useCallback( + (sessionId: string, onDeleted?: () => void) => { const epoch = currentAuthEpoch(); const input = { session_id: sessionId }; operationEpochs.set(input, epoch); @@ -210,7 +221,7 @@ export function useSessionMutations() { try { await chainSave(sessionId, async () => { assertCurrentOperation(epoch); - await deleteSessionMutation.mutateAsync(input); + await deleteSessionAsync(input); }); assertCurrentOperation(epoch); announcingToast.success(i18n.t('agents.sessionRow.sessionDeleted')); @@ -220,7 +231,11 @@ export function useSessionMutations() { } })(); }, - renameSession: (sessionId: string, title: string) => { + [deleteSessionAsync] + ); + + const renameSession = useCallback( + (sessionId: string, title: string) => { void (async () => { try { await renameSessionAsync(sessionId, title); @@ -229,6 +244,12 @@ export function useSessionMutations() { } })(); }, + [renameSessionAsync] + ); + + return { + deleteSession, + renameSession, renameSessionAsync, }; } diff --git a/apps/mobile/src/lib/hooks/use-session-mutations.user-title.test.ts b/apps/mobile/src/lib/hooks/use-session-mutations.user-title.test.ts index 19b9e03fe0..0b555da2e9 100644 --- a/apps/mobile/src/lib/hooks/use-session-mutations.user-title.test.ts +++ b/apps/mobile/src/lib/hooks/use-session-mutations.user-title.test.ts @@ -1,4 +1,5 @@ import type * as ReactQuery from '@tanstack/react-query'; +import type * as React from 'react'; import { afterAll, afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; import { @@ -23,6 +24,17 @@ const settled: (() => void)[] = []; const listKey = [['cliSessionsV2', 'list'], { type: 'infinite' }] as const; const activeFilter = { queryKey: [['activeSessions', 'list']] }; +// This suite calls the hook directly, outside a renderer, so its React hooks +// must be inert. `useCallback` is a real React hook; run it as identity, the +// same contract the react-query mocks below rely on. +vi.mock('react', async () => { + const actual = await vi.importActual('react'); + return { + ...actual, + useCallback: vi.fn( unknown>(fn: T) => fn), + }; +}); + // Execute the real rename mutation so the hook's optimistic write and its // recording of the user's title both run. vi.mock('@tanstack/react-query', async importOriginal => {