Fix duplicate marketplace plugin loading (#364)
Reproduction: - Enable `frontend-design@claude-code-plugins` - Enable `frontend-design@claude-plugins-official` - Start OpenClaude with both marketplace plugins active - Both plugins load, but downstream command and skill scopes key off the short plugin name, so both collapse to `frontend-design` and can interfere with interactive startup Fix: - Collapse duplicate marketplace plugins by short name during merge - Keep the enabled copy when enabled state differs; otherwise keep the later config entry - Add regression coverage for both cases
This commit is contained in:
71
src/utils/plugins/pluginLoader.test.ts
Normal file
71
src/utils/plugins/pluginLoader.test.ts
Normal file
@@ -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,
|
||||||
|
})
|
||||||
|
})
|
||||||
|
})
|
||||||
@@ -3045,24 +3045,63 @@ export function mergePluginSources(sources: {
|
|||||||
})
|
})
|
||||||
|
|
||||||
const sessionNames = new Set(sessionPlugins.map(p => p.name))
|
const sessionNames = new Set(sessionPlugins.map(p => p.name))
|
||||||
const marketplacePlugins = sources.marketplace.filter(p => {
|
// Different marketplaces can enable the same short plugin name, but
|
||||||
if (sessionNames.has(p.name)) {
|
// downstream command/skill loading scopes by plugin.name.
|
||||||
|
const marketplacePluginsByName = new Map<string, LoadedPlugin>()
|
||||||
|
for (const plugin of sources.marketplace) {
|
||||||
|
if (sessionNames.has(plugin.name)) {
|
||||||
logForDebugging(
|
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.
|
// Session first, then non-overridden marketplace, then builtin.
|
||||||
// Downstream first-match consumers see session plugins before
|
// Downstream first-match consumers see session plugins before
|
||||||
// installed ones for any that slipped past the name filter.
|
// installed ones for any that slipped past the name filter.
|
||||||
return {
|
return {
|
||||||
plugins: [...sessionPlugins, ...marketplacePlugins, ...sources.builtin],
|
plugins: [
|
||||||
|
...sessionPlugins,
|
||||||
|
...marketplacePluginsByName.values(),
|
||||||
|
...sources.builtin,
|
||||||
|
],
|
||||||
errors,
|
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.
|
* Main plugin loading function that discovers and loads all plugins.
|
||||||
*
|
*
|
||||||
|
|||||||
Reference in New Issue
Block a user