diff --git a/dashboard/src/workspaces/explorer/controller.ts b/dashboard/src/workspaces/explorer/controller.ts index 37d15f2fae4..5e53978856f 100644 --- a/dashboard/src/workspaces/explorer/controller.ts +++ b/dashboard/src/workspaces/explorer/controller.ts @@ -157,6 +157,18 @@ interface ExplorerFacet { readonly value: string; } +/** The request owns its admission scope, regardless of later UI selections. */ +interface PlannerRequest { + readonly scope: DashboardScope; + readonly query: string; + readonly generation: number; +} + +interface AcceptedPlannerRun { + readonly scope: DashboardScope; + readonly id: string; +} + /** How far the coordinator has got, counted in sources rather than in a * percentage: sources have incommensurable units, so "2 of 3 concluded" is the * only progress figure with a real denominator. */ @@ -220,13 +232,40 @@ export function useExplorerController(): ExplorerController { const [peeked, setPeeked] = useState(null); const searching = submitted !== ''; const terms = useMemo(() => queryTerms(submitted), [submitted]); + // A superseded POST can still be admitted after a newer search or after + // unmount. Its accepted run must be explicitly cancelled, not orphaned. + const requestGeneration = useRef(0); + const acceptedRun = useRef(null); + const retireAcceptedRun = () => { + const previous = acceptedRun.current; + acceptedRun.current = null; + if (previous !== null) void cancelPlannerQuery(previous.scope, previous.id); + }; + useEffect(() => { + return () => { + requestGeneration.current += 1; + const previous = acceptedRun.current; + acceptedRun.current = null; + if (previous !== null) void cancelPlannerQuery(previous.scope, previous.id); + }; + }, []); const planner = useMutation({ - mutationFn: (nextQuery: string) => createPlannerQuery(scope, nextQuery), - onSuccess: (result) => { + mutationFn: async (request: PlannerRequest) => { + const result = await createPlannerQuery(request.scope, request.query); if (result.outcome === 'envelope') { - setActiveRunId(result.envelope.payload.run_id); + const run = result.envelope.payload; + const live = !runIsTerminal(run.state); + // Record ownership as soon as the daemon admits the request. The + // observer can unmount before React Query runs its success callbacks. + if (request.generation === requestGeneration.current) { + acceptedRun.current = live ? { id: run.run_id, scope: request.scope } : null; + setActiveRunId(run.run_id); + } else if (live) { + void cancelPlannerQuery(request.scope, run.run_id); + } } + return result; }, }); const activeRunIdForQuery = activeRunId ?? ''; @@ -278,6 +317,19 @@ export function useExplorerController(): ExplorerController { runResult?.outcome === 'envelope' && runResult.envelope.payload.request.query === submitted ? runResult.envelope.payload : undefined; + const currentRunId = run?.run_id; + const currentRunState = run?.state; + useEffect(() => { + // A completed read no longer needs an explicit cancel on unmount/reset. + if ( + currentRunId !== undefined && + currentRunState !== undefined && + runIsTerminal(currentRunState) && + acceptedRun.current?.id === currentRunId + ) { + acceptedRun.current = null; + } + }, [currentRunId, currentRunState]); const graphBrowse = useEnvelope( ['explorer', 'graph-overview'], @@ -378,6 +430,8 @@ export function useExplorerController(): ExplorerController { }, [facet, laneFilter, lanes]); const dispatch = (nextQuery: string) => { + requestGeneration.current += 1; + retireAcceptedRun(); setActiveRunId(null); // A fresh run starts at the top of the backoff ladder: the previous // run's failures are not this one's, and must not slow its first reads. @@ -385,7 +439,9 @@ export function useExplorerController(): ExplorerController { planner.reset(); // Nothing is dispatched under a scope that will refuse it; the lanes // render the refusal instead. - if (writability.state === 'writable') planner.mutate(nextQuery); + if (writability.state === 'writable') { + planner.mutate({ query: nextQuery, scope, generation: requestGeneration.current }); + } }; // The rows on screen must belong to the scope in the register. When the @@ -407,6 +463,8 @@ export function useExplorerController(): ExplorerController { }, [requestScope]); const reset = () => { + requestGeneration.current += 1; + retireAcceptedRun(); setQuery(''); setSubmitted(''); setActiveRunId(null); diff --git a/dashboard/src/workspaces/explorer/controllerPolling.dom.test.tsx b/dashboard/src/workspaces/explorer/controllerPolling.dom.test.tsx index 90cbea0e59b..f8e6bc2f294 100644 --- a/dashboard/src/workspaces/explorer/controllerPolling.dom.test.tsx +++ b/dashboard/src/workspaces/explorer/controllerPolling.dom.test.tsx @@ -9,7 +9,7 @@ * either hammering or abandoning the run, and the poll ends with the surface , * by driving the real controller against a scripted daemon on a fake clock. */ -import { QueryClient, QueryClientProvider } from '@tanstack/react-query'; +import { MutationCache, QueryClient, QueryClientProvider } from '@tanstack/react-query'; import { act, renderHook } from '@testing-library/react'; import { createElement, type ReactNode } from 'react'; import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; @@ -24,7 +24,7 @@ const STATUS_PATH = `/api/explorer/queries/${RUN_ID}`; type RunState = 'pending' | 'completed'; -function runEnvelope(state: RunState) { +function runEnvelope(state: RunState, runId = RUN_ID, query = QUERY) { return { schema_revision: 1, scope: { @@ -52,8 +52,8 @@ function runEnvelope(state: RunState) { domain_state: state === 'completed' ? 'ready' : 'partial', legal_actions: [], payload: { - run_id: RUN_ID, - request: { query: QUERY, limit: 50, offset: 0 }, + run_id: runId, + request: { query, limit: 50, offset: 0 }, request_revision: 'explorer-query-request-v1', plan_revision: 'explorer-query-plan-v1', merge_revision: 'source-local-no-merge-v1', @@ -93,8 +93,11 @@ type StatusStep = */ function scriptedDaemon(script: readonly StatusStep[]) { const statusReads: StatusStep[] = []; - const fetchMock = vi.fn(async (input: RequestInfo | URL): Promise => { + const fetchMock = vi.fn(async (input: RequestInfo | URL, init?: RequestInit): Promise => { const url = String(input); + if (url.includes(STATUS_PATH) && init?.method === 'DELETE') { + return jsonResponse(200, runEnvelope('completed')); + } if (url.includes(STATUS_PATH)) { const step = script[Math.min(statusReads.length, script.length - 1)]; if (step === undefined) throw new Error('scripted daemon has no status step'); @@ -263,6 +266,32 @@ describe('explorer run-status polling', () => { expect(statusReads.length).toBe(afterUnmount); }); + it('cancels an accepted run when unmounted before mutation callbacks finish', async () => { + let releaseCallbacks!: () => void; + const pending = new Promise((resolve) => { releaseCallbacks = resolve; }); + const afterAdmission = vi.fn(() => pending); + const client = new QueryClient({ mutationCache: new MutationCache({ onSuccess: afterAdmission }) }); + const { fetchMock } = scriptedDaemon([{ kind: 'run', state: 'pending' }]); + vi.stubGlobal('fetch', fetchMock); + const view = renderHook(() => useExplorerController(), { + wrapper: ({ children }) => createElement(QueryClientProvider, { client }, children), + }); + try { + await act(async () => { + view.result.current.submit(QUERY); + await vi.advanceTimersByTimeAsync(0); + }); + expect(afterAdmission).toHaveBeenCalledOnce(); + view.unmount(); + expect(fetchMock.mock.calls.filter(([url, init]) => + String(url).endsWith(STATUS_PATH) && init?.method === 'DELETE', + )).toHaveLength(1); + } finally { + releaseCallbacks(); + client.clear(); + } + }); + it('stops polling when the search is reset', async () => { // Reset drops the active run, so the poll it was the only reader of ends // with it rather than surviving into the next search. @@ -284,3 +313,91 @@ describe('explorer run-status polling', () => { expect(statusReads.length).toBe(afterReset); }); }); + +/** A held admission POST can complete after a newer query or an unmount. + * The result must cancel its exact server run rather than reclaim the UI. */ +describe('explorer query admission ownership', () => { + it.each(['pending', 'completed'] as const)('keeps the newer query after a late %s admission', async (state) => { + let finishOlder!: (response: Response) => void; + const heldOlder = new Promise((resolve) => { + finishOlder = resolve; + }); + const cancellations: string[] = []; + const fetchMock = vi.fn((input: RequestInfo | URL, init?: RequestInit): Promise => { + const url = String(input); + if (init?.method === 'POST' && url.endsWith('/api/explorer/queries')) { + const { query } = JSON.parse(String(init.body)) as { query: string }; + if (query === 'older') return heldOlder; + return Promise.resolve(jsonResponse(202, runEnvelope('pending', 'run-newer', 'newer'))); + } + if (init?.method === 'DELETE') { + cancellations.push(url); + return Promise.resolve(jsonResponse(200, runEnvelope('completed', 'run-newer', 'newer'))); + } + if (url.endsWith('/api/explorer/queries/run-newer')) { + return Promise.resolve(jsonResponse(200, runEnvelope('pending', 'run-newer', 'newer'))); + } + return Promise.resolve(jsonResponse(404, { error: 'unknown run' })); + }); + vi.stubGlobal('fetch', fetchMock); + const view = renderHook(() => useExplorerController(), { wrapper }); + + await act(async () => { + view.result.current.submit('older'); + await vi.advanceTimersByTimeAsync(0); + }); + await act(async () => { + view.result.current.submit('newer'); + await vi.advanceTimersByTimeAsync(0); + }); + expect(view.result.current.run?.run_id).toBe('run-newer'); + + await act(async () => { + finishOlder(jsonResponse(202, runEnvelope(state, 'run-older', 'older'))); + await vi.advanceTimersByTimeAsync(0); + }); + expect(cancellations).toEqual(state === 'pending' ? ['/api/explorer/queries/run-older'] : []); + expect(view.result.current.run?.run_id).toBe('run-newer'); + view.unmount(); + }); + + it('cancels the old run on supersession and the live run on unmount', async () => { + const cancellations: string[] = []; + const fetchMock = vi.fn((input: RequestInfo | URL, init?: RequestInit): Promise => { + const url = String(input); + if (init?.method === 'POST' && url.endsWith('/api/explorer/queries')) { + const { query } = JSON.parse(String(init.body)) as { query: string }; + return Promise.resolve(jsonResponse(202, runEnvelope('pending', `run-${query}`, query))); + } + if (init?.method === 'DELETE') { + cancellations.push(url); + return Promise.resolve(jsonResponse(200, runEnvelope('completed'))); + } + if (url.includes('/api/explorer/queries/run-')) { + const query = url.substring(url.lastIndexOf('run-') + 4); + return Promise.resolve(jsonResponse(200, runEnvelope('pending', `run-${query}`, query))); + } + return Promise.resolve(jsonResponse(404, { error: 'unknown run' })); + }); + vi.stubGlobal('fetch', fetchMock); + const view = renderHook(() => useExplorerController(), { wrapper }); + + await act(async () => { + view.result.current.submit('first'); + await vi.advanceTimersByTimeAsync(0); + }); + expect(view.result.current.run?.run_id).toBe('run-first'); + await act(async () => { + view.result.current.submit('second'); + await vi.advanceTimersByTimeAsync(0); + }); + expect(cancellations).toEqual(['/api/explorer/queries/run-first']); + expect(view.result.current.run?.run_id).toBe('run-second'); + + view.unmount(); + expect(cancellations).toEqual([ + '/api/explorer/queries/run-first', + '/api/explorer/queries/run-second', + ]); + }); +});