fix(extensions): prevent stale plugin catalog races
This commit is contained in:
parent
6e875e5a40
commit
afad52f506
4 changed files with 102 additions and 12 deletions
|
|
@ -87,6 +87,12 @@ export const PluginDetailDialog = ({
|
|||
}
|
||||
}, [plugin, open, fetchPluginReadme]);
|
||||
|
||||
useEffect(() => {
|
||||
if (open) {
|
||||
setScope('user');
|
||||
}
|
||||
}, [open, plugin?.pluginId]);
|
||||
|
||||
useEffect(() => {
|
||||
if (scope === 'project' && !projectScopeAvailable) {
|
||||
setScope('user');
|
||||
|
|
|
|||
|
|
@ -148,6 +148,12 @@ export const PluginsPanel = ({
|
|||
}
|
||||
}, [loading, selectedPlugin, selectedPluginId, setSelectedPluginId]);
|
||||
|
||||
useEffect(() => {
|
||||
if (error && selectedPluginId) {
|
||||
setSelectedPluginId(null);
|
||||
}
|
||||
}, [error, selectedPluginId, setSelectedPluginId]);
|
||||
|
||||
const sortValue = `${pluginSort.field}:${pluginSort.order}`;
|
||||
const activeFilterCount =
|
||||
pluginFilters.categories.length +
|
||||
|
|
|
|||
|
|
@ -127,7 +127,8 @@ export interface ExtensionsSlice {
|
|||
// Slice Creator
|
||||
// =============================================================================
|
||||
|
||||
let pluginFetchInFlight: Promise<void> | null = null;
|
||||
let pluginFetchInFlight: { key: string; promise: Promise<void> } | null = null;
|
||||
let pluginCatalogRequestSeq = 0;
|
||||
let mcpDiagnosticsInFlight: Promise<void> | null = null;
|
||||
let skillsCatalogRequestSeq = 0;
|
||||
let skillsDetailRequestSeq = 0;
|
||||
|
|
@ -140,6 +141,10 @@ function hasAnyLoading(loadingMap: Record<string, boolean>): boolean {
|
|||
return Object.values(loadingMap).some(Boolean);
|
||||
}
|
||||
|
||||
function getPluginCatalogKey(projectPath?: string): string {
|
||||
return projectPath ?? '__user__';
|
||||
}
|
||||
|
||||
function getSkillsCatalogKey(projectPath?: string): string {
|
||||
return projectPath ?? USER_SKILLS_CATALOG_KEY;
|
||||
}
|
||||
|
|
@ -205,34 +210,52 @@ export const createExtensionsSlice: StateCreator<AppState, [], [], ExtensionsSli
|
|||
// ── Plugin catalog fetch ──
|
||||
fetchPluginCatalog: async (projectPath?: string, forceRefresh?: boolean) => {
|
||||
if (!api.plugins) return;
|
||||
const requestKey = getPluginCatalogKey(projectPath);
|
||||
|
||||
// Dedup concurrent requests
|
||||
if (pluginFetchInFlight && !forceRefresh) {
|
||||
await pluginFetchInFlight;
|
||||
if (pluginFetchInFlight && !forceRefresh && pluginFetchInFlight.key === requestKey) {
|
||||
await pluginFetchInFlight.promise;
|
||||
return;
|
||||
}
|
||||
|
||||
const requestSeq = ++pluginCatalogRequestSeq;
|
||||
set({ pluginCatalogLoading: true, pluginCatalogError: null });
|
||||
|
||||
const promise = (async () => {
|
||||
try {
|
||||
const result = await api.plugins!.getAll(projectPath, forceRefresh);
|
||||
set({
|
||||
pluginCatalog: result,
|
||||
pluginCatalogLoading: false,
|
||||
pluginCatalogProjectPath: projectPath ?? null,
|
||||
set(() => {
|
||||
if (requestSeq !== pluginCatalogRequestSeq) {
|
||||
return {};
|
||||
}
|
||||
|
||||
return {
|
||||
pluginCatalog: result,
|
||||
pluginCatalogLoading: false,
|
||||
pluginCatalogError: null,
|
||||
pluginCatalogProjectPath: projectPath ?? null,
|
||||
};
|
||||
});
|
||||
} catch (err) {
|
||||
set({
|
||||
pluginCatalogLoading: false,
|
||||
pluginCatalogError: err instanceof Error ? err.message : 'Failed to load plugins',
|
||||
set(() => {
|
||||
if (requestSeq !== pluginCatalogRequestSeq) {
|
||||
return {};
|
||||
}
|
||||
|
||||
return {
|
||||
pluginCatalogLoading: false,
|
||||
pluginCatalogError: err instanceof Error ? err.message : 'Failed to load plugins',
|
||||
pluginCatalogProjectPath: projectPath ?? null,
|
||||
};
|
||||
});
|
||||
} finally {
|
||||
pluginFetchInFlight = null;
|
||||
if (pluginFetchInFlight?.promise === promise) {
|
||||
pluginFetchInFlight = null;
|
||||
}
|
||||
}
|
||||
})();
|
||||
|
||||
pluginFetchInFlight = promise;
|
||||
pluginFetchInFlight = { key: requestKey, promise };
|
||||
await promise;
|
||||
},
|
||||
|
||||
|
|
|
|||
|
|
@ -166,6 +166,61 @@ describe('extensionsSlice', () => {
|
|||
expect(store.getState().pluginCatalogError).toBe('boom');
|
||||
expect(store.getState().pluginCatalogLoading).toBe(false);
|
||||
});
|
||||
|
||||
it('dedups concurrent requests for the same project key', async () => {
|
||||
let resolveFetch!: (plugins: EnrichedPlugin[]) => void;
|
||||
const inFlight = new Promise<EnrichedPlugin[]>((resolve) => {
|
||||
resolveFetch = resolve;
|
||||
});
|
||||
(api.plugins!.getAll as ReturnType<typeof vi.fn>).mockImplementation(() => inFlight);
|
||||
|
||||
const firstFetch = store.getState().fetchPluginCatalog('/tmp/project-a');
|
||||
const secondFetch = store.getState().fetchPluginCatalog('/tmp/project-a');
|
||||
|
||||
expect(api.plugins!.getAll).toHaveBeenCalledTimes(1);
|
||||
|
||||
resolveFetch([makePlugin({ pluginId: 'same@m' })]);
|
||||
await Promise.all([firstFetch, secondFetch]);
|
||||
|
||||
expect(store.getState().pluginCatalogProjectPath).toBe('/tmp/project-a');
|
||||
expect(store.getState().pluginCatalog.map((plugin) => plugin.pluginId)).toEqual(['same@m']);
|
||||
});
|
||||
|
||||
it('keeps the newest project catalog when project changes mid-flight', async () => {
|
||||
let resolveProjectA!: (plugins: EnrichedPlugin[]) => void;
|
||||
let resolveProjectB!: (plugins: EnrichedPlugin[]) => void;
|
||||
const projectAFetch = new Promise<EnrichedPlugin[]>((resolve) => {
|
||||
resolveProjectA = resolve;
|
||||
});
|
||||
const projectBFetch = new Promise<EnrichedPlugin[]>((resolve) => {
|
||||
resolveProjectB = resolve;
|
||||
});
|
||||
|
||||
(api.plugins!.getAll as ReturnType<typeof vi.fn>)
|
||||
.mockImplementationOnce(() => projectAFetch)
|
||||
.mockImplementationOnce(() => projectBFetch);
|
||||
|
||||
const firstFetch = store.getState().fetchPluginCatalog('/tmp/project-a');
|
||||
const secondFetch = store.getState().fetchPluginCatalog('/tmp/project-b');
|
||||
|
||||
expect(api.plugins!.getAll).toHaveBeenCalledTimes(2);
|
||||
|
||||
resolveProjectB([makePlugin({ pluginId: 'project-b@m' })]);
|
||||
await secondFetch;
|
||||
|
||||
expect(store.getState().pluginCatalogProjectPath).toBe('/tmp/project-b');
|
||||
expect(store.getState().pluginCatalog.map((plugin) => plugin.pluginId)).toEqual([
|
||||
'project-b@m',
|
||||
]);
|
||||
|
||||
resolveProjectA([makePlugin({ pluginId: 'project-a@m' })]);
|
||||
await firstFetch;
|
||||
|
||||
expect(store.getState().pluginCatalogProjectPath).toBe('/tmp/project-b');
|
||||
expect(store.getState().pluginCatalog.map((plugin) => plugin.pluginId)).toEqual([
|
||||
'project-b@m',
|
||||
]);
|
||||
});
|
||||
});
|
||||
|
||||
describe('fetchPluginReadme', () => {
|
||||
|
|
|
|||
Loading…
Reference in a new issue