Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 15 additions & 2 deletions apps/mobile/src/lib/active-sessions-live-sync.test-helpers.ts
Original file line number Diff line number Diff line change
@@ -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,
Expand Down Expand Up @@ -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 };
107 changes: 107 additions & 0 deletions apps/mobile/src/lib/hooks/use-session-mutations.mounted.test.tsx
Original file line number Diff line number Diff line change
@@ -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<unknown, Error, Input>;

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<typeof useSessionMutations>;

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);
}
});
});
26 changes: 14 additions & 12 deletions apps/mobile/src/lib/hooks/use-session-mutations.test.ts
Original file line number Diff line number Diff line change
@@ -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,
Expand All @@ -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';
Expand All @@ -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<typeof React>('react');
return { ...actual, useCallback: <T extends (...args: never[]) => unknown>(fn: T) => fn };
Comment thread
iscekic marked this conversation as resolved.
});

// Execute real mutations, including cancellation, context, and late rejection.
vi.mock('@tanstack/react-query', async importOriginal => {
const actual = await importOriginal<typeof ReactQuery>();
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -206,7 +208,7 @@ describe('useSessionMutations account publication', () => {
}
await Promise.all(outcomes);
await flush();
expectAccountBUnchanged();
expectMutationAccountUnchanged(client, listKey, messages);
expect(focused).toBe('account-b');
}
);
Expand Down Expand Up @@ -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([]);
}
Expand All @@ -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' },
]);
Expand All @@ -283,7 +285,7 @@ describe('useSessionMutations account publication', () => {
'inactive account'
);
unsubscribe();
expectAccountBUnchanged();
expectMutationAccountUnchanged(client, listKey, messages);
expect(rpc.rename.mock.calls).toEqual([]);
});

Expand Down
57 changes: 39 additions & 18 deletions apps/mobile/src/lib/hooks/use-session-mutations.ts
Original file line number Diff line number Diff line change
@@ -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';
Expand Down Expand Up @@ -185,32 +186,42 @@ 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);
void (async () => {
try {
await chainSave(sessionId, async () => {
assertCurrentOperation(epoch);
await deleteSessionMutation.mutateAsync(input);
await deleteSessionAsync(input);
});
assertCurrentOperation(epoch);
announcingToast.success(i18n.t('agents.sessionRow.sessionDeleted'));
Expand All @@ -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);
Expand All @@ -229,6 +244,12 @@ export function useSessionMutations() {
}
})();
},
[renameSessionAsync]
);

return {
deleteSession,
renameSession,
renameSessionAsync,
};
}
12 changes: 12 additions & 0 deletions apps/mobile/src/lib/hooks/use-session-mutations.user-title.test.ts
Original file line number Diff line number Diff line change
@@ -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 {
Expand All @@ -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<typeof React>('react');
return {
...actual,
useCallback: vi.fn(<T extends (...args: never[]) => 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 => {
Expand Down
Loading