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
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,12 @@ const listState = vi.hoisted(() => ({
isSearching: false,
isError: false,
organization: { organizationId: null as string | null, isLoaded: true },
// Mirrors the real hook's stored-query flags so the screen's loading
// decision can be exercised on the first render, before the request
// settles (isFetching false, isPending true).
storedIsPending: false,
storedIsFetching: false,
storedLoadedPageCount: 1,
storedQuery: vi.fn<(options: Parameters<typeof useAgentSessions>[0]) => void>(),
searchQuery: vi.fn<(options: Parameters<typeof useAgentSessionSearch>[0]) => void>(),
repositoryQuery: vi.fn<(options: Parameters<typeof useRecentAgentRepositories>[0]) => void>(),
Expand Down Expand Up @@ -135,8 +141,9 @@ vi.mock('@/lib/hooks/use-agent-sessions', async () => {
dateGroups: storedSessions.length > 0 ? [{ label: 'Today', sessions: storedSessions }] : [],
activeIsError: false,
storedIsError: listState.isError,
storedIsFetching: false,
storedLoadedPageCount: 1,
storedIsPending: listState.storedIsPending,
storedIsFetching: listState.storedIsFetching,
storedLoadedPageCount: listState.storedLoadedPageCount,
hasNextPage: false,
isFetchingNextPage: false,
fetchNextPage: vi.fn(),
Expand Down Expand Up @@ -258,6 +265,9 @@ describe('SessionHistoryScreen', () => {
listState.storedSessions = [];
listState.isSearching = false;
listState.isError = false;
listState.storedIsPending = false;
listState.storedIsFetching = false;
listState.storedLoadedPageCount = 1;
Object.assign(listState.organization, { organizationId: null, isLoaded: true });
listState.storedQuery.mockClear();
listState.searchQuery.mockClear();
Expand Down Expand Up @@ -460,6 +470,50 @@ describe('SessionHistoryScreen', () => {
}
);

// Regression: the empty state ("No past sessions") must not flash on the
// cold-open render. React Query v5 reports `isFetching: false` until the
// observer subscribes and starts the first fetch, while `isPending` stays
// true until the query settles — the screen must treat that first frame as
// loading, not as a settled empty list.
it('shows loading on the cold-open render before the request settles', async () => {
listState.storedSessions = [];
listState.storedIsPending = true;
listState.storedIsFetching = false;
listState.storedLoadedPageCount = 0;

const renderer = await renderScreen();

const content = findNodeByType(renderer, 'AgentSessionListContent');
expect(content.props.isLoading).toBe(true);
expect(content.props.hasAnySessions).toBe(false);
});

it('stops loading and renders cached rows during a background refetch', async () => {
listState.storedSessions = [{ session_id: 'cached', organization_id: null }];
listState.storedIsPending = false;
listState.storedIsFetching = true;
listState.storedLoadedPageCount = 1;

const renderer = await renderScreen();

const content = findNodeByType(renderer, 'AgentSessionListContent');
expect(content.props.isLoading).toBe(false);
expect(findNodeByType(renderer, 'AgentSessionListContent').props.sections).toHaveLength(1);
});

it('shows the settled empty state once the request completes with no rows', async () => {
listState.storedSessions = [];
listState.storedIsPending = false;
listState.storedIsFetching = false;
listState.storedLoadedPageCount = 1;

const renderer = await renderScreen();

const content = findNodeByType(renderer, 'AgentSessionListContent');
expect(content.props.isLoading).toBe(false);
expect(content.props.hasAnySessions).toBe(false);
});

it('renders the agents title with a back button and default header size', async () => {
const renderer = await renderScreen();
const header = findNodeByType(renderer, 'ScreenHeader');
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ type MockStoredSession = Pick<StoredSession, 'session_id' | 'organization_id'> &
const listState = vi.hoisted(() => ({
storedSessions: [] as MockStoredSession[],
isError: false,
storedIsPending: false,
storedIsFetching: false,
}));

Expand Down Expand Up @@ -188,6 +189,7 @@ vi.mock('@/lib/hooks/use-agent-sessions', () => ({
dateGroups: storedSessions.length > 0 ? [{ label: 'Today', sessions: storedSessions }] : [],
activeIsError: false,
storedIsError: listState.isError,
storedIsPending: listState.storedIsPending,
storedIsFetching: listState.storedIsFetching,
storedLoadedPageCount: 1,
hasNextPage: false,
Expand Down Expand Up @@ -261,6 +263,7 @@ beforeEach(() => {
},
];
listState.isError = false;
listState.storedIsPending = false;
listState.storedIsFetching = false;
});

Expand Down
12 changes: 8 additions & 4 deletions apps/mobile/src/components/agents/session-history-screen.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import { useFocusEffect, useNavigation } from 'expo-router';
import { SessionFilterModal } from '@/components/agents/platform-filter-modal';
import { AgentSessionListContent } from '@/components/agents/session-list-content';
import { SessionListHeaderActions } from '@/components/agents/session-list-header-actions';
import { selectSessionListIsLoading } from '@/components/agents/session-list-loading';
import { selectShowSearchBusy } from '@/components/agents/session-list-search-busy';
import { SessionListSearchHeader } from '@/components/agents/session-list-search-header';
import { useAgentSessionListData } from '@/components/agents/use-agent-session-list-data';
Expand Down Expand Up @@ -76,8 +77,7 @@ export function SessionHistoryScreen() {
const {
storedSessions,
activeSessionIds,
storedIsFetching,
storedLoadedPageCount,
storedIsPending,
paging,
handleRetry,
handleRefetch,
Expand Down Expand Up @@ -163,8 +163,12 @@ export function SessionHistoryScreen() {
clearFilters();
}, [clearSearchInput, searchController, clearFilters, isSearching]);

const isLoading =
!ready || (isSearching ? search.isPending : storedIsFetching && storedLoadedPageCount === 0);
const isLoading = selectSessionListIsLoading({
ready,
isSearching,
searchIsPending: search.isPending,
storedIsPending,
});

return (
<View className="flex-1 bg-background">
Expand Down
86 changes: 86 additions & 0 deletions apps/mobile/src/components/agents/session-list-loading.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,86 @@
import { describe, expect, it } from 'vitest';

import { selectSessionListContentSurface } from './session-list-content-surface';
import { selectSessionListIsLoading } from './session-list-loading';

function loading(overrides: Partial<Parameters<typeof selectSessionListIsLoading>[0]> = {}) {
return selectSessionListIsLoading({
ready: true,
isSearching: false,
searchIsPending: false,
storedIsPending: false,
...overrides,
});
}

describe('selectSessionListIsLoading', () => {
describe('cold open — first render before the request settles', () => {
it('treats the very first render as loading, not a settled empty list', () => {
// React Query v5 reports isLoading/isFetching false until the observer
// starts the fetch; only isPending is true. The surface must show
// skeletons, never the empty state, on this frame.
expect(loading({ isSearching: false, searchIsPending: false, storedIsPending: true })).toBe(
true
);
});

it('stays loading until the query inputs resolve', () => {
expect(loading({ ready: false, isSearching: false, storedIsPending: true })).toBe(true);
expect(loading({ ready: false, isSearching: false, storedIsPending: false })).toBe(true);
});

it('treats a pending search on the first render as loading', () => {
expect(loading({ isSearching: true, searchIsPending: true, storedIsPending: false })).toBe(
true
);
});
});

describe('after load', () => {
it('is not loading once the stored query settles with no rows (true empty)', () => {
expect(loading({ isSearching: false, searchIsPending: false, storedIsPending: false })).toBe(
false
);
});

it('stops loading when a search settles with no matches', () => {
expect(loading({ isSearching: true, searchIsPending: false, storedIsPending: false })).toBe(
false
);
});

it('reads the search flag, not the stored flag, while searching', () => {
expect(loading({ isSearching: true, searchIsPending: false, storedIsPending: true })).toBe(
false
);
expect(loading({ isSearching: false, searchIsPending: true, storedIsPending: false })).toBe(
false
);
});
});

// Ties the loading decision to the body-surface decision: the combination is
// what the screen actually renders, and it is where the flash came from.
describe('cold-open body-surface decision', () => {
const surfaceInput = {
isError: false,
hasAnySessions: false,
hasHistoryContent: false,
};

it('selects skeletons, never the history-empty surface, before the request settles', () => {
const isLoading = loading({ isSearching: false, storedIsPending: true });
expect(selectSessionListContentSurface({ isLoading, ...surfaceInput })).toEqual({
kind: 'section-list',
listEmpty: 'loading-skeletons',
});
});

it('selects the history-empty surface only after the request settles', () => {
const isLoading = loading({ isSearching: false, storedIsPending: false });
expect(selectSessionListContentSurface({ isLoading, ...surfaceInput })).toEqual({
kind: 'history-empty',
});
});
});
});
29 changes: 29 additions & 0 deletions apps/mobile/src/components/agents/session-list-loading.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
/**
* Loading decision shared by the stored session-list surfaces (the history
* screen and any other surface that renders the same stored rows).
*
* React Query v5's `isLoading` is `isPending && isFetching`, so it is false on
* the first render — the observer has not started the fetch yet — and while a
* query is paused (offline). The body surfaces gate their empty/error states on
* this flag, so keying "no data yet" off `isFetching` lets a cold open paint
* "No past sessions" for a frame before the request settles.
*
* `isPending` stays true until the query settles (success or error), which is
* exactly the "keep showing skeletons until the request settles" contract. It
* is false as soon as any page is cached, so a background refetch never blanks
* out rows that are already rendered.
*/
export function selectSessionListIsLoading(input: {
/** Query inputs (org, persisted filters, identity) have resolved. */
ready: boolean;
isSearching: boolean;
/** `search.isPending` — no search result cached yet. */
searchIsPending: boolean;
/** `stored.isPending` — no stored page cached yet. */
storedIsPending: boolean;
}): boolean {
if (!input.ready) {
return true;
}
return input.isSearching ? input.searchIsPending : input.storedIsPending;
}
Original file line number Diff line number Diff line change
Expand Up @@ -36,8 +36,7 @@ export function useAgentSessionListData(options: {
dateGroups,
activeIsError,
storedIsError,
storedIsFetching,
storedLoadedPageCount,
storedIsPending,
hasNextPage,
isFetchingNextPage,
fetchNextPage,
Expand Down Expand Up @@ -149,8 +148,7 @@ export function useAgentSessionListData(options: {
return {
storedSessions,
activeSessionIds,
storedIsFetching,
storedLoadedPageCount,
storedIsPending,
paging,
refetch,
handleRetry,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import { TrustedHostsScreen } from './trusted-hosts-screen';
const query = vi.hoisted(() => ({
data: undefined as DeviceSession[] | undefined,
isLoading: false,
isPending: false,
isError: false,
isFetching: false,
refetch: vi.fn(),
Expand Down Expand Up @@ -63,6 +64,7 @@ vi.mock('@/lib/format', () => ({ formatDate: () => 'Date' }));
beforeEach(() => {
query.data = undefined;
query.isLoading = false;
query.isPending = false;
query.isError = false;
query.refetch.mockClear();
hosts.hasLoaded = true;
Expand Down Expand Up @@ -110,7 +112,7 @@ describe('account surface states', () => {
});

it('keeps device loading ahead of error and empty states', async () => {
query.isLoading = true;
query.isPending = true;
query.isError = true;
const { renderer, unmount } = await renderWithProviders(createElement(DeviceSessionsScreen));
expect(renderer.root.findAll(node => String(node.type) === 'Skeleton')).toHaveLength(12);
Expand All @@ -119,6 +121,20 @@ describe('account surface states', () => {
unmount();
});

// Regression: React Query v5's `isLoading` is false on the first render
// before the observer starts fetching. The very first frame of a cold open
// must still be the skeleton, not the empty state.
it('shows the skeleton on the cold-open render before the request settles', async () => {
query.isPending = true;
query.isLoading = false;
query.isError = false;
query.data = undefined;
const { renderer, unmount } = await renderWithProviders(createElement(DeviceSessionsScreen));
expect(renderer.root.findAll(node => String(node.type) === 'Skeleton')).toHaveLength(12);
expect(renderer.root.findAll(node => String(node.type) === 'EmptyState')).toHaveLength(0);
unmount();
});

it('lifts trusted host emptiness outside the scroller', async () => {
const { renderer, unmount } = await renderWithProviders(createElement(TrustedHostsScreen));
expect(renderer.root.findAll(node => String(node.type) === 'TabScreenScrollView')).toHaveLength(
Expand Down
7 changes: 5 additions & 2 deletions apps/mobile/src/components/device-sessions-screen.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -84,13 +84,16 @@ export function DeviceSessionsScreen() {
const trpc = useTRPC();
const { t } = useTranslation();

const { data, isLoading, isError, isFetching, refetch } = useQuery({
const { data, isPending, isError, isFetching, refetch } = useQuery({
...trpc.user.listDeviceSessions.queryOptions(),
enabled: token != null,
});

const state = classifyDeviceSessionsState({
isLoading,
// `isPending`, not `isLoading`: React Query v5's `isLoading` is
// `isPending && isFetching`, so it is false on the first render before the
// fetch starts and the cold open would classify as `empty`.
isPending,
isError: isError && data === undefined,
data,
});
Expand Down
17 changes: 12 additions & 5 deletions apps/mobile/src/lib/device-sessions.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -73,31 +73,38 @@ describe('classifyDeviceSessionsState', () => {
it.each([
{
name: 'loading ahead of stale data',
args: { isLoading: true, isError: false, data: rows },
args: { isPending: true, isError: false, data: rows },
expected: 'loading',
},
{
// Cold open, first render: the observer has not started fetching yet so
// React Query's `isLoading` would be false, but `isPending` is true.
name: 'the first render before the request settles as loading, not empty',
args: { isPending: true, isError: false, data: undefined },
expected: 'loading',
},
{
name: 'a query error as retryable error',
args: { isLoading: false, isError: true, data: undefined },
args: { isPending: false, isError: true, data: undefined },
expected: 'error',
},
{
name: 'zero rows as empty',
args: { isLoading: false, isError: false, data: [] },
args: { isPending: false, isError: false, data: [] },
expected: 'empty',
},
{
name: 'rows with a current row as happy',
args: {
isLoading: false,
isPending: false,
isError: false,
data: [makeSession({ id: 'b', isCurrent: true }), ...rows],
},
expected: 'happy',
},
{
name: 'rows without a current row as no-current, never empty',
args: { isLoading: false, isError: false, data: rows },
args: { isPending: false, isError: false, data: rows },
expected: 'no-current',
},
])('classifies $name', ({ args, expected }) => {
Expand Down
Loading