diff --git a/src/utils/plugins/pluginLoader.test.ts b/src/utils/plugins/pluginLoader.test.ts new file mode 100644 index 00000000..b8fd205b --- /dev/null +++ b/src/utils/plugins/pluginLoader.test.ts @@ -0,0 +1,71 @@ +import { describe, expect, test } from 'bun:test' + +import type { LoadedPlugin } from '../../types/plugin.js' +import { mergePluginSources } from './pluginLoader.js' + +function marketplacePlugin( + name: string, + marketplace: string, + enabled: boolean, +): LoadedPlugin { + const pluginId = `${name}@${marketplace}` + return { + name, + manifest: { name } as LoadedPlugin['manifest'], + path: `/tmp/${pluginId}`, + source: pluginId, + repository: pluginId, + enabled, + } +} + +describe('mergePluginSources', () => { + test('keeps the enabled copy when duplicate marketplace plugins disagree on enabled state', () => { + const enabledOfficial = marketplacePlugin( + 'frontend-design', + 'claude-plugins-official', + true, + ) + const disabledLegacy = marketplacePlugin( + 'frontend-design', + 'claude-code-plugins', + false, + ) + + const result = mergePluginSources({ + session: [], + marketplace: [disabledLegacy, enabledOfficial], + builtin: [], + }) + + expect(result.plugins).toEqual([enabledOfficial]) + expect(result.errors).toEqual([]) + }) + + test('keeps the later copy when duplicate marketplace plugins are both enabled', () => { + const legacy = marketplacePlugin( + 'frontend-design', + 'claude-code-plugins', + true, + ) + const official = marketplacePlugin( + 'frontend-design', + 'claude-plugins-official', + true, + ) + + const result = mergePluginSources({ + session: [], + marketplace: [legacy, official], + builtin: [], + }) + + expect(result.plugins).toEqual([official]) + expect(result.errors).toHaveLength(1) + expect(result.errors[0]).toMatchObject({ + type: 'generic-error', + source: legacy.source, + plugin: legacy.name, + }) + }) +}) diff --git a/src/utils/plugins/pluginLoader.ts b/src/utils/plugins/pluginLoader.ts index 0c861bf4..196a452a 100644 --- a/src/utils/plugins/pluginLoader.ts +++ b/src/utils/plugins/pluginLoader.ts @@ -3045,24 +3045,63 @@ export function mergePluginSources(sources: { }) const sessionNames = new Set(sessionPlugins.map(p => p.name)) - const marketplacePlugins = sources.marketplace.filter(p => { - if (sessionNames.has(p.name)) { + // Different marketplaces can enable the same short plugin name, but + // downstream command/skill loading scopes by plugin.name. + const marketplacePluginsByName = new Map() + for (const plugin of sources.marketplace) { + if (sessionNames.has(plugin.name)) { logForDebugging( - `Plugin "${p.name}" from --plugin-dir overrides installed version`, + `Plugin "${plugin.name}" from --plugin-dir overrides installed version`, ) - return false + continue } - return true - }) + const existing = marketplacePluginsByName.get(plugin.name) + if (!existing) { + marketplacePluginsByName.set(plugin.name, plugin) + continue + } + + const winner = selectMarketplacePlugin(existing, plugin) + const dropped = winner === existing ? plugin : existing + marketplacePluginsByName.set(plugin.name, winner) + + logForDebugging( + `Ignoring duplicate marketplace plugin "${plugin.name}" from ${dropped.source}; using ${winner.source}`, + { level: 'warn' }, + ) + if (existing.enabled && plugin.enabled) { + errors.push({ + type: 'generic-error', + source: dropped.source, + plugin: plugin.name, + error: `Duplicate marketplace plugin "${plugin.name}" ignored: using "${winner.source}" and skipping "${dropped.source}" to avoid short-name collisions`, + }) + } + } // Session first, then non-overridden marketplace, then builtin. // Downstream first-match consumers see session plugins before // installed ones for any that slipped past the name filter. return { - plugins: [...sessionPlugins, ...marketplacePlugins, ...sources.builtin], + plugins: [ + ...sessionPlugins, + ...marketplacePluginsByName.values(), + ...sources.builtin, + ], errors, } } +function selectMarketplacePlugin( + current: LoadedPlugin, + candidate: LoadedPlugin, +): LoadedPlugin { + if (current.enabled !== candidate.enabled) { + return candidate.enabled ? candidate : current + } + + return candidate +} + /** * Main plugin loading function that discovers and loads all plugins. *