Skip to content

Commit 92b9da3

Browse files
fix(ui): load all agents so the picker and deep-links work past the first page (#347)
### What - Page through the **full** agent list in `useAgents` instead of fetching only the default first page. - Add `useAgentByName` to validate a deep-linked `agent_name` directly, so a valid agent opens even if it's outside the loaded list. - Only clear `agent_name` when it's present *and* invalid; accept an agent found in the list **or** by name. - Cap the picker's chip grid height and let it scroll, so a long agent list doesn't push the title/prompt input off-screen. - Distinguish a real 404 (agent doesn't exist) from transient errors in the by-name lookup, and validate only against freshly-fetched data, so a blip or a mid-refetch never bounces a valid deep link. ### Why The picker fetched agents with an unpaginated `list()` call, so on accounts with more than one page of agents only the first ~50 showed. Those agents were invisible in the picker and because the same list backed deep-link validation, deep-linking to one bounced back to the home grid. Loading the full list then surfaced a layout issue where a tall picker overflowed a vertically-centered container with no way to scroll. ### Testing - Unit tests for the pagination loop (multi-page, short-page, empty, exact-boundary, safety-bound, mid-loop error) and the by-name lookup. - `npm run typecheck` + `npm run lint` clean. - Manually verified in a dev environment against an account with 100+ agents: all agents appear, the picker scrolls with the title/input fixed, and deep-linking a valid agent opens its chat. Before: https://github.com/user-attachments/assets/46f78616-f525-499f-8047-e7fbffd54910 After: https://github.com/user-attachments/assets/9a3b27ed-ba88-4d93-ae1f-0b2c7372f973 <!-- greptile_comment --> <h3>Greptile Summary</h3> This PR fixes the agent picker and deep-link routing by paginating through the full agent list (instead of stopping after the default first page), adding a `useAgentByName` hook for direct deep-link validation, and capping the picker's chip grid height to prevent layout overflow on large accounts. - **`useAgents`** now loops through all pages up to a `MAX_AGENT_PAGES` safety bound (100 × 100 = 10 000 agents max), warns loudly if truncated, and exports its constants so tests can import them rather than duplicating hardcoded values. - **`useAgentByName`** validates a deep-linked agent name directly against the backend: 404 resolves to `null`, any other error is re-thrown so React Query surfaces it as `isError`, and the effect in `agentex-ui-root.tsx` respects `couldNotDetermine` to avoid clearing a valid deep link on a transient failure. - **`AgentsList`** gains `max-h-[60vh] overflow-y-auto` so a long agent list scrolls within the centered container rather than pushing the title and input off-screen. <details><summary><h3>Confidence Score: 5/5</h3></summary> Safe to merge — all changed paths are well-tested, the catch narrowing and couldNotDetermine guard are correctly implemented, and the pagination loop has a clear termination condition. The pagination loop terminates reliably on a short page or at the safety bound, both hooks narrow their error handling correctly (404 → null, everything else re-thrown), and the validation effect is gated on both isFetching signals so it never acts on stale data. Unit tests cover all meaningful boundary cases. The previous review thread's concerns have been fully addressed. No files require special attention. </details> <details><summary><h3>Important Files Changed</h3></summary> | Filename | Overview | |----------|----------| | agentex-ui/hooks/use-agents.ts | Replaces single unpaginated list() call with an async loop that accumulates pages until a short page signals the end; exports PAGE_SIZE and MAX_AGENT_PAGES for test imports; adds safety-bound warning when truncation occurs. | | agentex-ui/hooks/use-agent-by-name.ts | New hook that validates a deep-linked agent name directly against the backend; narrows 404 to null (success) and re-throws all other errors so transient failures surface via isError rather than silently clearing the URL. | | agentex-ui/components/agentex-ui-root.tsx | Integrates useAgentByName alongside useAgents; gates validation on both isFetching signals; couldNotDetermine guard prevents clearing a valid deep link on transient API errors. | | agentex-ui/components/agents-list/agents-list.tsx | Single-line change adds max-h-[60vh] and overflow-y-auto to the chip grid container so a long agent list scrolls instead of overflowing the vertically-centered layout. | | agentex-ui/hooks/use-agents.test.tsx | New unit tests covering multi-page, short-page, empty-list, exact-multiple, safety-bound, and mid-loop error scenarios; imports PAGE_SIZE and MAX_AGENT_PAGES directly from source after previous review feedback. | | agentex-ui/hooks/use-agent-by-name.test.tsx | New unit tests covering disabled-when-no-name, successful lookup, 404→null, and non-404 error surfacing, using NotFoundError and APIError from the agentex SDK. | </details> <details><summary><h3>Flowchart</h3></summary> <a href="#gh-light-mode-only"> ```mermaid %%{init: {'theme': 'neutral'}}%% flowchart TD A[Page loads with agent_name in URL] --> B{isAgentsFetching\nOR isAgentByNameFetching?} B -- Yes --> C[Return early — wait for both queries to settle] B -- No --> D{agentName present\nin URL?} D -- No --> E{localAgentName\nin localStorage?} E -- Yes --> F[Restore: updateParams agent_name = localAgentName] E -- No --> G[No-op] D -- Yes --> H{Agent found\nin paginated list?} H -- Yes --> I[agentInList = agent] H -- No --> J{useAgentByName\nresult?} J -- Agent returned --> K[agentByName = Agent] J -- 404 → null --> L[agentByName = null] J -- Non-404 error --> M[isAgentByNameError = true\ncouldNotDetermine = true] I --> N{status === Ready?} K --> N L --> O[isAgentValid = false\ncouldNotDetermine = false] M --> P[Leave URL alone\npreserve deep link] N -- Yes --> Q[isAgentValid = true\nLeave URL alone — agent opens] N -- No --> O O --> R[Clear agent_name from URL\nsetLocalAgentName undefined] ``` </a> <a href="#gh-dark-mode-only"> ```mermaid %%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%% flowchart TD A[Page loads with agent_name in URL] --> B{isAgentsFetching\nOR isAgentByNameFetching?} B -- Yes --> C[Return early — wait for both queries to settle] B -- No --> D{agentName present\nin URL?} D -- No --> E{localAgentName\nin localStorage?} E -- Yes --> F[Restore: updateParams agent_name = localAgentName] E -- No --> G[No-op] D -- Yes --> H{Agent found\nin paginated list?} H -- Yes --> I[agentInList = agent] H -- No --> J{useAgentByName\nresult?} J -- Agent returned --> K[agentByName = Agent] J -- 404 → null --> L[agentByName = null] J -- Non-404 error --> M[isAgentByNameError = true\ncouldNotDetermine = true] I --> N{status === Ready?} K --> N L --> O[isAgentValid = false\ncouldNotDetermine = false] M --> P[Leave URL alone\npreserve deep link] N -- Yes --> Q[isAgentValid = true\nLeave URL alone — agent opens] N -- No --> O O --> R[Clear agent_name from URL\nsetLocalAgentName undefined] ``` </a> </details> <sub>Reviews (3): Last reviewed commit: ["Merge origin/main into vineetvora/agx1-3..."](433e6a2) | [Re-trigger Greptile](https://app.greptile.com/api/retrigger?id=42404585)</sub> <!-- /greptile_comment --> --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent 2ffde24 commit 92b9da3

6 files changed

Lines changed: 368 additions & 9 deletions

File tree

agentex-ui/components/agentex-ui-root.tsx

Lines changed: 27 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import { PrimaryContent } from '@/components/primary-content/primary-content';
88
import { useAgentexClient } from '@/components/providers';
99
import { TaskSidebar } from '@/components/task-sidebar/task-sidebar';
1010
import { TracesSidebar } from '@/components/traces-sidebar/traces-sidebar';
11+
import { useAgentByName } from '@/hooks/use-agent-by-name';
1112
import { useAgents } from '@/hooks/use-agents';
1213
import { useLocalStorageState } from '@/hooks/use-local-storage-state';
1314
import {
@@ -19,18 +20,39 @@ export function AgentexUIRoot() {
1920
const { agentName, taskID, updateParams } = useSafeSearchParams();
2021
const [isTracesSidebarOpen, setIsTracesSidebarOpen] = useState(false);
2122
const { agentexClient } = useAgentexClient();
22-
const { data: agents = [], isLoading } = useAgents(agentexClient);
23+
const { data: agents = [], isFetching: isAgentsFetching } =
24+
useAgents(agentexClient);
25+
// Validate the deep-linked agent directly against the backend so a valid agent opens
26+
// even if it sits outside the loaded list (e.g. on accounts with many agents).
27+
const {
28+
data: agentByName,
29+
isFetching: isAgentByNameFetching,
30+
isError: isAgentByNameError,
31+
} = useAgentByName(agentexClient, agentName);
2332
const [localAgentName, setLocalAgentName] = useLocalStorageState<
2433
string | undefined
2534
>('lastSelectedAgent', undefined);
2635

36+
// Wait until neither query is fetching before validating. We gate on `isFetching` (not
37+
// `isLoading`, which is false once a query has cached data) so we never validate against
38+
// stale data mid-refetch. A disabled by-name query reports `isFetching: false`, so it
39+
// doesn't block the localStorage-restore path when there's no agent_name. Deps are
40+
// intentionally narrowed so we re-validate on fetch settle / agent_name change.
2741
useEffect(() => {
28-
if (isLoading) return;
42+
if (isAgentsFetching || isAgentByNameFetching) return;
2943

30-
const selectedAgent = agents.find(agent => agent.name === agentName);
44+
// Accept an agent found in the (paginated) list OR resolved directly by name, so a valid
45+
// deep-linked agent opens even if it falls outside the loaded list.
46+
const agentInList = agents.find(agent => agent.name === agentName);
47+
const selectedAgent = agentInList ?? agentByName ?? undefined;
3148
const isAgentValid = selectedAgent && selectedAgent.status === 'Ready';
3249

33-
if (!isAgentValid) {
50+
// If the agent isn't in the loaded list and the by-name lookup errored (a transient
51+
// failure, not a 404), we can't tell whether it's valid — leave the URL alone rather
52+
// than bouncing a possibly-valid deep link to the home grid.
53+
const couldNotDetermine = !agentInList && isAgentByNameError;
54+
55+
if (agentName && !isAgentValid && !couldNotDetermine) {
3456
updateParams({ [SearchParamKey.AGENT_NAME]: null });
3557
setLocalAgentName(undefined);
3658
}
@@ -39,7 +61,7 @@ export function AgentexUIRoot() {
3961
updateParams({ [SearchParamKey.AGENT_NAME]: localAgentName });
4062
}
4163
// eslint-disable-next-line react-hooks/exhaustive-deps
42-
}, [isLoading]);
64+
}, [isAgentsFetching, isAgentByNameFetching, isAgentByNameError, agentName]);
4365

4466
const handleSelectTask = useCallback(
4567
(taskId: string | null) => {

agentex-ui/components/agents-list/agents-list.tsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,7 @@ export function AgentsList({ agents, isLoading = false }: AgentsListProps) {
5050
return (
5151
<TooltipProvider delayDuration={200}>
5252
<motion.div
53-
className="mb-2 flex max-w-4xl flex-wrap items-center justify-center gap-2"
53+
className="mb-2 flex max-h-[60vh] max-w-4xl flex-wrap items-center justify-center gap-2 overflow-y-auto"
5454
layout={hasMounted.current}
5555
>
5656
{displayedAgents.length > 0 ? (
Lines changed: 101 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,101 @@
1+
import type { ReactNode } from 'react';
2+
3+
import { QueryClient, QueryClientProvider } from '@tanstack/react-query';
4+
import { renderHook, waitFor } from '@testing-library/react';
5+
import { APIError, NotFoundError } from 'agentex';
6+
import { describe, expect, it, vi } from 'vitest';
7+
8+
import { useAgentByName } from './use-agent-by-name';
9+
10+
import type AgentexSDK from 'agentex';
11+
import type { Agent } from 'agentex/resources';
12+
13+
function createWrapper() {
14+
const queryClient = new QueryClient({
15+
defaultOptions: { queries: { retry: false } },
16+
});
17+
return function Wrapper({ children }: { children: ReactNode }) {
18+
return (
19+
<QueryClientProvider client={queryClient}>{children}</QueryClientProvider>
20+
);
21+
};
22+
}
23+
24+
function clientWith(retrieveByName: ReturnType<typeof vi.fn>) {
25+
return { agents: { retrieveByName } } as unknown as AgentexSDK;
26+
}
27+
28+
describe('useAgentByName', () => {
29+
it('does not fetch when no agent name is provided', () => {
30+
const retrieveByName = vi.fn();
31+
32+
const { result } = renderHook(
33+
() => useAgentByName(clientWith(retrieveByName), null),
34+
{
35+
wrapper: createWrapper(),
36+
}
37+
);
38+
39+
expect(retrieveByName).not.toHaveBeenCalled();
40+
expect(result.current.fetchStatus).toBe('idle');
41+
expect(result.current.data).toBeUndefined();
42+
});
43+
44+
it('returns the agent when the lookup succeeds', async () => {
45+
const agent = {
46+
id: 'a1',
47+
name: 'interview-agent',
48+
status: 'Ready',
49+
} as Agent;
50+
const retrieveByName = vi.fn().mockResolvedValueOnce(agent);
51+
52+
const { result } = renderHook(
53+
() => useAgentByName(clientWith(retrieveByName), 'interview-agent'),
54+
{ wrapper: createWrapper() }
55+
);
56+
57+
await waitFor(() => expect(result.current.isSuccess).toBe(true));
58+
59+
expect(result.current.data).toEqual(agent);
60+
expect(retrieveByName).toHaveBeenCalledWith('interview-agent');
61+
});
62+
63+
it('resolves to null (not an error) on a 404', async () => {
64+
const notFound = new NotFoundError(
65+
404,
66+
undefined,
67+
'Not found',
68+
new Headers()
69+
);
70+
const retrieveByName = vi.fn().mockRejectedValueOnce(notFound);
71+
72+
const { result } = renderHook(
73+
() => useAgentByName(clientWith(retrieveByName), 'missing-agent'),
74+
{ wrapper: createWrapper() }
75+
);
76+
77+
await waitFor(() => expect(result.current.isSuccess).toBe(true));
78+
79+
expect(result.current.data).toBeNull();
80+
expect(result.current.isError).toBe(false);
81+
});
82+
83+
it('surfaces a non-404 error instead of masquerading as not-found', async () => {
84+
const serverError = new APIError(
85+
500,
86+
undefined,
87+
'Server error',
88+
new Headers()
89+
);
90+
const retrieveByName = vi.fn().mockRejectedValueOnce(serverError);
91+
92+
const { result } = renderHook(
93+
() => useAgentByName(clientWith(retrieveByName), 'some-agent'),
94+
{ wrapper: createWrapper() }
95+
);
96+
97+
await waitFor(() => expect(result.current.isError).toBe(true));
98+
99+
expect(result.current.data).toBeUndefined();
100+
});
101+
});
Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,48 @@
1+
import { useQuery } from '@tanstack/react-query';
2+
import { APIError } from 'agentex';
3+
4+
import type AgentexSDK from 'agentex';
5+
import type { Agent } from 'agentex/resources';
6+
7+
export const agentByNameKeys = {
8+
byName: (name: string) => ['agents', 'by-name', name] as const,
9+
};
10+
11+
/**
12+
* Fetches a single agent by its unique name.
13+
*
14+
* Used to validate a deep-linked `agent_name` directly against the backend so that opening
15+
* an agent does not depend on the entire (paginated) agent list having been loaded first.
16+
* The query is disabled when no name is provided. A 404 resolves to `null` ("no such agent"
17+
* — a normal outcome), while any other failure (network / 5xx / auth) is re-thrown so React
18+
* Query surfaces it as an error rather than masquerading as "not found" (which could
19+
* otherwise clear a valid deep link).
20+
*
21+
* @param agentexClient - AgentexSDK - The SDK client used to communicate with the Agentex API
22+
* @param agentName - The agent name to look up; query is disabled when absent (null/undefined)
23+
* @returns UseQueryResult<Agent | null> - React Query result containing the agent, or null if not found
24+
*/
25+
export function useAgentByName(
26+
agentexClient: AgentexSDK,
27+
agentName: string | null | undefined
28+
) {
29+
return useQuery({
30+
queryKey: agentByNameKeys.byName(agentName ?? ''),
31+
queryFn: async (): Promise<Agent | null> => {
32+
try {
33+
return await agentexClient.agents.retrieveByName(agentName as string);
34+
} catch (error) {
35+
// A 404 means the name isn't a real, reachable agent — a normal "not found"
36+
// outcome, so resolve to null. Re-throw anything else (network / 5xx / auth) so a
37+
// transient failure surfaces as a query error instead of masquerading as
38+
// "not found", which could otherwise clear a valid deep link.
39+
if (error instanceof APIError && error.status === 404) {
40+
return null;
41+
}
42+
throw error;
43+
}
44+
},
45+
enabled: !!agentName,
46+
refetchOnWindowFocus: false,
47+
});
48+
}
Lines changed: 146 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,146 @@
1+
import type { ReactNode } from 'react';
2+
3+
import { QueryClient, QueryClientProvider } from '@tanstack/react-query';
4+
import { renderHook, waitFor } from '@testing-library/react';
5+
import { describe, expect, it, vi } from 'vitest';
6+
7+
import {
8+
AGENTS_PAGE_SIZE as PAGE_SIZE,
9+
MAX_AGENT_PAGES,
10+
useAgents,
11+
} from './use-agents';
12+
13+
import type AgentexSDK from 'agentex';
14+
import type { Agent } from 'agentex/resources';
15+
16+
function makeAgents(count: number, prefix: string): Agent[] {
17+
return Array.from(
18+
{ length: count },
19+
(_, i) =>
20+
({
21+
id: `${prefix}-${i}`,
22+
name: `${prefix}-${i}`,
23+
status: 'Ready',
24+
}) as Agent
25+
);
26+
}
27+
28+
function createWrapper() {
29+
const queryClient = new QueryClient({
30+
defaultOptions: { queries: { retry: false } },
31+
});
32+
return function Wrapper({ children }: { children: ReactNode }) {
33+
return (
34+
<QueryClientProvider client={queryClient}>{children}</QueryClientProvider>
35+
);
36+
};
37+
}
38+
39+
function clientWith(list: ReturnType<typeof vi.fn>) {
40+
return { agents: { list } } as unknown as AgentexSDK;
41+
}
42+
43+
describe('useAgents', () => {
44+
it('pages through every agent until a short page is returned', async () => {
45+
const list = vi
46+
.fn()
47+
.mockResolvedValueOnce(makeAgents(PAGE_SIZE, 'p1')) // full page -> keep going
48+
.mockResolvedValueOnce(makeAgents(24, 'p2')); // short page -> stop
49+
50+
const { result } = renderHook(() => useAgents(clientWith(list)), {
51+
wrapper: createWrapper(),
52+
});
53+
54+
await waitFor(() => expect(result.current.isSuccess).toBe(true));
55+
56+
expect(result.current.data).toHaveLength(PAGE_SIZE + 24);
57+
expect(list).toHaveBeenCalledTimes(2);
58+
expect(list).toHaveBeenNthCalledWith(1, {
59+
limit: PAGE_SIZE,
60+
page_number: 1,
61+
});
62+
expect(list).toHaveBeenNthCalledWith(2, {
63+
limit: PAGE_SIZE,
64+
page_number: 2,
65+
});
66+
});
67+
68+
it('makes a single request when the first page is not full', async () => {
69+
const list = vi.fn().mockResolvedValueOnce(makeAgents(10, 'only'));
70+
71+
const { result } = renderHook(() => useAgents(clientWith(list)), {
72+
wrapper: createWrapper(),
73+
});
74+
75+
await waitFor(() => expect(result.current.isSuccess).toBe(true));
76+
77+
expect(result.current.data).toHaveLength(10);
78+
expect(list).toHaveBeenCalledTimes(1);
79+
});
80+
81+
it('returns an empty list without paging when there are no agents', async () => {
82+
const list = vi.fn().mockResolvedValueOnce([]);
83+
84+
const { result } = renderHook(() => useAgents(clientWith(list)), {
85+
wrapper: createWrapper(),
86+
});
87+
88+
await waitFor(() => expect(result.current.isSuccess).toBe(true));
89+
90+
expect(result.current.data).toEqual([]);
91+
expect(list).toHaveBeenCalledTimes(1);
92+
});
93+
94+
it('stops at the exact-multiple boundary without looping forever', async () => {
95+
// Total is an exact multiple of the page size: a full page followed by an empty page.
96+
const list = vi
97+
.fn()
98+
.mockResolvedValueOnce(makeAgents(PAGE_SIZE, 'p1'))
99+
.mockResolvedValueOnce([]);
100+
101+
const { result } = renderHook(() => useAgents(clientWith(list)), {
102+
wrapper: createWrapper(),
103+
});
104+
105+
await waitFor(() => expect(result.current.isSuccess).toBe(true));
106+
107+
expect(result.current.data).toHaveLength(PAGE_SIZE);
108+
expect(list).toHaveBeenCalledTimes(2);
109+
});
110+
111+
it('stops at the safety bound and warns when every page stays full', async () => {
112+
const warn = vi.spyOn(console, 'warn').mockImplementation(() => {});
113+
// Always return a full page so the loop can only stop at MAX_AGENT_PAGES.
114+
const list = vi.fn().mockResolvedValue(makeAgents(PAGE_SIZE, 'full'));
115+
116+
const { result } = renderHook(() => useAgents(clientWith(list)), {
117+
wrapper: createWrapper(),
118+
});
119+
120+
await waitFor(() => expect(result.current.isSuccess).toBe(true));
121+
122+
expect(list).toHaveBeenCalledTimes(MAX_AGENT_PAGES);
123+
expect(result.current.data).toHaveLength(PAGE_SIZE * MAX_AGENT_PAGES);
124+
expect(warn).toHaveBeenCalledWith(
125+
expect.stringContaining('MAX_AGENT_PAGES')
126+
);
127+
128+
warn.mockRestore();
129+
});
130+
131+
it('surfaces an error instead of a partial list when a page fails mid-loop', async () => {
132+
const list = vi
133+
.fn()
134+
.mockResolvedValueOnce(makeAgents(PAGE_SIZE, 'p1'))
135+
.mockRejectedValueOnce(new Error('boom'));
136+
137+
const { result } = renderHook(() => useAgents(clientWith(list)), {
138+
wrapper: createWrapper(),
139+
});
140+
141+
await waitFor(() => expect(result.current.isError).toBe(true));
142+
143+
expect(result.current.data).toBeUndefined();
144+
expect(list).toHaveBeenCalledTimes(2);
145+
});
146+
});

0 commit comments

Comments
 (0)