Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 8 additions & 3 deletions agentex-ui/components/providers/agentex-provider.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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
);
Expand Down
83 changes: 83 additions & 0 deletions agentex-ui/hooks/use-safe-search-params.test.tsx
Original file line number Diff line number Diff line change
@@ -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');
});
});
25 changes: 16 additions & 9 deletions agentex-ui/hooks/use-safe-search-params.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Expand Down
Loading