From f0e97a6b2d05960b1bf608537380ba3bad210a8e Mon Sep 17 00:00:00 2001 From: OrcaWin Date: Tue, 27 Oct 2026 23:58:16 -0700 Subject: [PATCH] fix(mobile): stop repository requests for obsolete task searches (#34740) * fix(mobile): stop obsolete task repository requests * fix(mobile): release task-list spinners when a load is abandoned * fix(mobile): release spinners only for in-flight abandoned task loads --------- Co-authored-by: OrcaWin <293788423+OrcaWin@users.noreply.github.com> --- ...ask-list-linear-linear.searchissues-1.json | 21 +- .../mobile-task-request-cancellation.test.tsx | 314 ++++++++++++++++++ ...use-mobile-tasks-provider-load-actions.tsx | 15 +- .../use-mobile-tasks-task-list-loading.tsx | 32 +- ...ile-tasks-task-pagination-actions.test.tsx | 7 +- ...e-mobile-tasks-task-pagination-actions.tsx | 2 +- 6 files changed, 373 insertions(+), 18 deletions(-) create mode 100644 mobile/src/tasks/mobile-task-request-cancellation.test.tsx diff --git a/mobile/rpc-foundation/goldens/matrix-tasks.task-list-linear-linear.searchissues-1.json b/mobile/rpc-foundation/goldens/matrix-tasks.task-list-linear-linear.searchissues-1.json index c2a6cf2d71d..2f063fa8be9 100644 --- a/mobile/rpc-foundation/goldens/matrix-tasks.task-list-linear-linear.searchissues-1.json +++ b/mobile/rpc-foundation/goldens/matrix-tasks.task-list-linear-linear.searchissues-1.json @@ -72,6 +72,11 @@ "loading": false, "refreshing": false }, + "44679ef67a40": { + "name": "loading", + "ordinal": 12, + "value": false + }, "5b1f238d80cf": { "name": "loading", "ordinal": 9, @@ -134,11 +139,6 @@ "ordinal": 6, "value": false }, - "7560584e376a": { - "name": "error", - "ordinal": 13, - "value": "Connection closed" - }, "7b6df16c2cf0": { "name": "linear.listIssues#1", "ordinal": 4, @@ -631,6 +631,11 @@ } } }, + "cf9696e63483": { + "name": "refreshing", + "ordinal": 13, + "value": false + }, "d0a895d46972": { "name": "refreshing", "ordinal": 15, @@ -886,10 +891,8 @@ "88d78bf42794", "104bf14d3af8", "5b1f238d80cf", - "126a1c470845", - "7560584e376a", - "83e72fb8e8cd", - "d0a895d46972" + "44679ef67a40", + "cf9696e63483" ] } }, diff --git a/mobile/src/tasks/mobile-task-request-cancellation.test.tsx b/mobile/src/tasks/mobile-task-request-cancellation.test.tsx new file mode 100644 index 00000000000..ef41b73d865 --- /dev/null +++ b/mobile/src/tasks/mobile-task-request-cancellation.test.tsx @@ -0,0 +1,314 @@ +import { createElement, useLayoutEffect, useState } from 'react' +import { act, create, type ReactTestRenderer } from 'react-test-renderer' +import { afterEach, describe, expect, it, vi } from 'vitest' +import { createFakeRpcClient, type SentRequest } from '../mobile-web-shell/bridge-host-test-fakes' +import type { ConnectionState, RpcResponse } from '../transport/types' +import type { TaskItem } from './mobile-tasks-project-workspace-types' +import type { + ProviderLoadActionsInput, + ProviderLoadActionsModel +} from './use-mobile-tasks-provider-load-actions' +import type { TaskListLoadingModel } from './use-mobile-tasks-task-list-loading' + +vi.mock('./mobile-tasks-dependencies', async () => ({ + ...(await import('react')), + ...(await import('../theme/mobile-theme')), + ...(await import('../../../src/shared/work-items')), + ...(await import('./hosted-repo-selection')), + ...(await import('./github-work-item-source-errors')), + ...(await import('./mobile-workspace-name')), + TaskProviderLogo: () => null +})) +vi.mock('./mobile-tasks-legacy-foundation', async () => ({ + ...(await import('./mobile-tasks-item-mapping')), + ...(await import('./mobile-tasks-options')), + ...(await import('./mobile-tasks-repository-presentation')) +})) + +import { useMobileTasksProviderLoadActions } from './use-mobile-tasks-provider-load-actions' +import { useMobileTasksTaskListLoading } from './use-mobile-tasks-task-list-loading' +import { useMobileTasksTaskPaginationActions } from './use-mobile-tasks-task-pagination-actions' + +type Provider = 'github' | 'gitlab' +const mounted = new Set() +afterEach(() => { + act(() => { + for (const renderer of mounted) { + renderer.unmount() + } + }) + mounted.clear() +}) + +function reply(request: SentRequest): RpcResponse { + if (request.method === 'github.countWorkItems') { + return { id: 'count', ok: true, result: { count: 100 } } + } + const params = request.args[1] + const query = + params && typeof params === 'object' && 'query' in params ? String(params.query) : '' + return { + id: 'list', + ok: true, + result: { + items: [ + { + id: 'issue-1', + type: 'issue', + number: 1, + title: query, + state: 'open', + labels: [], + url: 'https://example.com/issues/1', + updatedAt: '2026-10-27T00:00:00Z', + author: null + } + ] + } + } +} + +async function mount(provider: Provider, repoCount = 30, counted = false) { + const client = createFakeRpcClient() + const scope: { + query: string + provider: Provider + client: typeof client + connState: ConnectionState + } = { + query: 'old-query', + provider, + client, + connState: 'connected' + } + const repos = Array.from({ length: repoCount }, (_, index) => ({ + id: `repo-${index}`, + displayName: `Repo ${index}`, + path: `/repo-${index}`, + kind: 'git' as const + })) + const shared = { + clientRef: { current: client }, + loadGenerationRef: { current: 0 }, + defaultLinearTeamSelectionRef: { current: null }, + jiraStatusRequestIdRef: { current: 0 }, + setJiraConnection: vi.fn(), + setLinearConnected: vi.fn(), + setLinearTeams: vi.fn(), + setLinearWorkspaces: vi.fn(), + setSelectedLinearTeamIds: vi.fn(), + setSelectedLinearWorkspaceId: vi.fn(), + setError: vi.fn(), + setGithubRepoSources: vi.fn(), + setGithubSourceErrors: vi.fn(), + setGithubSourceFallbacks: vi.fn(), + setLoading: vi.fn(), + setRefreshing: vi.fn(), + setGithubLoadingTargetPage: vi.fn(), + repoListEnsureLoaded: async () => repos, + selectedLinearTeamIds: new Set(), + effectiveRepoIds: new Set(counted ? repos.map((repo) => repo.id) : []), + serverRepoList: [], + jiraConnection: { connected: false }, + resetGitHubItemsState: vi.fn() + } + let current: ReturnType | undefined + function Harness() { + const [items, setItems] = useState([]) + const [githubPages, setGithubPages] = useState[][]>( + [] + ) + const [githubCurrentPage, setGithubCurrentPage] = useState(0) + const [githubCountedTotalPages, setGithubCountedTotalPages] = useState(null) + const [githubPaginationLoading, setGithubPaginationLoading] = useState(false) + useLayoutEffect(() => { + shared.clientRef.current = scope.client + }, [scope.client]) + const input = { + ...shared, + client: scope.client, + connState: scope.connState, + provider: scope.provider, + appliedQuery: scope.query, + githubKind: 'issues', + githubMode: 'items', + gitlabFilter: 'opened', + gitlabView: 'project', + jiraFilter: 'assigned', + linearConnected: false, + linearFilter: 'all', + linearOrderBy: 'priority', + selectedLinearWorkspaceId: null, + taskUiReady: true, + tasksSupported: true, + taskStateHydrated: true, + items, + setItems, + githubPages, + setGithubPages, + githubCurrentPage, + setGithubCurrentPage, + githubCountedTotalPages, + setGithubCountedTotalPages, + githubPaginationLoading, + setGithubPaginationLoading, + selectedHostedRepos: repos + } + // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: this fixture supplies every input read by the provider, list and pagination hooks on the task-list paths exercised here. + const provided = useMobileTasksProviderLoadActions(input as unknown as ProviderLoadActionsInput) + // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: the fixture above includes the list hook's runtime fields; the full app model also carries unrelated dialogs and mutations. + const loaded = useMobileTasksTaskListLoading(provided as ProviderLoadActionsModel) + // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: the fixture above includes all page-navigation fields and never invokes account connection or retry mutations. + current = useMobileTasksTaskPaginationActions(loaded as TaskListLoadingModel) + return null + } + let renderer: ReactTestRenderer + await act(async () => { + renderer = create(createElement(Harness)) + mounted.add(renderer) + }) + const answered = new Set() + const pending = () => client.requests.filter((request) => !answered.has(request)) + return { + client, + scope, + model: () => current!, + async render() { + await act(async () => renderer.update(createElement(Harness))) + }, + async load(options?: { silent?: boolean }) { + let request: Promise | undefined + await act(async () => { + request = current!.loadTasks(options) + }) + return { request: request! } + }, + pending, + async answer(requests = pending()) { + await act(async () => { + for (const request of requests) { + answered.add(request) + request.resolve(reply(request)) + } + }) + }, + async drain() { + for (let turn = 0; turn < repoCount + 2 && pending().length > 0; turn++) { + await this.answer() + } + expect(pending()).toHaveLength(0) + }, + unmount() { + act(() => renderer.unmount()) + mounted.delete(renderer) + } + } +} + +const starts = (requests: readonly SentRequest[], method: string) => + requests.filter((request) => request.method === method) + +describe('task repository request ownership', () => { + for (const provider of ['github', 'gitlab'] as const) { + for (const repoCount of [30, 300]) { + it(`stops queued ${provider} requests when a ${repoCount}-repo search is replaced`, async () => { + const fixture = await mount(provider, repoCount) + const old = await fixture.load() + const oldRequests = fixture.pending() + expect(oldRequests).toHaveLength(3) + fixture.scope.query = 'new-query' + await fixture.render() + const live = await fixture.load() + await fixture.answer(oldRequests) + expect(starts(fixture.client.requests, `${provider}.listWorkItems`)).toHaveLength(6) + await fixture.drain() + await Promise.all([old.request, live.request]) + expect(starts(fixture.client.requests, `${provider}.listWorkItems`)).toHaveLength( + repoCount + 3 + ) + expect(fixture.model().items).toHaveLength(repoCount) + expect(fixture.model().items.every((item) => item.title.includes('new-query'))).toBe(true) + }) + } + } + + it('stops obsolete page counts when the provider changes, keeping live GitLab rows', async () => { + const fixture = await mount('github', 30, true) + const first = await fixture.load() + while (fixture.pending().some((request) => request.method === 'github.listWorkItems')) { + await fixture.answer( + fixture.pending().filter((request) => request.method === 'github.listWorkItems') + ) + } + await first.request + const counts = fixture.pending() + expect(starts(counts, 'github.countWorkItems')).toHaveLength(3) + fixture.scope.provider = 'gitlab' + await fixture.render() + const live = await fixture.load() + await fixture.answer(counts) + await fixture.drain() + await live.request + expect(starts(fixture.client.requests, 'github.countWorkItems')).toHaveLength(3) + expect(fixture.model().items).toHaveLength(30) + expect(fixture.model().items.every((item) => item.provider === 'gitlab')).toBe(true) + }) + + it('stops queued next-page requests when the list reloads', async () => { + const fixture = await mount('github') + const first = await fixture.load() + await fixture.drain() + await first.request + let page: Promise | undefined + await act(async () => { + page = fixture.model().handleGitHubPageChange(1) + }) + const pageRequests = fixture.pending() + expect(pageRequests).toHaveLength(3) + fixture.scope.query = 'new-query' + await fixture.render() + const live = await fixture.load() + await fixture.answer(pageRequests) + await fixture.drain() + await Promise.all([page, live.request]) + const nextPage = fixture.client.requests.filter((request) => { + const params = request.args[1] + return params && typeof params === 'object' && 'page' in params && params.page === 2 + }) + expect(nextPage).toHaveLength(3) + expect(fixture.model().githubPages).toHaveLength(1) + expect(fixture.model().items.every((item) => item.title.includes('new-query'))).toBe(true) + }) + + for (const reason of ['unmount', 'disconnect', 'client-change'] as const) { + it(`stops the queued list after ${reason}`, async () => { + const fixture = await mount('github') + const old = await fixture.load() + if (reason === 'unmount') { + fixture.unmount() + } else { + if (reason === 'disconnect') { + fixture.scope.connState = 'disconnected' + } else { + fixture.scope.client = createFakeRpcClient() + } + await fixture.render() + } + await fixture.drain() + await old.request + expect(starts(fixture.client.requests, 'github.listWorkItems')).toHaveLength(3) + expect(fixture.model().items).toEqual([]) + expect(fixture.model().setLoading).toHaveBeenLastCalledWith(false) + }) + } + + it('releases the refresh spinner when a silent reload is abandoned by disconnect', async () => { + const fixture = await mount('github') + const old = await fixture.load({ silent: true }) + fixture.scope.connState = 'disconnected' + await fixture.render() + await fixture.drain() + await old.request + expect(fixture.model().setRefreshing).toHaveBeenLastCalledWith(false) + }) +}) diff --git a/mobile/src/tasks/use-mobile-tasks-provider-load-actions.tsx b/mobile/src/tasks/use-mobile-tasks-provider-load-actions.tsx index 73902f30439..fc154e1a02b 100644 --- a/mobile/src/tasks/use-mobile-tasks-provider-load-actions.tsx +++ b/mobile/src/tasks/use-mobile-tasks-provider-load-actions.tsx @@ -133,7 +133,8 @@ export function useMobileTasksProviderLoadActions boolean = () => true ): Promise<{ items: Array> failedCount: number @@ -150,6 +151,9 @@ export function useMobileTasksProviderLoadActions { + if (!isCurrent()) { + return { items: [], repoId: repo.id } + } try { const reply = await githubWorkItemSearchRead.request(requestClient, { repo: `id:${repo.id}`, @@ -218,11 +222,18 @@ export function useMobileTasksProviderLoadActions => { + async ( + requestClient: RpcClient, + queriedRepos: RepoSummary[], + isCurrent: () => boolean = () => true + ): Promise => { const counts = await mapWithConcurrency( queriedRepos, GITHUB_REPO_CONCURRENCY, async (repo) => { + if (!isCurrent()) { + return 0 + } try { const reply = await githubWorkItemCountRead.request( requestClient, diff --git a/mobile/src/tasks/use-mobile-tasks-task-list-loading.tsx b/mobile/src/tasks/use-mobile-tasks-task-list-loading.tsx index 5b71b0ca597..a9fd6ad3bb4 100644 --- a/mobile/src/tasks/use-mobile-tasks-task-list-loading.tsx +++ b/mobile/src/tasks/use-mobile-tasks-task-list-loading.tsx @@ -60,6 +60,7 @@ export function useMobileTasksTaskListLoading(model: ProviderLoadActionsModel) { taskStateHydrated, tasksSupported } = model + const unsettledLoadRef = useRef(0) const loadTasks = useCallback( async (options: { silent?: boolean } = {}): Promise => { if (!client || connState !== 'connected' || !tasksSupported || !taskStateHydrated) { @@ -67,6 +68,7 @@ export function useMobileTasksTaskListLoading(model: ProviderLoadActionsModel) { } const generation = loadGenerationRef.current + 1 loadGenerationRef.current = generation + unsettledLoadRef.current = generation const requestClient = client const isCurrent = () => loadGenerationRef.current === generation && clientRef.current === requestClient @@ -126,7 +128,7 @@ export function useMobileTasksTaskListLoading(model: ProviderLoadActionsModel) { return } if (provider === 'github') { - const page = await fetchGitHubItemsPage(requestClient, queriedRepos) + const page = await fetchGitHubItemsPage(requestClient, queriedRepos, 0, isCurrent) if (!isCurrent()) { return } @@ -140,11 +142,13 @@ export function useMobileTasksTaskListLoading(model: ProviderLoadActionsModel) { setGithubCurrentPage(0) setItems(page.items) if (effectiveRepoIds.size > 0) { - void countGitHubItemPages(requestClient, queriedRepos).then((totalPages) => { - if (isCurrent()) { - setGithubCountedTotalPages(totalPages) + void countGitHubItemPages(requestClient, queriedRepos, isCurrent).then( + (totalPages) => { + if (isCurrent()) { + setGithubCountedTotalPages(totalPages) + } } - }) + ) } else { // Do not spend GitHub Search quota on all-repo totals before the user narrows scope. setGithubCountedTotalPages(null) @@ -175,6 +179,9 @@ export function useMobileTasksTaskListLoading(model: ProviderLoadActionsModel) { queriedRepos, GITHUB_REPO_CONCURRENCY, async (repo) => { + if (!isCurrent()) { + return { items: [] } + } try { const reply = await gitlabWorkItemSearchRead.request(requestClient, { repo: `id:${repo.id}`, @@ -256,6 +263,9 @@ export function useMobileTasksTaskListLoading(model: ProviderLoadActionsModel) { resetGitHubItemsState() setError(err instanceof Error ? err.message : 'Failed to load tasks') } finally { + if (unsettledLoadRef.current === generation) { + unsettledLoadRef.current = 0 + } if (isCurrent()) { setLoading(false) setRefreshing(false) @@ -285,6 +295,18 @@ export function useMobileTasksTaskListLoading(model: ProviderLoadActionsModel) { tasksSupported ] ) + // Abandoning an in-flight load must release its spinners; a successor load in the same commit re-sets them. + useEffect( + () => () => { + loadGenerationRef.current += 1 + if (unsettledLoadRef.current !== 0) { + unsettledLoadRef.current = 0 + setLoading(false) + setRefreshing(false) + } + }, + [loadTasks, loadGenerationRef, setLoading, setRefreshing] + ) // Server-repo changes quietly reload unfiltered lists; a changed loadTasks already owns filter reloads, while Jira/Linear read independently. const listedServerReposRef = useRef({ serverRepoList, loadTasks }) useEffect(() => { diff --git a/mobile/src/tasks/use-mobile-tasks-task-pagination-actions.test.tsx b/mobile/src/tasks/use-mobile-tasks-task-pagination-actions.test.tsx index fd0ec7325e8..0ee485528e8 100644 --- a/mobile/src/tasks/use-mobile-tasks-task-pagination-actions.test.tsx +++ b/mobile/src/tasks/use-mobile-tasks-task-pagination-actions.test.tsx @@ -85,7 +85,12 @@ describe('GitHub task page change', () => { await act(async () => { await changePage?.(1) }) - expect(model.fetchGitHubItemsPage).toHaveBeenCalledWith(client, model.selectedHostedRepos, 1) + expect(model.fetchGitHubItemsPage).toHaveBeenCalledWith( + client, + model.selectedHostedRepos, + 1, + expect.any(Function) + ) expect(model.setGithubPages).toHaveBeenCalledWith([firstPage, oldSelectionPage]) expect(model.setItems).toHaveBeenCalledWith(oldSelectionPage) }) diff --git a/mobile/src/tasks/use-mobile-tasks-task-pagination-actions.tsx b/mobile/src/tasks/use-mobile-tasks-task-pagination-actions.tsx index 5ce9e1fa239..d24506a4aff 100644 --- a/mobile/src/tasks/use-mobile-tasks-task-pagination-actions.tsx +++ b/mobile/src/tasks/use-mobile-tasks-task-pagination-actions.tsx @@ -168,7 +168,7 @@ export function useMobileTasksTaskPaginationActions(model: TaskListLoadingModel) setGithubPaginationLoading(true) setGithubLoadingTargetPage(targetPage) try { - const page = await fetchGitHubItemsPage(client, selectedHostedRepos, targetPage) + const page = await fetchGitHubItemsPage(client, selectedHostedRepos, targetPage, isCurrent) if (!isCurrent() || page.items.length === 0) { return }