diff --git a/agentex-ui/components/agentex-ui-root.tsx b/agentex-ui/components/agentex-ui-root.tsx index 3466f2de..e027b974 100644 --- a/agentex-ui/components/agentex-ui-root.tsx +++ b/agentex-ui/components/agentex-ui-root.tsx @@ -8,6 +8,7 @@ import { PrimaryContent } from '@/components/primary-content/primary-content'; import { useAgentexClient } from '@/components/providers'; import { TaskSidebar } from '@/components/task-sidebar/task-sidebar'; import { TracesSidebar } from '@/components/traces-sidebar/traces-sidebar'; +import { useAgentByName } from '@/hooks/use-agent-by-name'; import { useAgents } from '@/hooks/use-agents'; import { useLocalStorageState } from '@/hooks/use-local-storage-state'; import { @@ -19,18 +20,39 @@ export function AgentexUIRoot() { const { agentName, taskID, updateParams } = useSafeSearchParams(); const [isTracesSidebarOpen, setIsTracesSidebarOpen] = useState(false); const { agentexClient } = useAgentexClient(); - const { data: agents = [], isLoading } = useAgents(agentexClient); + const { data: agents = [], isFetching: isAgentsFetching } = + useAgents(agentexClient); + // Validate the deep-linked agent directly against the backend so a valid agent opens + // even if it sits outside the loaded list (e.g. on accounts with many agents). + const { + data: agentByName, + isFetching: isAgentByNameFetching, + isError: isAgentByNameError, + } = useAgentByName(agentexClient, agentName); const [localAgentName, setLocalAgentName] = useLocalStorageState< string | undefined >('lastSelectedAgent', undefined); + // Wait until neither query is fetching before validating. We gate on `isFetching` (not + // `isLoading`, which is false once a query has cached data) so we never validate against + // stale data mid-refetch. A disabled by-name query reports `isFetching: false`, so it + // doesn't block the localStorage-restore path when there's no agent_name. Deps are + // intentionally narrowed so we re-validate on fetch settle / agent_name change. useEffect(() => { - if (isLoading) return; + if (isAgentsFetching || isAgentByNameFetching) return; - const selectedAgent = agents.find(agent => agent.name === agentName); + // Accept an agent found in the (paginated) list OR resolved directly by name, so a valid + // deep-linked agent opens even if it falls outside the loaded list. + const agentInList = agents.find(agent => agent.name === agentName); + const selectedAgent = agentInList ?? agentByName ?? undefined; const isAgentValid = selectedAgent && selectedAgent.status === 'Ready'; - if (!isAgentValid) { + // If the agent isn't in the loaded list and the by-name lookup errored (a transient + // failure, not a 404), we can't tell whether it's valid — leave the URL alone rather + // than bouncing a possibly-valid deep link to the home grid. + const couldNotDetermine = !agentInList && isAgentByNameError; + + if (agentName && !isAgentValid && !couldNotDetermine) { updateParams({ [SearchParamKey.AGENT_NAME]: null }); setLocalAgentName(undefined); } @@ -39,7 +61,7 @@ export function AgentexUIRoot() { updateParams({ [SearchParamKey.AGENT_NAME]: localAgentName }); } // eslint-disable-next-line react-hooks/exhaustive-deps - }, [isLoading]); + }, [isAgentsFetching, isAgentByNameFetching, isAgentByNameError, agentName]); const handleSelectTask = useCallback( (taskId: string | null) => { diff --git a/agentex-ui/components/agents-list/agents-list.tsx b/agentex-ui/components/agents-list/agents-list.tsx index 49c450c4..1c6d3c6e 100644 --- a/agentex-ui/components/agents-list/agents-list.tsx +++ b/agentex-ui/components/agents-list/agents-list.tsx @@ -50,7 +50,7 @@ export function AgentsList({ agents, isLoading = false }: AgentsListProps) { return ( {displayedAgents.length > 0 ? ( diff --git a/agentex-ui/hooks/use-agent-by-name.test.tsx b/agentex-ui/hooks/use-agent-by-name.test.tsx new file mode 100644 index 00000000..f0da260d --- /dev/null +++ b/agentex-ui/hooks/use-agent-by-name.test.tsx @@ -0,0 +1,101 @@ +import type { ReactNode } from 'react'; + +import { QueryClient, QueryClientProvider } from '@tanstack/react-query'; +import { renderHook, waitFor } from '@testing-library/react'; +import { APIError, NotFoundError } from 'agentex'; +import { describe, expect, it, vi } from 'vitest'; + +import { useAgentByName } from './use-agent-by-name'; + +import type AgentexSDK from 'agentex'; +import type { Agent } from 'agentex/resources'; + +function createWrapper() { + const queryClient = new QueryClient({ + defaultOptions: { queries: { retry: false } }, + }); + return function Wrapper({ children }: { children: ReactNode }) { + return ( + {children} + ); + }; +} + +function clientWith(retrieveByName: ReturnType) { + return { agents: { retrieveByName } } as unknown as AgentexSDK; +} + +describe('useAgentByName', () => { + it('does not fetch when no agent name is provided', () => { + const retrieveByName = vi.fn(); + + const { result } = renderHook( + () => useAgentByName(clientWith(retrieveByName), null), + { + wrapper: createWrapper(), + } + ); + + expect(retrieveByName).not.toHaveBeenCalled(); + expect(result.current.fetchStatus).toBe('idle'); + expect(result.current.data).toBeUndefined(); + }); + + it('returns the agent when the lookup succeeds', async () => { + const agent = { + id: 'a1', + name: 'interview-agent', + status: 'Ready', + } as Agent; + const retrieveByName = vi.fn().mockResolvedValueOnce(agent); + + const { result } = renderHook( + () => useAgentByName(clientWith(retrieveByName), 'interview-agent'), + { wrapper: createWrapper() } + ); + + await waitFor(() => expect(result.current.isSuccess).toBe(true)); + + expect(result.current.data).toEqual(agent); + expect(retrieveByName).toHaveBeenCalledWith('interview-agent'); + }); + + it('resolves to null (not an error) on a 404', async () => { + const notFound = new NotFoundError( + 404, + undefined, + 'Not found', + new Headers() + ); + const retrieveByName = vi.fn().mockRejectedValueOnce(notFound); + + const { result } = renderHook( + () => useAgentByName(clientWith(retrieveByName), 'missing-agent'), + { wrapper: createWrapper() } + ); + + await waitFor(() => expect(result.current.isSuccess).toBe(true)); + + expect(result.current.data).toBeNull(); + expect(result.current.isError).toBe(false); + }); + + it('surfaces a non-404 error instead of masquerading as not-found', async () => { + const serverError = new APIError( + 500, + undefined, + 'Server error', + new Headers() + ); + const retrieveByName = vi.fn().mockRejectedValueOnce(serverError); + + const { result } = renderHook( + () => useAgentByName(clientWith(retrieveByName), 'some-agent'), + { wrapper: createWrapper() } + ); + + await waitFor(() => expect(result.current.isError).toBe(true)); + + expect(result.current.data).toBeUndefined(); + }); +}); diff --git a/agentex-ui/hooks/use-agent-by-name.ts b/agentex-ui/hooks/use-agent-by-name.ts new file mode 100644 index 00000000..46225972 --- /dev/null +++ b/agentex-ui/hooks/use-agent-by-name.ts @@ -0,0 +1,48 @@ +import { useQuery } from '@tanstack/react-query'; +import { APIError } from 'agentex'; + +import type AgentexSDK from 'agentex'; +import type { Agent } from 'agentex/resources'; + +export const agentByNameKeys = { + byName: (name: string) => ['agents', 'by-name', name] as const, +}; + +/** + * Fetches a single agent by its unique name. + * + * Used to validate a deep-linked `agent_name` directly against the backend so that opening + * an agent does not depend on the entire (paginated) agent list having been loaded first. + * The query is disabled when no name is provided. A 404 resolves to `null` ("no such agent" + * — a normal outcome), while any other failure (network / 5xx / auth) is re-thrown so React + * Query surfaces it as an error rather than masquerading as "not found" (which could + * otherwise clear a valid deep link). + * + * @param agentexClient - AgentexSDK - The SDK client used to communicate with the Agentex API + * @param agentName - The agent name to look up; query is disabled when absent (null/undefined) + * @returns UseQueryResult - React Query result containing the agent, or null if not found + */ +export function useAgentByName( + agentexClient: AgentexSDK, + agentName: string | null | undefined +) { + return useQuery({ + queryKey: agentByNameKeys.byName(agentName ?? ''), + queryFn: async (): Promise => { + try { + return await agentexClient.agents.retrieveByName(agentName as string); + } catch (error) { + // A 404 means the name isn't a real, reachable agent — a normal "not found" + // outcome, so resolve to null. Re-throw anything else (network / 5xx / auth) so a + // transient failure surfaces as a query error instead of masquerading as + // "not found", which could otherwise clear a valid deep link. + if (error instanceof APIError && error.status === 404) { + return null; + } + throw error; + } + }, + enabled: !!agentName, + refetchOnWindowFocus: false, + }); +} diff --git a/agentex-ui/hooks/use-agents.test.tsx b/agentex-ui/hooks/use-agents.test.tsx new file mode 100644 index 00000000..f902212f --- /dev/null +++ b/agentex-ui/hooks/use-agents.test.tsx @@ -0,0 +1,146 @@ +import type { ReactNode } from 'react'; + +import { QueryClient, QueryClientProvider } from '@tanstack/react-query'; +import { renderHook, waitFor } from '@testing-library/react'; +import { describe, expect, it, vi } from 'vitest'; + +import { + AGENTS_PAGE_SIZE as PAGE_SIZE, + MAX_AGENT_PAGES, + useAgents, +} from './use-agents'; + +import type AgentexSDK from 'agentex'; +import type { Agent } from 'agentex/resources'; + +function makeAgents(count: number, prefix: string): Agent[] { + return Array.from( + { length: count }, + (_, i) => + ({ + id: `${prefix}-${i}`, + name: `${prefix}-${i}`, + status: 'Ready', + }) as Agent + ); +} + +function createWrapper() { + const queryClient = new QueryClient({ + defaultOptions: { queries: { retry: false } }, + }); + return function Wrapper({ children }: { children: ReactNode }) { + return ( + {children} + ); + }; +} + +function clientWith(list: ReturnType) { + return { agents: { list } } as unknown as AgentexSDK; +} + +describe('useAgents', () => { + it('pages through every agent until a short page is returned', async () => { + const list = vi + .fn() + .mockResolvedValueOnce(makeAgents(PAGE_SIZE, 'p1')) // full page -> keep going + .mockResolvedValueOnce(makeAgents(24, 'p2')); // short page -> stop + + const { result } = renderHook(() => useAgents(clientWith(list)), { + wrapper: createWrapper(), + }); + + await waitFor(() => expect(result.current.isSuccess).toBe(true)); + + expect(result.current.data).toHaveLength(PAGE_SIZE + 24); + expect(list).toHaveBeenCalledTimes(2); + expect(list).toHaveBeenNthCalledWith(1, { + limit: PAGE_SIZE, + page_number: 1, + }); + expect(list).toHaveBeenNthCalledWith(2, { + limit: PAGE_SIZE, + page_number: 2, + }); + }); + + it('makes a single request when the first page is not full', async () => { + const list = vi.fn().mockResolvedValueOnce(makeAgents(10, 'only')); + + const { result } = renderHook(() => useAgents(clientWith(list)), { + wrapper: createWrapper(), + }); + + await waitFor(() => expect(result.current.isSuccess).toBe(true)); + + expect(result.current.data).toHaveLength(10); + expect(list).toHaveBeenCalledTimes(1); + }); + + it('returns an empty list without paging when there are no agents', async () => { + const list = vi.fn().mockResolvedValueOnce([]); + + const { result } = renderHook(() => useAgents(clientWith(list)), { + wrapper: createWrapper(), + }); + + await waitFor(() => expect(result.current.isSuccess).toBe(true)); + + expect(result.current.data).toEqual([]); + expect(list).toHaveBeenCalledTimes(1); + }); + + it('stops at the exact-multiple boundary without looping forever', async () => { + // Total is an exact multiple of the page size: a full page followed by an empty page. + const list = vi + .fn() + .mockResolvedValueOnce(makeAgents(PAGE_SIZE, 'p1')) + .mockResolvedValueOnce([]); + + const { result } = renderHook(() => useAgents(clientWith(list)), { + wrapper: createWrapper(), + }); + + await waitFor(() => expect(result.current.isSuccess).toBe(true)); + + expect(result.current.data).toHaveLength(PAGE_SIZE); + expect(list).toHaveBeenCalledTimes(2); + }); + + it('stops at the safety bound and warns when every page stays full', async () => { + const warn = vi.spyOn(console, 'warn').mockImplementation(() => {}); + // Always return a full page so the loop can only stop at MAX_AGENT_PAGES. + const list = vi.fn().mockResolvedValue(makeAgents(PAGE_SIZE, 'full')); + + const { result } = renderHook(() => useAgents(clientWith(list)), { + wrapper: createWrapper(), + }); + + await waitFor(() => expect(result.current.isSuccess).toBe(true)); + + expect(list).toHaveBeenCalledTimes(MAX_AGENT_PAGES); + expect(result.current.data).toHaveLength(PAGE_SIZE * MAX_AGENT_PAGES); + expect(warn).toHaveBeenCalledWith( + expect.stringContaining('MAX_AGENT_PAGES') + ); + + warn.mockRestore(); + }); + + it('surfaces an error instead of a partial list when a page fails mid-loop', async () => { + const list = vi + .fn() + .mockResolvedValueOnce(makeAgents(PAGE_SIZE, 'p1')) + .mockRejectedValueOnce(new Error('boom')); + + const { result } = renderHook(() => useAgents(clientWith(list)), { + wrapper: createWrapper(), + }); + + await waitFor(() => expect(result.current.isError).toBe(true)); + + expect(result.current.data).toBeUndefined(); + expect(list).toHaveBeenCalledTimes(2); + }); +}); diff --git a/agentex-ui/hooks/use-agents.ts b/agentex-ui/hooks/use-agents.ts index be5efde4..05a19bb1 100644 --- a/agentex-ui/hooks/use-agents.ts +++ b/agentex-ui/hooks/use-agents.ts @@ -7,11 +7,29 @@ export const agentsKeys = { all: ['agents'] as const, }; +/** + * Page size used when paging through the full agent list. + * + * The `/agents` endpoint is offset-paginated (1-indexed `page_number`) and defaults to + * 50 agents per page, so a single unpaginated call only ever returns the first page. We + * request a larger page and keep fetching until a page comes back short. + */ +export const AGENTS_PAGE_SIZE = 100; + +/** + * Upper bound on the number of pages we fetch, as a safety valve against an unbounded + * loop if the backend ever stops short-paging. At {@link AGENTS_PAGE_SIZE} per page this + * covers far more agents than any real account. + */ +export const MAX_AGENT_PAGES = 100; + /** * Fetches the complete list of agents available in the system. * - * This hook retrieves all agent definitions that can execute tasks. Refetch on window focus - * is disabled to prevent unnecessary API calls when switching browser tabs. + * The list endpoint is paginated, so this pages through every result — accumulating until + * a page returns fewer than {@link AGENTS_PAGE_SIZE} items — rather than returning only the + * default first page. Refetch on window focus is disabled to prevent unnecessary API calls + * when switching browser tabs. * * @param agentexClient - AgentexSDK - The SDK client used to communicate with the Agentex API * @returns UseQueryResult - React Query result containing the array of agent definitions @@ -20,7 +38,31 @@ export function useAgents(agentexClient: AgentexSDK) { return useQuery({ queryKey: agentsKeys.all, queryFn: async (): Promise => { - return agentexClient.agents.list(); + const allAgents: Agent[] = []; + + for (let pageNumber = 1; pageNumber <= MAX_AGENT_PAGES; pageNumber++) { + const page = await agentexClient.agents.list({ + limit: AGENTS_PAGE_SIZE, + page_number: pageNumber, + }); + + allAgents.push(...page); + + // A short page means we've reached the end of the list. + if (page.length < AGENTS_PAGE_SIZE) break; + + // Still a full page on the last allowed iteration: we've hit the safety bound + // with more agents likely remaining. Warn loudly rather than silently truncating + // (a silent truncation would reintroduce the "missing agents" bug this fix closes). + if (pageNumber === MAX_AGENT_PAGES) { + console.warn( + `useAgents: reached MAX_AGENT_PAGES (${MAX_AGENT_PAGES}) with a full final page; ` + + `the agent list may be truncated at ${allAgents.length} agents.` + ); + } + } + + return allAgents; }, refetchOnWindowFocus: false, });