Skip to content

Commit dad58b2

Browse files
fix(agentex-ui): merge URL updates against the live URL to fix account-switch races
updateParams rebuilt the query string from the useSearchParams() snapshot, which lags a render behind router.push. On an account switch two navigations fire — the provider clears task_id, the agent-validation effect clears agent_name — and whichever read the stale snapshot landed last, clobbering the other: intermittently resurrecting task_id, dropping agent_name, or reverting account_id while the sidebar already showed the new account's tasks. Merge each update against window.location.search (always current) so concurrent writes compose instead of overwrite, and clear task_id + agent_name atomically in the switch itself so it's a single authoritative navigation. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
1 parent 824286c commit dad58b2

3 files changed

Lines changed: 111 additions & 12 deletions

File tree

agentex-ui/components/providers/agentex-provider.tsx

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -73,9 +73,15 @@ export function AgentexProvider({
7373
updateParams(
7474
{
7575
[SearchParamKey.SGP_ACCOUNT_ID]: id,
76-
// An explicit switch (not bootstrap, which passes replace) drops the open task:
77-
// it's account-scoped and won't resolve under the new account.
78-
...(replace ? {} : { [SearchParamKey.TASK_ID]: null }),
76+
// An explicit switch (not bootstrap, which passes replace) drops the open task and
77+
// selected agent in one navigation: both are account-scoped and won't resolve under
78+
// the new account. Clearing them here keeps the switch a single atomic URL write.
79+
...(replace
80+
? {}
81+
: {
82+
[SearchParamKey.TASK_ID]: null,
83+
[SearchParamKey.AGENT_NAME]: null,
84+
}),
7985
},
8086
replace
8187
);
Lines changed: 83 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,83 @@
1+
import { renderHook } from '@testing-library/react';
2+
import { afterEach, describe, expect, it, vi } from 'vitest';
3+
4+
import { SearchParamKey, useSafeSearchParams } from './use-safe-search-params';
5+
6+
// Router spies + a mutable snapshot the mocked `useSearchParams` returns. Created via
7+
// vi.hoisted so the hoisted vi.mock factory can reference them.
8+
const mocks = vi.hoisted(() => ({
9+
push: vi.fn<(url: string) => void>(),
10+
replace: vi.fn<(url: string) => void>(),
11+
snapshot: new URLSearchParams(''),
12+
}));
13+
14+
vi.mock('next/navigation', () => ({
15+
useRouter: () => ({ push: mocks.push, replace: mocks.replace }),
16+
usePathname: () => '/',
17+
useSearchParams: () => mocks.snapshot,
18+
}));
19+
20+
function setLiveUrl(search: string) {
21+
window.history.replaceState(null, '', `${window.location.origin}/${search}`);
22+
}
23+
24+
function pushedQuery() {
25+
expect(mocks.push).toHaveBeenCalledTimes(1);
26+
const url = mocks.push.mock.calls[0]![0];
27+
return new URLSearchParams(url.split('?')[1] ?? '');
28+
}
29+
30+
afterEach(() => {
31+
mocks.push.mockClear();
32+
mocks.replace.mockClear();
33+
});
34+
35+
describe('useSafeSearchParams.updateParams', () => {
36+
it('merges against the live URL, not a stale snapshot (no write-clobber)', () => {
37+
// The live URL already reflects a committed account switch: new account, task_id dropped.
38+
setLiveUrl('?account_id=new&agent_name=dua-agent');
39+
// But the React snapshot still shows the pre-switch state (the lag that caused the bug).
40+
mocks.snapshot = new URLSearchParams(
41+
'account_id=old&task_id=T&agent_name=dua-agent'
42+
);
43+
44+
const { result } = renderHook(() => useSafeSearchParams());
45+
// A second writer (the agent-validation effect) clears only agent_name.
46+
result.current.updateParams({ [SearchParamKey.AGENT_NAME]: null });
47+
48+
// It must build from the live URL: keep account_id=new, leave task_id dropped — not
49+
// resurrect account_id=old / task_id=T from the stale snapshot.
50+
const qs = pushedQuery();
51+
expect(qs.get('account_id')).toBe('new');
52+
expect(qs.get('task_id')).toBeNull();
53+
expect(qs.get('agent_name')).toBeNull();
54+
});
55+
56+
it('sets, deletes, and preserves unlisted params in one update', () => {
57+
setLiveUrl('?account_id=acc&agent_name=keep');
58+
mocks.snapshot = new URLSearchParams('account_id=acc&agent_name=keep');
59+
60+
const { result } = renderHook(() => useSafeSearchParams());
61+
result.current.updateParams({
62+
[SearchParamKey.TASK_ID]: 'new-task',
63+
[SearchParamKey.AGENT_NAME]: null,
64+
});
65+
66+
const qs = pushedQuery();
67+
expect(qs.get('task_id')).toBe('new-task'); // set
68+
expect(qs.get('agent_name')).toBeNull(); // deleted
69+
expect(qs.get('account_id')).toBe('acc'); // preserved (unlisted)
70+
});
71+
72+
it('uses router.replace when replace=true', () => {
73+
setLiveUrl('?account_id=acc');
74+
mocks.snapshot = new URLSearchParams('account_id=acc');
75+
76+
const { result } = renderHook(() => useSafeSearchParams());
77+
result.current.updateParams({ [SearchParamKey.TASK_ID]: 'x' }, true);
78+
79+
expect(mocks.push).not.toHaveBeenCalled();
80+
expect(mocks.replace).toHaveBeenCalledTimes(1);
81+
expect(mocks.replace.mock.calls[0]![0]).toContain('task_id=x');
82+
});
83+
});

agentex-ui/hooks/use-safe-search-params.ts

Lines changed: 19 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -31,23 +31,33 @@ export function useSafeSearchParams(): SafeSearchParams {
3131

3232
const updateParams = useCallback(
3333
(updates: SearchParamUpdates, replace: boolean = false): void => {
34-
const params = new URLSearchParams(searchParams.toString());
34+
// Merge against the live URL, not the `searchParams` snapshot — the snapshot lags a
35+
// render behind router navigations, so two updates firing before it catches up (e.g. an
36+
// account switch clearing task_id while an effect clears agent_name) would each rebuild
37+
// from the stale query and the last push would clobber the other, resurrecting task_id
38+
// or reverting account_id.
39+
const live =
40+
typeof window !== 'undefined'
41+
? window.location.search
42+
: searchParams.toString();
43+
const params = new URLSearchParams(live);
3544

3645
Object.entries(updates).forEach(([key, value]) => {
3746
const paramKey = key as SearchParamKey;
38-
if (value !== undefined) {
39-
if (value === null) {
40-
params.delete(paramKey);
41-
} else {
42-
params.set(paramKey, value);
43-
}
47+
if (value === undefined) return;
48+
if (value === null) {
49+
params.delete(paramKey);
50+
} else {
51+
params.set(paramKey, value);
4452
}
4553
});
4654

55+
const query = params.toString();
56+
const url = query ? `${pathname}?${query}` : pathname;
4757
if (replace) {
48-
router.replace(`${pathname}?${params.toString()}`);
58+
router.replace(url);
4959
} else {
50-
router.push(`${pathname}?${params.toString()}`);
60+
router.push(url);
5161
}
5262
},
5363
[pathname, searchParams, router]

0 commit comments

Comments
 (0)