make changes

This commit is contained in:
Christine Betts
2026-02-13 13:58:28 -05:00
parent 8d96c8ce72
commit f1f2070ea9
8 changed files with 452 additions and 57 deletions

View File

@@ -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');
});
});

View File

@@ -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<RegistryExtension | undefined> {

View File

@@ -42,6 +42,8 @@ export interface SearchableListProps<T extends GenericListItem> {
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<T extends GenericListItem>({
searchPlaceholder = 'Search...',
maxItemsToShow = 10,
renderItem,
header,
footer,
}: SearchableListProps<T>): React.JSX.Element {
disableFiltering = false,
onSearch,
}: SearchableListProps<T> & {
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<string[]>([]);
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

View File

@@ -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();

View File

@@ -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<RegistryExtension[]>([]);
const [loading, setLoading] = useState(true);
const [error, setError] = useState<string | null>(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({
</Box>
<Box flexShrink={0} marginLeft={2}>
<Text color={theme.text.secondary}>
Reg: {extensions.length} | Inst: {installedExtensions.length}
{installedExtensions.length &&
`${installedExtensions.length} installed`}
</Text>
</Box>
</Box>
@@ -206,6 +177,8 @@ export function ExtensionRegistryView({
header={header}
footer={footer}
maxItemsToShow={8}
disableFiltering={true}
onSearch={search}
/>
);
}

View File

@@ -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<RegistryExtension[]>((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);
});
});

View File

@@ -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<RegistryExtension[]>([]);
const [loading, setLoading] = useState(true);
const [error, setError] = useState<string | null>(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<NodeJS.Timeout | undefined>(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,
};
}

View File

@@ -24,6 +24,7 @@ export interface UseFuzzyListProps<T extends GenericListItem> {
items: T[];
initialQuery?: string;
onSearch?: (query: string) => void;
disableFiltering?: boolean;
}
export interface UseFuzzyListResult<T extends GenericListItem> {
@@ -38,6 +39,7 @@ export function useFuzzyList<T extends GenericListItem>({
items,
initialQuery = '',
onSearch,
disableFiltering = false,
}: UseFuzzyListProps<T>): UseFuzzyListResult<T> {
// Search state
const [searchQuery, setSearchQuery] = useState(initialQuery);
@@ -46,40 +48,81 @@ export function useFuzzyList<T extends GenericListItem>({
);
// 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<T extends GenericListItem>({
// 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(() => {