diff --git a/agentex-ui/components/providers/agentex-provider.tsx b/agentex-ui/components/providers/agentex-provider.tsx index 6b71d1a1..04b43bbb 100644 --- a/agentex-ui/components/providers/agentex-provider.tsx +++ b/agentex-ui/components/providers/agentex-provider.tsx @@ -73,9 +73,14 @@ export function AgentexProvider({ updateParams( { [SearchParamKey.SGP_ACCOUNT_ID]: id, - // An explicit switch (not bootstrap, which passes replace) drops the open task: - // it's account-scoped and won't resolve under the new account. - ...(replace ? {} : { [SearchParamKey.TASK_ID]: null }), + // Explicit switch (bootstrap passes replace): drop the account-scoped task + agent + // in one navigation so they don't linger under the new account. + ...(replace + ? {} + : { + [SearchParamKey.TASK_ID]: null, + [SearchParamKey.AGENT_NAME]: null, + }), }, replace ); diff --git a/agentex-ui/hooks/use-safe-search-params.test.tsx b/agentex-ui/hooks/use-safe-search-params.test.tsx new file mode 100644 index 00000000..6c736be9 --- /dev/null +++ b/agentex-ui/hooks/use-safe-search-params.test.tsx @@ -0,0 +1,83 @@ +import { renderHook } from '@testing-library/react'; +import { afterEach, describe, expect, it, vi } from 'vitest'; + +import { SearchParamKey, useSafeSearchParams } from './use-safe-search-params'; + +// Router spies + a mutable snapshot the mocked `useSearchParams` returns. Created via +// vi.hoisted so the hoisted vi.mock factory can reference them. +const mocks = vi.hoisted(() => ({ + push: vi.fn<(url: string) => void>(), + replace: vi.fn<(url: string) => void>(), + snapshot: new URLSearchParams(''), +})); + +vi.mock('next/navigation', () => ({ + useRouter: () => ({ push: mocks.push, replace: mocks.replace }), + usePathname: () => '/', + useSearchParams: () => mocks.snapshot, +})); + +function setLiveUrl(search: string) { + window.history.replaceState(null, '', `${window.location.origin}/${search}`); +} + +function pushedQuery() { + expect(mocks.push).toHaveBeenCalledTimes(1); + const url = mocks.push.mock.calls[0]![0]; + return new URLSearchParams(url.split('?')[1] ?? ''); +} + +afterEach(() => { + mocks.push.mockClear(); + mocks.replace.mockClear(); +}); + +describe('useSafeSearchParams.updateParams', () => { + it('merges against the live URL, not a stale snapshot (no write-clobber)', () => { + // The live URL already reflects a committed account switch: new account, task_id dropped. + setLiveUrl('?account_id=new&agent_name=dua-agent'); + // But the React snapshot still shows the pre-switch state (the lag that caused the bug). + mocks.snapshot = new URLSearchParams( + 'account_id=old&task_id=T&agent_name=dua-agent' + ); + + const { result } = renderHook(() => useSafeSearchParams()); + // A second writer (the agent-validation effect) clears only agent_name. + result.current.updateParams({ [SearchParamKey.AGENT_NAME]: null }); + + // It must build from the live URL: keep account_id=new, leave task_id dropped — not + // resurrect account_id=old / task_id=T from the stale snapshot. + const qs = pushedQuery(); + expect(qs.get('account_id')).toBe('new'); + expect(qs.get('task_id')).toBeNull(); + expect(qs.get('agent_name')).toBeNull(); + }); + + it('sets, deletes, and preserves unlisted params in one update', () => { + setLiveUrl('?account_id=acc&agent_name=keep'); + mocks.snapshot = new URLSearchParams('account_id=acc&agent_name=keep'); + + const { result } = renderHook(() => useSafeSearchParams()); + result.current.updateParams({ + [SearchParamKey.TASK_ID]: 'new-task', + [SearchParamKey.AGENT_NAME]: null, + }); + + const qs = pushedQuery(); + expect(qs.get('task_id')).toBe('new-task'); // set + expect(qs.get('agent_name')).toBeNull(); // deleted + expect(qs.get('account_id')).toBe('acc'); // preserved (unlisted) + }); + + it('uses router.replace when replace=true', () => { + setLiveUrl('?account_id=acc'); + mocks.snapshot = new URLSearchParams('account_id=acc'); + + const { result } = renderHook(() => useSafeSearchParams()); + result.current.updateParams({ [SearchParamKey.TASK_ID]: 'x' }, true); + + expect(mocks.push).not.toHaveBeenCalled(); + expect(mocks.replace).toHaveBeenCalledTimes(1); + expect(mocks.replace.mock.calls[0]![0]).toContain('task_id=x'); + }); +}); diff --git a/agentex-ui/hooks/use-safe-search-params.ts b/agentex-ui/hooks/use-safe-search-params.ts index f6ba3fd5..dc7aaa8e 100644 --- a/agentex-ui/hooks/use-safe-search-params.ts +++ b/agentex-ui/hooks/use-safe-search-params.ts @@ -31,23 +31,30 @@ export function useSafeSearchParams(): SafeSearchParams { const updateParams = useCallback( (updates: SearchParamUpdates, replace: boolean = false): void => { - const params = new URLSearchParams(searchParams.toString()); + // Merge against the live URL, not the `searchParams` snapshot: the snapshot lags a + // render behind router navigations, so concurrent updates would clobber each other. + const live = + typeof window !== 'undefined' + ? window.location.search + : searchParams.toString(); + const params = new URLSearchParams(live); Object.entries(updates).forEach(([key, value]) => { const paramKey = key as SearchParamKey; - if (value !== undefined) { - if (value === null) { - params.delete(paramKey); - } else { - params.set(paramKey, value); - } + if (value === undefined) return; + if (value === null) { + params.delete(paramKey); + } else { + params.set(paramKey, value); } }); + const query = params.toString(); + const url = query ? `${pathname}?${query}` : pathname; if (replace) { - router.replace(`${pathname}?${params.toString()}`); + router.replace(url); } else { - router.push(`${pathname}?${params.toString()}`); + router.push(url); } }, [pathname, searchParams, router]