From 097fa6dca7efe8ec6baeeaf5753a494e53ab0aa8 Mon Sep 17 00:00:00 2001 From: Rei <2064470640@qq.com> Date: Fri, 6 Nov 2026 10:29:57 +0800 Subject: [PATCH] fix(frontend): fence a confirmed background-task cancellation against stale polls (#7914) A background-task detail or list read issued before the user cancels can resolve after the mutation's success handler and overwrite the confirmed "cancelled" snapshot with the pre-cancel "working" one, so the expanded card shows the task as active again (and polling resumes) until a later response corrects it. useCancelBackgroundTask now awaits cancelQueries for the thread's ["background-tasks", threadId] prefix before seeding the confirmed detail and list entry, so a read that started before the cancel is dropped instead of landing afterwards. The following list invalidation is unchanged, so a mounted card still refreshes from the server. This is the same ordering contract as the memory mutations fixed in #6959. Regression: frontend/tests/unit/core/background-tasks/hooks.dom.test.tsx uses a real QueryClient with deferred detail and list reads; it ends on "working" without the fix and keeps "cancelled" with it. Co-authored-by: REI-330 <173389910+REI-330@users.noreply.github.com> Co-authored-by: AochenShen99 --- backend/tests/test_agent_guidance_check.py | 1 + frontend/src/core/background-tasks/AGENTS.md | 8 + frontend/src/core/background-tasks/hooks.ts | 9 +- .../core/background-tasks/hooks.dom.test.tsx | 198 ++++++++++++++++++ 4 files changed, 215 insertions(+), 1 deletion(-) create mode 100644 frontend/src/core/background-tasks/AGENTS.md create mode 100644 frontend/tests/unit/core/background-tasks/hooks.dom.test.tsx diff --git a/backend/tests/test_agent_guidance_check.py b/backend/tests/test_agent_guidance_check.py index bfd613acf3..b85373dde8 100644 --- a/backend/tests/test_agent_guidance_check.py +++ b/backend/tests/test_agent_guidance_check.py @@ -59,6 +59,7 @@ EXPECTED_GUIDANCE_PATHS = { "frontend/src/components/workspace/AGENTS.md", "frontend/src/components/workspace/browser-view/AGENTS.md", "frontend/src/core/auth/AGENTS.md", + "frontend/src/core/background-tasks/AGENTS.md", "frontend/src/core/memory/AGENTS.md", "frontend/src/core/models/AGENTS.md", "frontend/tests/e2e/AGENTS.md", diff --git a/frontend/src/core/background-tasks/AGENTS.md b/frontend/src/core/background-tasks/AGENTS.md new file mode 100644 index 0000000000..08f50c28b1 --- /dev/null +++ b/frontend/src/core/background-tasks/AGENTS.md @@ -0,0 +1,8 @@ +# Background-task queries + +Cancelling a task must not be undone by a poll that started before the cancel +answered: the mutation drops the thread's `["background-tasks", threadId]` +reads before seeding the confirmed detail and list entry, then invalidates the +list prefix. Same ordering contract as `core/memory` (#6959). Regressions use a +real `QueryClient` in +`frontend/tests/unit/core/background-tasks/hooks.dom.test.tsx`. diff --git a/frontend/src/core/background-tasks/hooks.ts b/frontend/src/core/background-tasks/hooks.ts index 95c78573b5..5a2a2fe022 100644 --- a/frontend/src/core/background-tasks/hooks.ts +++ b/frontend/src/core/background-tasks/hooks.ts @@ -55,7 +55,14 @@ export function useCancelBackgroundTask(threadId: string) { const { t } = useI18n(); return useMutation({ mutationFn: (taskId: string) => cancelBackgroundTask(threadId, taskId), - onSuccess: (task) => { + onSuccess: async (task) => { + // This thread's task list and detail reads poll the same caches the + // confirmation is about to seed. A read that started before the cancel + // answered must not land afterwards and put the card back to "working" + // (#7177); same ordering contract as the memory mutations (#6959). + await queryClient.cancelQueries({ + queryKey: backgroundTasksQueryKey(threadId), + }); queryClient.setQueryData( backgroundTaskQueryKey(threadId, task.task_id), task, diff --git a/frontend/tests/unit/core/background-tasks/hooks.dom.test.tsx b/frontend/tests/unit/core/background-tasks/hooks.dom.test.tsx new file mode 100644 index 0000000000..46337db800 --- /dev/null +++ b/frontend/tests/unit/core/background-tasks/hooks.dom.test.tsx @@ -0,0 +1,198 @@ +import { afterEach, expect, test, rs } from "@rstest/core"; +import { QueryClient, QueryClientProvider } from "@tanstack/react-query"; +import { act, cleanup, renderHook } from "@testing-library/react"; +import type { ReactNode } from "react"; + +import * as api from "@/core/background-tasks/api"; +import { + backgroundTaskQueryKey, + backgroundTasksQueryKey, + useCancelBackgroundTask, +} from "@/core/background-tasks/hooks"; +import type { + BackgroundTask, + BackgroundTaskDetail, + BackgroundTaskStatus, +} from "@/core/background-tasks/types"; +import { enUS } from "@/core/i18n/locales/en-US"; + +rs.mock("@/core/background-tasks/api", () => ({ + fetchBackgroundTasks: rs.fn(), + fetchBackgroundTask: rs.fn(), + cancelBackgroundTask: rs.fn(), +})); +rs.mock("@/core/i18n/hooks", () => ({ useI18n: () => ({ t: enUS }) })); + +const THREAD_ID = "thread-1"; +const TASK_ID = "task-1"; + +function task(status: BackgroundTaskStatus): BackgroundTask { + return { + task_id: TASK_ID, + task_name: "MCP task", + status, + created_at: "2026-10-20T07:00:00Z", + updated_at: "2026-10-20T07:00:03Z", + error: null, + tracking_degraded: false, + cancel_requested: status === "cancelled", + }; +} + +function detail(status: BackgroundTaskStatus): BackgroundTaskDetail { + return { + ...task(status), + last_polled_at: null, + last_poll_error: null, + last_cancel_error: null, + cancel_attempt_count: 0, + notification_status: "none", + notification_error: null, + notification_attempt_count: 0, + result: null, + result_preview: null, + result_truncated: false, + result_artifact: null, + input_required: null, + }; +} + +function deferred() { + let resolve!: (value: T) => void; + const promise = new Promise((done) => { + resolve = done; + }); + return { promise, resolve }; +} + +const clients: QueryClient[] = []; +afterEach(() => { + cleanup(); + for (const client of clients.splice(0)) client.clear(); + rs.resetAllMocks(); +}); + +function makeClient(seed: BackgroundTaskDetail): QueryClient { + const client = new QueryClient({ + defaultOptions: { + queries: { retry: false, gcTime: Infinity }, + mutations: { retry: false, gcTime: Infinity }, + }, + }); + clients.push(client); + client.setQueryData(backgroundTasksQueryKey(THREAD_ID), [task(seed.status)]); + client.setQueryData(backgroundTaskQueryKey(THREAD_ID, TASK_ID), seed); + return client; +} + +function wrapperFor(client: QueryClient) { + return function wrapper({ children }: { children: ReactNode }) { + return ( + {children} + ); + }; +} + +test("a detail or list poll that started before the cancel cannot undo it", async () => { + const working = detail("working"); + const cancelled = detail("cancelled"); + const staleDetail = deferred(); + const staleList = deferred(); + rs.mocked(api.fetchBackgroundTask).mockReturnValue(staleDetail.promise); + rs.mocked(api.fetchBackgroundTasks).mockReturnValue(staleList.promise); + rs.mocked(api.cancelBackgroundTask).mockResolvedValue(cancelled); + + const client = makeClient(working); + const { result } = renderHook(() => useCancelBackgroundTask(THREAD_ID), { + wrapper: wrapperFor(client), + }); + + // Both the expanded card's detail poll and the list poll were in flight when + // the user pressed Cancel, so both still carry the pre-cancel snapshot. + let detailRead!: Promise; + let listRead!: Promise; + act(() => { + detailRead = client.fetchQuery({ + queryKey: backgroundTaskQueryKey(THREAD_ID, TASK_ID), + queryFn: () => api.fetchBackgroundTask(THREAD_ID, TASK_ID), + }); + listRead = client.fetchQuery({ + queryKey: backgroundTasksQueryKey(THREAD_ID), + queryFn: () => api.fetchBackgroundTasks(THREAD_ID), + }); + }); + expect(api.fetchBackgroundTask).toHaveBeenCalledTimes(1); + expect(api.fetchBackgroundTasks).toHaveBeenCalledTimes(1); + + await act(async () => { + await result.current.mutateAsync(TASK_ID); + }); + const detailKey = backgroundTaskQueryKey(THREAD_ID, TASK_ID); + const listKey = backgroundTasksQueryKey(THREAD_ID); + expect(client.getQueryData(detailKey)?.status).toBe( + "cancelled", + ); + expect(client.getQueryData(listKey)?.[0]?.status).toBe( + "cancelled", + ); + + await act(async () => { + staleDetail.resolve(working); + staleList.resolve([task("working")]); + await Promise.allSettled([detailRead, listRead]); + }); + + // The confirmed cancellation survives the reads that resolved after it, so + // the card does not fall back to "working" and resume polling. + expect(client.getQueryData(detailKey)?.status).toBe( + "cancelled", + ); + expect(client.getQueryData(listKey)?.[0]?.status).toBe( + "cancelled", + ); +}); + +test("a failed cancel keeps the cache and lets the in-flight read finish", async () => { + const working = detail("working"); + const refreshed = detail("working"); + refreshed.last_polled_at = "2026-10-20T07:00:06Z"; + const staleDetail = deferred(); + rs.mocked(api.fetchBackgroundTask).mockReturnValue(staleDetail.promise); + rs.mocked(api.cancelBackgroundTask).mockRejectedValue( + new Error("Cancel failed"), + ); + + const client = makeClient(working); + const { result } = renderHook(() => useCancelBackgroundTask(THREAD_ID), { + wrapper: wrapperFor(client), + }); + + let detailRead!: Promise; + act(() => { + detailRead = client.fetchQuery({ + queryKey: backgroundTaskQueryKey(THREAD_ID, TASK_ID), + queryFn: () => api.fetchBackgroundTask(THREAD_ID, TASK_ID), + }); + }); + + await act(async () => { + await expect(result.current.mutateAsync(TASK_ID)).rejects.toThrow( + "Cancel failed", + ); + }); + expect( + client.getQueryData( + backgroundTaskQueryKey(THREAD_ID, TASK_ID), + ), + ).toEqual(working); + + await act(async () => { + staleDetail.resolve(refreshed); + await detailRead; + }); + expect( + client.getQueryData( + backgroundTaskQueryKey(THREAD_ID, TASK_ID), + )?.last_polled_at, + ).toBe("2026-10-20T07:00:06Z"); +});