diff --git a/propr-ui/e2e/list-search-lazy-navigation.pw.ts b/propr-ui/e2e/list-search-lazy-navigation.pw.ts new file mode 100644 index 000000000..31593c727 --- /dev/null +++ b/propr-ui/e2e/list-search-lazy-navigation.pw.ts @@ -0,0 +1,202 @@ +import { expect, test, type Locator, type Page } from '@playwright/test'; +import { capture, now, ago, tasks } from './task-list-desktop.fixture'; + +// A list search that is cleared just before the user leaves must not navigate back to the +// list while the destination's lazy route chunk is still loading. The departing list stays +// mounted during that transition, with its debounced search write still pending. + +const QUERY = 'propr-validation-no-match'; +// Longer than every list's search debounce (Tasks/Goals 400ms, Plans 300ms). +const PAST_DEBOUNCE_MS = 1_000; + +const goal = (id: string, title: string) => ({ + id, owner: 'operator', repository: 'integry/propr', title, objective: `${title} for the release.`, + launchStrategy: 'orchestrate', initialPrompt: `/goal ${title}`, attachments: [], + baseBranch: null, branchName: `goal/${id}`, worktreePath: `/tmp/${id}`, + agent: { id: 'agent-1', alias: 'codex', type: 'codex' }, + requestedModel: 'gpt-6-astra', effectiveModel: 'gpt-6-astra', + maxParallelTasks: 3, ultrafix: true, desiredState: 'running', resultState: null, + failureReason: null, pausePending: false, + control: { requestGeneration: 0, acknowledgedGeneration: 0, pending: false }, + taskId: `${id}-task`, sessionId: null, conversationId: null, finalPr: null, + checkpoint: null, artifacts: [], + artifactStats: { issues: 0, openIssues: 0, pullRequests: 0, openPullRequests: 0 }, + liveSummary: { currentTask: null, todos: [], tokenUsage: { input_tokens: 0, output_tokens: 0 }, nativeGoal: null }, + taskState: 'claude_execution', createdAt: ago(60), updatedAt: ago(5), + startedAt: ago(60), pausedAt: null, completedAt: null, + elapsedMs: 60_000, activeMs: 60_000, pausedMs: 0, +}); +const goals = [goal('goal-1', 'Launch Customer Analytics Dashboard'), goal('goal-2', 'Prepare Billing API')]; +const drafts = [ + { draft_id: 'draft-1', repository: 'integry/propr', name: 'Mobile search for list pages', initial_prompt: 'Mobile search', status: 'draft', updated_at: ago(30), created_at: ago(90) }, +]; + +const matches = (text: string, search: string | null) => !search || text.toLowerCase().includes(search.toLowerCase()); + +async function stub(page: Page) { + await page.clock.install({ time: now }); + await page.routeWebSocket('**/socket.io/**', socket => socket.close()); + await page.route('**/api/**', route => { + const url = new URL(route.request().url()); + const search = url.searchParams.get('search'); + const responses: Record = { + '/api/auth/demo-mode': { demoMode: false }, + '/api/auth/user': { id: 'user-1', login: 'operator', username: 'operator', displayName: 'Operations', email: null, avatarUrl: null, role: 'admin', permissions: [], authorizationSource: 'local' }, + '/api/instance/catalog': { agents: [{ id: 'agent-1', alias: 'codex', type: 'codex', enabled: true, models: ['gpt-6-astra'], defaultModel: 'gpt-6-astra' }], repositories: [{ name: 'integry/propr', enabled: true }] }, + '/api/tasks': (() => { + const found = tasks.filter(task => matches(task.title, search)); + return { tasks: found, total: found.length, totalRuns: found.length }; + })(), + '/api/stats/repositories': { repositories: [{ repository: 'integry/propr', total: 30, completed: 20, failed: 2, inProgress: 1, successRate: 80 }] }, + '/api/planner/drafts': (() => { + const found = drafts.filter(item => matches(item.name, search)); + return { drafts: found, total: found.length, page: 1, limit: 50, hasMore: false }; + })(), + '/api/planner/drafts/repositories': { repositories: [{ repo: 'integry/propr', count: 1 }], total: 1 }, + '/api/goals': { goals }, + '/api/queue/stats': { active: 0, waiting: 0, completed: 0, failed: 0 }, + '/api/stats/generating-plans': { count: 0 }, + '/api/notifications/unread-count': { unreadCount: 0 }, + '/api/notifications/preferences': { preferences: {}, quietHours: { start: null, end: null, timezone: 'UTC' }, badgeEnabled: false }, + }; + return url.pathname in responses + ? route.fulfill({ json: responses[url.pathname] }) + : route.fulfill({ status: 503, json: { error: 'Unavailable in lazy navigation fixture' } }); + }); +} + +/** + * Holds the real built Settings route chunk until `release` is called, then lets the + * original, unchanged response through. + */ +async function holdSettingsChunk(page: Page) { + let release!: () => void; + const released = new Promise(resolve => { release = resolve; }); + let requested!: () => void; + const wasRequested = new Promise(resolve => { requested = resolve; }); + await page.route(/\/assets\/SettingsPage-[\w-]+\.js$/, async route => { + requested(); + await released; + await route.continue(); + }); + return { wasRequested, release }; +} + +/** Stops the page's timers (the installed clock) from here until the test advances them. */ +const pauseTimers = async (page: Page) => page.clock.pauseAt(await page.evaluate(() => Date.now()) + 50); + +const routePath = (page: Page) => page.evaluate(() => window.location.pathname); +const routeSearch = (page: Page) => page.evaluate(() => window.location.search); + +// Goals and Plans write a cleared search at once; Tasks writes it through its debounce. +const lists = [ + { name: 'Tasks', path: '/tasks', label: 'Search tasks', clearsAtOnce: false }, + { name: 'Plans', path: '/plans', label: 'Search plans', clearsAtOnce: true }, + { name: 'Goals', path: '/goals', label: 'Search goals', clearsAtOnce: true }, +] as const; + +const viewports = [ + { name: '320', width: 320, height: 640, phone: true }, + { name: '390', width: 390, height: 844, phone: true }, + { name: 'desktop', width: 1280, height: 800, phone: false }, +] as const; + +// The web build's BrowserRouter. The native HashRouter only exists in the desktop +// renderer build; hooks.lazyRoute.test.tsx covers it with the same lazy transition. +for (const viewport of viewports) { + test.describe(`${viewport.name}`, () => { + test.use({ hasTouch: viewport.phone, isMobile: viewport.phone }); + test.beforeEach(async ({ page }) => { + await page.setViewportSize({ width: viewport.width, height: viewport.height }); + await stub(page); + }); + + const press = async (locator: Locator) => (viewport.phone ? locator.tap() : locator.click()); + const openSettings = async (page: Page) => { + if (viewport.phone) { + await press(page.getByRole('navigation', { name: 'Primary navigation' }).getByRole('button', { name: 'More' })); + await press(page.getByRole('dialog', { name: 'ProPR', exact: true }).getByRole('link', { name: 'Settings', exact: true })); + } else { + await press(page.locator('aside').first().getByRole('link', { name: 'Settings', exact: true })); + } + }; + const visit = async (page: Page, path: string) => { + await page.goto(path); + await expect.poll(() => routePath(page)).toBe(path); + }; + + /** Leaves for Settings while its chunk is held, and checks the list never takes the URL back. */ + /** + * Leaves for Settings while its chunk is held, with the page's timers paused since the + * list edit, so a pending debounce is certain to still be queued when the route changes. + */ + const leaveForDelayedSettings = async (page: Page, list: typeof lists[number], chunk: Awaited>, preview?: string) => { + await openSettings(page); + await chunk.wasRequested; + await expect.poll(() => routePath(page)).toBe('/settings'); + // Every list search debounce elapses while the destination is still loading. + await page.clock.runFor(PAST_DEBOUNCE_MS); + expect(await routePath(page)).toBe('/settings'); + await page.clock.resume(); + + chunk.release(); + await expect(page.getByRole('heading', { name: 'Settings', exact: true }).first()).toBeVisible(); + expect(await routePath(page)).toBe('/settings'); + await page.waitForTimeout(PAST_DEBOUNCE_MS); + expect(await routePath(page)).toBe('/settings'); + await expect(page.getByRole('textbox', { name: list.label })).toHaveCount(0); + if (preview) await capture(page, preview); + + // Back still returns to the list, without the cleared search. + await page.goBack(); + await expect.poll(() => routePath(page)).toBe(list.path); + await expect(page.getByRole('textbox', { name: list.label })).toHaveValue(''); + expect(await routeSearch(page)).not.toContain('search='); + }; + + for (const list of lists) { + test(`${list.name}: a cleared search does not pull navigation back while Settings loads`, async ({ page }) => { + const chunk = await holdSettingsChunk(page); + await visit(page, list.path); + const search = page.getByRole('textbox', { name: list.label }); + await expect(search).toBeVisible(); + + await press(search); + await page.keyboard.type(QUERY); + await expect.poll(() => routeSearch(page)).toContain(`search=${QUERY}`); + // Anything the clear leaves queued (Goals re-commits its debounced value) is still pending on leaving. + if (list.clearsAtOnce) await pauseTimers(page); + await press(page.getByRole('button', { name: 'Clear search', exact: true })); + await expect(search).toHaveValue(''); + await expect(search).toBeFocused(); + await expect.poll(() => routeSearch(page)).not.toContain('search='); + if (!list.clearsAtOnce) await pauseTimers(page); + + await leaveForDelayedSettings(page, list, chunk, list.name === 'Goals' ? `lazy-settings-after-goals-clear-${viewport.name}` : undefined); + }); + + test(`${list.name}: a typed search still pending does not pull navigation back while Settings loads`, async ({ page }) => { + const chunk = await holdSettingsChunk(page); + await visit(page, list.path); + const search = page.getByRole('textbox', { name: list.label }); + await expect(search).toBeVisible(); + + // Leave before the debounce commits the query. The departing list must drop it. + await press(search); + await pauseTimers(page); + await page.keyboard.type(QUERY); + await expect(search).toHaveValue(QUERY); + expect(await routeSearch(page)).not.toContain('search='); + await leaveForDelayedSettings(page, list, chunk); + }); + } + + test('control: with no list edit, Settings still loads behind the held chunk', async ({ page }) => { + const chunk = await holdSettingsChunk(page); + await visit(page, '/goals'); + await expect(page.getByRole('textbox', { name: 'Search goals' })).toBeVisible(); + await pauseTimers(page); + await leaveForDelayedSettings(page, lists[2], chunk); + }); + }); +} diff --git a/propr-ui/src/components/TaskList.tsx b/propr-ui/src/components/TaskList.tsx index b821f7a4f..750f5d015 100644 --- a/propr-ui/src/components/TaskList.tsx +++ b/propr-ui/src/components/TaskList.tsx @@ -21,7 +21,7 @@ import { groupTasksForDisplay, selectValue, } from './TaskList/utils'; -import { useDebouncedCallback, useTaskSearchParams } from './TaskList/hooks'; +import { useDebouncedCallback, useListSearchParams } from './TaskList/hooks'; import { isDialogOpen, isTypingTarget } from './TaskList/keyboardOwnership'; import { useLiveRefreshScheduler } from '../hooks/useLiveRefreshScheduler'; import type { TaskUpdatePayload } from '@propr/shared'; @@ -141,7 +141,7 @@ function useRowKeyboardNavigation( } const TaskList: React.FC = ({ limit, showViewAll = false, hideFilters = false, selectedTaskId = null, onSelectTask, refreshKey = 0, onGroupsChange }) => { - const [searchParams, setSearchParams] = useTaskSearchParams(); + const [searchParams, setSearchParams] = useListSearchParams(); const navigate = useNavigate(); const { onTaskUpdate, isConnected } = useSocket(); const currentUser = useCurrentUser(); diff --git a/propr-ui/src/components/TaskList/hooks.lazyRoute.test.tsx b/propr-ui/src/components/TaskList/hooks.lazyRoute.test.tsx new file mode 100644 index 000000000..ff6e66a10 --- /dev/null +++ b/propr-ui/src/components/TaskList/hooks.lazyRoute.test.tsx @@ -0,0 +1,112 @@ +import { act, fireEvent, render, screen } from '@testing-library/react'; +import React, { Suspense, lazy, useCallback, useState } from 'react'; +import { BrowserRouter, HashRouter, Link, Route, Routes } from 'react-router-dom'; +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; +import { useDebouncedCallback, useListSearchParams } from './hooks'; + +// A departing list stays mounted while a lazy destination route loads (React Router +// renders navigations in a transition). Its debounced search write must not take the +// URL back to the list. + +/** The Goals/Plans/Tasks search shape: instant input, debounced URL write, immediate clear. */ +const SearchList: React.FC = () => { + const [params, setParams] = useListSearchParams(); + const [query, setQuery] = useState(params.get('search') ?? ''); + const commit = useCallback((value: string) => { + setParams(current => { + const next = new URLSearchParams(current); + if (value) next.set('search', value); + else next.delete('search'); + return next; + }, { replace: true }); + }, [setParams]); + useDebouncedCallback(query, commit, 400); + return ( + <> +

