diff --git a/packages/cli/src/config/extensionRegistryClient.test.ts b/packages/cli/src/config/extensionRegistryClient.test.ts index 187390ceb0..1e765b1d8f 100644 --- a/packages/cli/src/config/extensionRegistryClient.test.ts +++ b/packages/cli/src/config/extensionRegistryClient.test.ts @@ -224,4 +224,59 @@ describe('ExtensionRegistryClient', () => { 'Failed to fetch extensions: Not Found', ); }); + + it('should deduplicate extensions by ID', async () => { + const duplicateExtensions = [ + mockExtensions[0], + mockExtensions[0], // Duplicate + mockExtensions[1], + ]; + + fetchMock.mockResolvedValue({ + ok: true, + json: async () => duplicateExtensions, + }); + + const result = await client.getAllExtensions(); + expect(result).toHaveLength(2); + expect(result[0].id).toBe('ext1'); + expect(result[1].id).toBe('ext2'); + }); + + it('should not return irrelevant results for specific queries', async () => { + const extensions = [ + { + ...mockExtensions[0], + id: 'conductor', + extensionName: 'conductor', + extensionDescription: + 'Conductor is a Gemini CLI extension that allows you to specify, plan, and implement software features.', + fullName: 'google/conductor', + }, + { + ...mockExtensions[1], + id: 'dataplex', + extensionName: 'dataplex', + extensionDescription: + 'Connect to Dataplex Universal Catalog to discover, manage, monitor, and govern data and AI artifacts across your data platform', + fullName: 'google/dataplex', + }, + ]; + + fetchMock.mockResolvedValue({ + ok: true, + json: async () => extensions, + }); + + const results = await client.searchExtensions('conductor'); + + // Conductor should definitely be first + expect(results[0].id).toBe('conductor'); + + // Dataplex should ideally NOT be in the results, or at least be ranked lower (which it will be if it matches at all). + // But user complaint is that it IS in results. + // Let's assert it is NOT in results if we want strictness. + const ids = results.map((r) => r.id); + expect(ids).not.toContain('dataplex'); + }); }); diff --git a/packages/cli/src/config/extensionRegistryClient.ts b/packages/cli/src/config/extensionRegistryClient.ts index 80baf6b562..26940c962b 100644 --- a/packages/cli/src/config/extensionRegistryClient.ts +++ b/packages/cli/src/config/extensionRegistryClient.ts @@ -85,11 +85,24 @@ export class ExtensionRegistryClient { const fzf = new AsyncFzf(allExtensions, { selector: (ext: RegistryExtension) => - `${ext.extensionName} ${ext.extensionDescription} ${ext.fullName}`, + `${ext.extensionName} ${ext.extensionDescription || ''} ${ + ext.fullName || '' + }`, fuzzy: 'v2', }); const results = await fzf.find(query); - return results.map((r: { item: RegistryExtension }) => r.item); + + if (results.length === 0) { + return []; + } + + const maxScore = results[0].score; + const THRESHOLD_RATIO = 0.75; + const threshold = maxScore * THRESHOLD_RATIO; + + return results + .filter((r: { score: number }) => r.score >= threshold) + .map((r: { item: RegistryExtension }) => r.item); } async getExtension(id: string): Promise { diff --git a/packages/cli/src/ui/components/shared/SearchableList.tsx b/packages/cli/src/ui/components/shared/SearchableList.tsx index 3382707de5..877f00abe0 100644 --- a/packages/cli/src/ui/components/shared/SearchableList.tsx +++ b/packages/cli/src/ui/components/shared/SearchableList.tsx @@ -42,6 +42,8 @@ export interface SearchableListProps { footer?: | React.ReactNode | ((pagination: SearchableListPaginationInfo) => React.ReactNode); + /** If true, disables client-side filtering. Useful if items are already filtered externally. */ + disableFiltering?: boolean; } export interface SearchableListPaginationInfo { @@ -63,21 +65,38 @@ export function SearchableList({ searchPlaceholder = 'Search...', maxItemsToShow = 10, renderItem, + header, footer, -}: SearchableListProps): React.JSX.Element { + disableFiltering = false, + onSearch, +}: SearchableListProps & { + onSearch?: (query: string) => void; +}): React.JSX.Element { const { filteredItems, searchBuffer, maxLabelWidth } = useFuzzyList({ items, initialQuery: initialSearchQuery, + disableFiltering, + onSearch, }); const [activeIndex, setActiveIndex] = useState(0); const [scrollOffset, setScrollOffset] = useState(0); - // Reset selection when filtered items change + const prevFilteredKeysRef = React.useRef([]); useEffect(() => { - setActiveIndex(0); - setScrollOffset(0); + const currentKeys = filteredItems.map((item) => item.key); + const prevKeys = prevFilteredKeysRef.current; + + const hasChanged = + currentKeys.length !== prevKeys.length || + currentKeys.some((key, index) => key !== prevKeys[index]); + + if (hasChanged) { + setActiveIndex(0); + setScrollOffset(0); + prevFilteredKeysRef.current = currentKeys; + } }, [filteredItems]); // Calculate visible items diff --git a/packages/cli/src/ui/components/views/ExtensionRegistryView.test.tsx b/packages/cli/src/ui/components/views/ExtensionRegistryView.test.tsx index b774155ce9..8477b06264 100644 --- a/packages/cli/src/ui/components/views/ExtensionRegistryView.test.tsx +++ b/packages/cli/src/ui/components/views/ExtensionRegistryView.test.tsx @@ -40,7 +40,7 @@ describe('ExtensionRegistryView', () => { // Return a promise that doesn't resolve immediately to keep the loading state active vi.spyOn( ExtensionRegistryClient.prototype, - 'getAllExtensions', + 'searchExtensions', ).mockReturnValue(new Promise(() => {})); const mockExtensionManager = { @@ -58,7 +58,7 @@ describe('ExtensionRegistryView', () => { it('should render extensions after fetching', async () => { vi.spyOn( ExtensionRegistryClient.prototype, - 'getAllExtensions', + 'searchExtensions', ).mockResolvedValue(mockExtensions as unknown as RegistryExtension[]); const mockExtensionManager = { @@ -71,9 +71,11 @@ describe('ExtensionRegistryView', () => { />, ); + // Wait for effect and debounce await act(async () => { await Promise.resolve(); await Promise.resolve(); + // Add a small delay for debounce/async logic if needed, though mocking resolved value should be enough if called immediately }); const frame = lastFrame(); @@ -86,7 +88,7 @@ describe('ExtensionRegistryView', () => { it('should render error message on fetch failure', async () => { vi.spyOn( ExtensionRegistryClient.prototype, - 'getAllExtensions', + 'searchExtensions', ).mockRejectedValue(new Error('Fetch failed')); const mockExtensionManager = { @@ -113,7 +115,7 @@ describe('ExtensionRegistryView', () => { it('should call onSelect when an item is selected', async () => { vi.spyOn( ExtensionRegistryClient.prototype, - 'getAllExtensions', + 'searchExtensions', ).mockResolvedValue(mockExtensions as unknown as RegistryExtension[]); const onSelect = vi.fn(); diff --git a/packages/cli/src/ui/components/views/ExtensionRegistryView.tsx b/packages/cli/src/ui/components/views/ExtensionRegistryView.tsx index 4b2b454f23..b5d6be9911 100644 --- a/packages/cli/src/ui/components/views/ExtensionRegistryView.tsx +++ b/packages/cli/src/ui/components/views/ExtensionRegistryView.tsx @@ -5,16 +5,15 @@ */ import type React from 'react'; -import { useState, useEffect, useMemo } from 'react'; +import { useMemo } from 'react'; import { Box, Text } from 'ink'; -import { - ExtensionRegistryClient, - type RegistryExtension, -} from '../../../config/extensionRegistryClient.js'; +import type { RegistryExtension } from '../../../config/extensionRegistryClient.js'; + import { SearchableList } from '../shared/SearchableList.js'; import type { GenericListItem } from '../../hooks/useFuzzyList.js'; import { theme } from '../../semantic-colors.js'; +import { useExtensionRegistry } from '../../hooks/useExtensionRegistry.js'; import { ExtensionUpdateState } from '../../state/extensions.js'; import { useExtensionUpdates } from '../../hooks/useExtensionUpdates.js'; import { useConfig } from '../../contexts/ConfigContext.js'; @@ -35,9 +34,7 @@ export function ExtensionRegistryView({ onClose, extensionManager, }: ExtensionRegistryViewProps): React.JSX.Element { - const [extensions, setExtensions] = useState([]); - const [loading, setLoading] = useState(true); - const [error, setError] = useState(null); + const { extensions, loading, error, search } = useExtensionRegistry(); const config = useConfig(); const { extensionsUpdateState } = useExtensionUpdates( @@ -48,33 +45,6 @@ export function ExtensionRegistryView({ const installedExtensions = extensionManager.getExtensions(); - const client = useMemo(() => new ExtensionRegistryClient(), []); - - useEffect(() => { - let active = true; - const fetchExtensions = async () => { - try { - // Fetch all extensions to enable comprehensive local fuzzy search. - // Display virtualization/pagination is handled by SearchableList. - const extensions = await client.getAllExtensions(); - if (active) { - setExtensions(extensions); - setLoading(false); - } - } catch (err) { - if (active) { - setError(err instanceof Error ? err.message : String(err)); - setLoading(false); - } - } - }; - - void fetchExtensions(); - return () => { - active = false; - }; - }, [client]); - const items: ExtensionItem[] = useMemo( () => extensions.map((ext) => ({ @@ -158,7 +128,8 @@ export function ExtensionRegistryView({ - Reg: {extensions.length} | Inst: {installedExtensions.length} + {installedExtensions.length && + `${installedExtensions.length} installed`} @@ -206,6 +177,8 @@ export function ExtensionRegistryView({ header={header} footer={footer} maxItemsToShow={8} + disableFiltering={true} + onSearch={search} /> ); } diff --git a/packages/cli/src/ui/hooks/useExtensionRegistry.test.ts b/packages/cli/src/ui/hooks/useExtensionRegistry.test.ts new file mode 100644 index 0000000000..03b0a20bf5 --- /dev/null +++ b/packages/cli/src/ui/hooks/useExtensionRegistry.test.ts @@ -0,0 +1,191 @@ +/** + * @license + * Copyright 2026 Google LLC + * SPDX-License-Identifier: Apache-2.0 + */ + +import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; +import { renderHook } from '../../test-utils/render.js'; +import { useExtensionRegistry } from './useExtensionRegistry.js'; +import { + ExtensionRegistryClient, + type RegistryExtension, +} from '../../config/extensionRegistryClient.js'; +import { act } from 'react'; + +vi.mock('../../config/extensionRegistryClient.js'); + +const mockExtensions = [ + { + id: 'ext-1', + extensionName: 'Extension 1', + extensionDescription: 'Description 1', + }, + { + id: 'ext-2', + extensionName: 'Extension 2', + extensionDescription: 'Description 2', + }, +] as RegistryExtension[]; + +describe('useExtensionRegistry', () => { + beforeEach(() => { + vi.clearAllMocks(); + vi.useFakeTimers(); + }); + + afterEach(() => { + vi.useRealTimers(); + }); + + it('should fetch extensions on mount', async () => { + vi.spyOn( + ExtensionRegistryClient.prototype, + 'searchExtensions', + ).mockResolvedValue(mockExtensions); + + const { result } = renderHook(() => useExtensionRegistry()); + + expect(result.current.loading).toBe(true); + expect(result.current.extensions).toEqual([]); + + await act(async () => { + await Promise.resolve(); + }); + + expect(result.current.loading).toBe(false); + expect(result.current.extensions).toEqual(mockExtensions); + expect( + ExtensionRegistryClient.prototype.searchExtensions, + ).toHaveBeenCalledWith(''); + }); + + it('should handle search with debounce', async () => { + vi.spyOn( + ExtensionRegistryClient.prototype, + 'searchExtensions', + ).mockResolvedValue(mockExtensions); + + const { result } = renderHook(() => useExtensionRegistry()); + + await act(async () => { + await Promise.resolve(); + }); + + // Initial load done + expect( + ExtensionRegistryClient.prototype.searchExtensions, + ).toHaveBeenCalledTimes(1); + + // Search + act(() => { + result.current.search('test'); + }); + + // Should not happen immediately due to debounce + expect( + ExtensionRegistryClient.prototype.searchExtensions, + ).toHaveBeenCalledTimes(1); + + // Advance time + await act(async () => { + vi.advanceTimersByTime(300); + await Promise.resolve(); // Allow potential async effects to run + }); + + expect( + ExtensionRegistryClient.prototype.searchExtensions, + ).toHaveBeenCalledTimes(2); + expect( + ExtensionRegistryClient.prototype.searchExtensions, + ).toHaveBeenCalledWith('test'); + }); + + it('should handle race conditions by ignoring outdated responses', async () => { + // Setup a delayed response for the first query 'a' + let resolveA: (value: RegistryExtension[]) => void; + const promiseA = new Promise((resolve) => { + resolveA = resolve; + }); + + // Immediate response for query 'b' + const responseB = [mockExtensions[1]]; + + vi.spyOn( + ExtensionRegistryClient.prototype, + 'searchExtensions', + ).mockImplementation(async (query) => { + if (query === 'a') return promiseA; + if (query === 'b') return responseB; + return []; + }); + + const { result } = renderHook(() => useExtensionRegistry('')); + + await act(async () => { + await Promise.resolve(); // Initial load empty + }); + + // Search 'a' + act(() => { + result.current.search('a'); + vi.advanceTimersByTime(300); + }); + + // Search 'b' immediately after (conceptually, though heavily simplified test here) + // Actually, to test race condition: + // 1. Trigger search 'a'. + // 2. Trigger search 'b'. + // 3. 'b' resolves. + // 4. 'a' resolves later. + // 5. State should match 'b'. + + act(() => { + result.current.search('b'); + vi.advanceTimersByTime(300); + }); + + await act(async () => { + await Promise.resolve(); // 'b' resolves immediately + }); + + expect(result.current.extensions).toEqual(responseB); + + // Now resolve 'a' + await act(async () => { + resolveA!(mockExtensions); + await Promise.resolve(); + }); + + // Should still be 'b' because 'a' was outdated + expect(result.current.extensions).toEqual(responseB); + }); + + it('should not update state if extensions are identical', async () => { + vi.spyOn( + ExtensionRegistryClient.prototype, + 'searchExtensions', + ).mockResolvedValue(mockExtensions); + + const { result } = renderHook(() => useExtensionRegistry()); + + await act(async () => { + await Promise.resolve(); + }); + + const initialExtensions = result.current.extensions; + + // Trigger another search that returns identical content + act(() => { + result.current.search('test'); + vi.advanceTimersByTime(300); + }); + + await act(async () => { + await Promise.resolve(); + }); + + // The reference should be exactly the same + expect(result.current.extensions).toBe(initialExtensions); + }); +}); diff --git a/packages/cli/src/ui/hooks/useExtensionRegistry.ts b/packages/cli/src/ui/hooks/useExtensionRegistry.ts new file mode 100644 index 0000000000..1905aa027c --- /dev/null +++ b/packages/cli/src/ui/hooks/useExtensionRegistry.ts @@ -0,0 +1,98 @@ +/** + * @license + * Copyright 2026 Google LLC + * SPDX-License-Identifier: Apache-2.0 + */ + +import { useState, useEffect, useMemo, useCallback, useRef } from 'react'; +import { + ExtensionRegistryClient, + type RegistryExtension, +} from '../../config/extensionRegistryClient.js'; + +export interface UseExtensionRegistryResult { + extensions: RegistryExtension[]; + loading: boolean; + error: string | null; + search: (query: string) => void; +} + +export function useExtensionRegistry( + initialQuery = '', +): UseExtensionRegistryResult { + const [extensions, setExtensions] = useState([]); + const [loading, setLoading] = useState(true); + const [error, setError] = useState(null); + + const client = useMemo(() => new ExtensionRegistryClient(), []); + + // Ref to track the latest query to avoid race conditions + const latestQueryRef = useRef(initialQuery); + // Ref for debounce timeout + const debounceTimeoutRef = useRef(undefined); + + const searchExtensions = useCallback( + async (query: string) => { + try { + setLoading(true); + const results = await client.searchExtensions(query); + // Only update if this is still the latest query + if (query === latestQueryRef.current) { + // Check if results are different from current extensions + setExtensions((prev) => { + if ( + prev.length === results.length && + prev.every((ext, i) => ext.id === results[i].id) + ) { + return prev; + } + return results; + }); + setError(null); + setLoading(false); + } + } catch (err) { + if (query === latestQueryRef.current) { + setError(err instanceof Error ? err.message : String(err)); + setExtensions([]); + setLoading(false); + } + } + }, + [client], + ); + + const search = useCallback( + (query: string) => { + latestQueryRef.current = query; + + // Clear existing timeout + if (debounceTimeoutRef.current) { + clearTimeout(debounceTimeoutRef.current); + } + + // Debounce + debounceTimeoutRef.current = setTimeout(() => { + void searchExtensions(query); + }, 300); + }, + [searchExtensions], + ); + + // Initial load + useEffect(() => { + void searchExtensions(initialQuery); + return () => { + if (debounceTimeoutRef.current) { + clearTimeout(debounceTimeoutRef.current); + } + }; + }, [initialQuery, searchExtensions]); + + return { + extensions, + loading, + error, + search, + }; +} diff --git a/packages/cli/src/ui/hooks/useFuzzyList.ts b/packages/cli/src/ui/hooks/useFuzzyList.ts index be0b3832df..28469bd0a8 100644 --- a/packages/cli/src/ui/hooks/useFuzzyList.ts +++ b/packages/cli/src/ui/hooks/useFuzzyList.ts @@ -24,6 +24,7 @@ export interface UseFuzzyListProps { items: T[]; initialQuery?: string; onSearch?: (query: string) => void; + disableFiltering?: boolean; } export interface UseFuzzyListResult { @@ -38,6 +39,7 @@ export function useFuzzyList({ items, initialQuery = '', onSearch, + disableFiltering = false, }: UseFuzzyListProps): UseFuzzyListResult { // Search state const [searchQuery, setSearchQuery] = useState(initialQuery); @@ -46,40 +48,81 @@ export function useFuzzyList({ ); // FZF instance for fuzzy searching - const fzfInstance = useMemo(() => new AsyncFzf(items, { + // FZF instance for fuzzy searching - skip if filtering is disabled + const fzfInstance = useMemo(() => { + if (disableFiltering) return null; + return new AsyncFzf(items, { fuzzy: 'v2', casing: 'case-insensitive', selector: (item: T) => item.label, - }), [items]); + }); + }, [items, disableFiltering]); // Perform search useEffect(() => { let active = true; - if (!searchQuery.trim() || !fzfInstance) { + if (!searchQuery.trim() || (!fzfInstance && !disableFiltering)) { setFilteredKeys(items.map((i) => i.key)); return; } const doSearch = async () => { - const results = await fzfInstance.find(searchQuery); + // If filtering is disabled, or no query/fzf, just return all items (or handle external search elsewhere) + if (disableFiltering) { + onSearch?.(searchQuery); + // When filtering is disabled, we assume the items passed in are already filtered + // so we set filteredKeys to all items + const allKeys = items.map((i) => i.key); + setFilteredKeys((prev) => { + if ( + prev.length === allKeys.length && + prev.every((key, index) => key === allKeys[index]) + ) { + return prev; + } + return allKeys; + }); + return; + } - if (!active) return; + if (fzfInstance) { + const results = await fzfInstance.find(searchQuery); - const matchedKeys = results.map((res: { item: T }) => res.item.key); - setFilteredKeys(matchedKeys); - onSearch?.(searchQuery); + if (!active) return; + + const matchedKeys = results.map((res: { item: T }) => res.item.key); + setFilteredKeys((prev) => { + if ( + prev.length === matchedKeys.length && + prev.every((key, index) => key === matchedKeys[index]) + ) { + return prev; + } + return matchedKeys; + }); + onSearch?.(searchQuery); + } }; void doSearch().catch((error) => { // eslint-disable-next-line no-console console.error('Search failed:', error); - setFilteredKeys(items.map((i) => i.key)); // Reset to all items on error + const allKeys = items.map((i) => i.key); + setFilteredKeys((prev) => { + if ( + prev.length === allKeys.length && + prev.every((key, index) => key === allKeys[index]) + ) { + return prev; + } + return allKeys; + }); }); return () => { active = false; }; - }, [searchQuery, fzfInstance, items, onSearch]); + }, [searchQuery, fzfInstance, items, onSearch, disableFiltering]); // Get mainAreaWidth for search buffer viewport from UIState const { mainAreaWidth } = useUIState(); @@ -99,9 +142,10 @@ export function useFuzzyList({ // Filtered items to display const filteredItems = useMemo(() => { + if (disableFiltering) return items; if (!searchQuery) return items; return items.filter((item) => filteredKeys.includes(item.key)); - }, [items, filteredKeys, searchQuery]); + }, [items, filteredKeys, searchQuery, disableFiltering]); // Calculate max label width for alignment const maxLabelWidth = useMemo(() => {