List

+ setQuery(event.target.value)} /> + + Settings + + ); +}; + +const routerLocation = (router: 'browser' | 'hash') => { + if (router === 'hash') return window.location.hash.slice(1) || '/'; + return `${window.location.pathname}${window.location.search}`; +}; + +const renderApp = (router: 'browser' | 'hash') => { + let release!: () => void; + const chunk = new Promise<{ default: React.FC }>(resolve => { + release = () => resolve({ default: () =>

Settings

}); + }); + const Settings = lazy(() => chunk); + const Router = router === 'hash' ? HashRouter : BrowserRouter; + render( + + Loading

}> + + } /> + } /> + +
+
, + ); + return { release: () => act(async () => { release(); await chunk; }) }; +}; + +describe.each(['browser', 'hash'] as const)('list search during a lazy route transition (%s router)', router => { + beforeEach(() => { + vi.useFakeTimers({ shouldAdvanceTime: true }); + window.history.replaceState(null, '', router === 'hash' ? '/#/goals' : '/goals'); + }); + afterEach(() => { + vi.useRealTimers(); + window.history.replaceState(null, '', '/'); + }); + + const leaveWhileSettingsLoads = async (release: () => Promise) => { + fireEvent.click(screen.getByRole('link', { name: 'Settings' })); + expect(routerLocation(router)).toBe('/settings'); + // The departing list is still on screen, and every debounce elapses. + expect(screen.getByRole('heading', { name: 'List' })).toBeTruthy(); + await act(async () => { vi.advanceTimersByTime(1_000); }); + expect(routerLocation(router)).toBe('/settings'); + + await release(); + expect(await screen.findByRole('heading', { name: 'Settings' })).toBeTruthy(); + await act(async () => { vi.advanceTimersByTime(1_000); }); + expect(routerLocation(router)).toBe('/settings'); + }; + + it('a cleared search does not navigate back to the list', async () => { + const { release } = renderApp(router); + fireEvent.change(screen.getByRole('textbox', { name: 'Search' }), { target: { value: 'no-match' } }); + await act(async () => { vi.advanceTimersByTime(400); }); + expect(routerLocation(router)).toBe('/goals?search=no-match'); + + fireEvent.click(screen.getByRole('button', { name: 'Clear search' })); + expect(routerLocation(router)).toBe('/goals'); + await leaveWhileSettingsLoads(release); + }); + + it('a typed search still pending does not navigate back to the list', async () => { + const { release } = renderApp(router); + fireEvent.change(screen.getByRole('textbox', { name: 'Search' }), { target: { value: 'no-match' } }); + await leaveWhileSettingsLoads(release); + }); + + it('control: with no edit, the delayed destination opens', async () => { + const { release } = renderApp(router); + await leaveWhileSettingsLoads(release); + }); + + it('still writes the search while the list stays open', async () => { + renderApp(router); + fireEvent.change(screen.getByRole('textbox', { name: 'Search' }), { target: { value: 'billing' } }); + expect(routerLocation(router)).toBe('/goals'); + await act(async () => { vi.advanceTimersByTime(400); }); + expect(routerLocation(router)).toBe('/goals?search=billing'); + }); +}); diff --git a/propr-ui/src/components/TaskList/hooks.ts b/propr-ui/src/components/TaskList/hooks.ts index 9b41e5f45..5640c6910 100644 --- a/propr-ui/src/components/TaskList/hooks.ts +++ b/propr-ui/src/components/TaskList/hooks.ts @@ -2,11 +2,13 @@ import { useCallback, useContext, useEffect, useLayoutEffect, useRef } from 'rea import { UNSAFE_NavigationContext, createSearchParams, useLocation, useNavigate, useSearchParams, type Location, type Navigator, type SetURLSearchParams } from 'react-router-dom'; /** - * Task filters and selection share this write boundary. React Router's normal - * functional setter reads render-time params, not the current history entry. - * History can already have advanced while a transition is still rendering. + * Task, Plan and Goal filters and selection share this write boundary. React + * Router's normal functional setter reads render-time params, not the current + * history entry. History can already have advanced while a transition is still + * rendering, e.g. while a lazy destination route loads and the departing list + * stays mounted with its debounced search still pending. */ -export function useTaskSearchParams(): ReturnType { +export function useListSearchParams(): ReturnType { const [params] = useSearchParams(); const origin = useLocation(); const navigate = useNavigate(); diff --git a/propr-ui/src/hooks/useInboxSelection.test.tsx b/propr-ui/src/hooks/useInboxSelection.test.tsx index ee2a2dd3b..be01f142f 100644 --- a/propr-ui/src/hooks/useInboxSelection.test.tsx +++ b/propr-ui/src/hooks/useInboxSelection.test.tsx @@ -6,7 +6,7 @@ import { useInboxSelection } from './useInboxSelection'; // The write boundary refuses every write, as it does once history has moved on before the URL write. vi.mock('../components/TaskList/hooks', () => ({ - useTaskSearchParams: () => [useSearchParams()[0], () => undefined], + useListSearchParams: () => [useSearchParams()[0], () => undefined], })); const wrapper = ({ children }: { children: React.ReactNode }) => ( diff --git a/propr-ui/src/hooks/useInboxSelection.ts b/propr-ui/src/hooks/useInboxSelection.ts index a6cd4004c..09ff5d224 100644 --- a/propr-ui/src/hooks/useInboxSelection.ts +++ b/propr-ui/src/hooks/useInboxSelection.ts @@ -1,6 +1,6 @@ import { useCallback, useEffect, useRef, useState } from 'react'; import { useLocation } from 'react-router-dom'; -import { useTaskSearchParams } from '../components/TaskList/hooks'; +import { useListSearchParams } from '../components/TaskList/hooks'; /** Tailwind `xl`: wide enough to show the queue and a triage verdict side by side. */ export const INBOX_SPLIT_QUERY = '(min-width: 1280px)'; @@ -23,7 +23,7 @@ export interface InboxSelection { * through the queue with j/k does not fill the back button. */ export function useInboxSelection(): InboxSelection { - const [searchParams, setSearchParams] = useTaskSearchParams(); + const [searchParams, setSearchParams] = useListSearchParams(); const urlId = searchParams.get('notification') || null; // The router applies a URL change inside a transition, so for a moment the // URL is ahead of what renders. The latest choice is held here until the diff --git a/propr-ui/src/hooks/useTaskSelection.ts b/propr-ui/src/hooks/useTaskSelection.ts index c9a48623c..dcc727f38 100644 --- a/propr-ui/src/hooks/useTaskSelection.ts +++ b/propr-ui/src/hooks/useTaskSelection.ts @@ -1,5 +1,5 @@ import { useCallback, useEffect, useState } from 'react'; -import { useTaskSearchParams } from '../components/TaskList/hooks'; +import { useListSearchParams } from '../components/TaskList/hooks'; /** Tailwind `xl`: wide enough to show the task list and a task side by side. */ export const TASK_SPLIT_QUERY = '(min-width: 1280px)'; @@ -22,7 +22,7 @@ export interface TaskSelection { * stepping through tasks with j/k does not fill the back button. */ export function useTaskSelection(): TaskSelection { - const [searchParams, setSearchParams] = useTaskSearchParams(); + const [searchParams, setSearchParams] = useListSearchParams(); const urlTaskId = searchParams.get('task') || null; // The router applies a URL change inside a transition, so for a moment the // URL is ahead of what renders. The latest choice is held here until the diff --git a/propr-ui/src/pages/GoalsPage.tsx b/propr-ui/src/pages/GoalsPage.tsx index 982fc2651..0446f9129 100644 --- a/propr-ui/src/pages/GoalsPage.tsx +++ b/propr-ui/src/pages/GoalsPage.tsx @@ -3,7 +3,7 @@ import { CreationDialog } from '../components/CreationDialog'; import { PreviewThumbnails } from '../components/PreviewMedia'; /* eslint-disable max-lines -- goal list and split-pane console intentionally share this route-level surface */ import React, { useCallback, useEffect, useMemo, useRef, useState } from 'react'; -import { Link, useNavigate, useParams, useSearchParams } from 'react-router-dom'; +import { Link, useNavigate, useParams } from 'react-router-dom'; import { Activity, AlertTriangle, Check, CheckCircle2, CircleDot, CirclePause, CirclePlay, CircleSlash, CircleStop, Copy, ExternalLink, FileText, Filter, GitPullRequest, LoaderCircle, Send, @@ -17,7 +17,7 @@ import { getGoalAttachmentUrl, type Goal, type GoalCapability, type GoalLaunchStrategy, type GoalVisualPreview, } from '../api/goals'; -import { useDebouncedCallback } from '../components/TaskList/hooks'; +import { useDebouncedCallback, useListSearchParams } from '../components/TaskList/hooks'; import { useTaskLiveData } from '../components/TaskDetails/useTaskLiveData'; import TodoList from '../components/TaskDetails/TodoList'; import ExecutionEventLog from '../components/TaskDetails/ExecutionEventLog'; @@ -631,7 +631,7 @@ function GoalQueueRow({ goal, goalAgents }: { goal: Goal; goalAgents: Array<{ ty function GoalList() { const navigate = useNavigate(); const isMobile = useIsMobile(); - const [searchParams, setSearchParams] = useSearchParams(); + const [searchParams, setSearchParams] = useListSearchParams(); const [isCreating, setIsCreating] = useState(false); const repositoryFilter = searchParams.get('repository') || 'all'; const statusFilter = searchParams.get('status') || 'all'; diff --git a/propr-ui/src/pages/PlansPage.tsx b/propr-ui/src/pages/PlansPage.tsx index 2889a0467..e40b2bf4a 100644 --- a/propr-ui/src/pages/PlansPage.tsx +++ b/propr-ui/src/pages/PlansPage.tsx @@ -1,6 +1,7 @@ // CI trigger: 2026-02-01 import React, { useEffect, useState, useMemo, useCallback, useRef } from 'react'; -import { useSearchParams, useNavigate } from 'react-router-dom'; +import { useNavigate } from 'react-router-dom'; +import { useListSearchParams } from '../components/TaskList/hooks'; import { useDocumentTitle } from '../hooks/useDocumentTitle'; import { getDrafts, deleteDraft, abortGeneration, DraftListItem, getDraftRepositories } from '../api/proprApi'; import { Filter } from 'lucide-react'; @@ -21,7 +22,7 @@ const PlansPage: React.FC = () => { useDocumentTitle('Plans'); const navigate = useNavigate(); const isMobile = useIsMobile(); - const [searchParams, setSearchParams] = useSearchParams(); + const [searchParams, setSearchParams] = useListSearchParams(); const { onDraftUpdate, onActivityUpdate, subscribeToActivity, unsubscribeFromActivity, isConnected } = useSocket(); // Derive state from URL parameters