From 3d19f018abaf8f9fd1c1c405c74fa88147554875 Mon Sep 17 00:00:00 2001 From: Tortes Date: Tue, 21 Apr 2026 03:49:05 +0800 Subject: [PATCH] fix(plugins): prefer higher-precedence manifests for duplicate plugin ids Keep only the highest-precedence manifest when distinct discovered plugins share an id, while preserving the newer installed-global precedence behavior on main. Lower-precedence duplicates now warn against the ignored manifest source instead of loading as disabled plugin entries. Thanks @Tortes. --- CHANGELOG.md | 1 + src/plugins/loader.test.ts | 21 +++++-- src/plugins/manifest-registry.test.ts | 43 +++++++++++++- src/plugins/manifest-registry.ts | 86 +++++++++++++-------------- 4 files changed, 97 insertions(+), 54 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 9f7024ec963..5f85581bf8c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,6 +13,7 @@ Docs: https://docs.openclaw.ai ### Fixes - Plugins/memory: preserve the active memory capability when read-only snapshot plugin loads run, so status and provider discovery paths no longer wipe memory public artifacts. (#69219) Thanks @zeroaltitude. +- Plugins: keep only the highest-precedence manifest when distinct discovered plugins share an id, so lower-precedence global or workspace duplicates no longer load beside bundled or config-selected plugins. (#41626) Thanks @Tortes. - fix(security): block MINIMAX_API_HOST workspace env injection and remove env-driven URL routing [AI-assisted]. (#67300) Thanks @pgondhi987. - Cron/delivery: treat explicit `delivery.mode: "none"` runs as not requested even if the runner reports `delivered: false`, so no-delivery cron jobs no longer persist false delivery failures or errors. (#69285) Thanks @matsuri1987. - Plugins/install: repair active and default-enabled bundled plugin runtime dependencies before import in packaged installs, so bundled Discord, WhatsApp, Slack, Telegram, and provider plugins work without putting their dependency trees in core. diff --git a/src/plugins/loader.test.ts b/src/plugins/loader.test.ts index 19a61f900c5..6ab65153ac6 100644 --- a/src/plugins/loader.test.ts +++ b/src/plugins/loader.test.ts @@ -429,13 +429,22 @@ function expectPluginSourcePrecedence( }, ) { const entries = registry.plugins.filter((entry) => entry.id === scenario.pluginId); - const loaded = entries.find((entry) => entry.status === "loaded"); - const overridden = entries.find((entry) => entry.status === "disabled"); + expect(entries, scenario.label).toHaveLength(1); + const loaded = entries[0]; expect(loaded?.origin, scenario.label).toBe(scenario.expectedLoadedOrigin); - expect(overridden?.origin, scenario.label).toBe(scenario.expectedDisabledOrigin); - if (scenario.expectedDisabledError) { - expect(overridden?.error, scenario.label).toContain(scenario.expectedDisabledError); - } + expect(loaded?.status, scenario.label).toBe("loaded"); + const expectedWarning = + scenario.expectedDisabledError ?? + `${scenario.expectedDisabledOrigin} plugin will be overridden by ${scenario.expectedLoadedOrigin} plugin`; + expect( + registry.diagnostics.some( + (diag) => + diag.level === "warn" && + diag.pluginId === scenario.pluginId && + diag.message.includes(expectedWarning), + ), + scenario.label, + ).toBe(true); } function expectPluginOriginAndStatus(params: { diff --git a/src/plugins/manifest-registry.test.ts b/src/plugins/manifest-registry.test.ts index 5cc0111a596..0ae869a4479 100644 --- a/src/plugins/manifest-registry.test.ts +++ b/src/plugins/manifest-registry.test.ts @@ -310,7 +310,7 @@ afterEach(() => { }); describe("loadPluginManifestRegistry", () => { - it("emits duplicate warning for truly distinct plugins with same id", () => { + it("keeps only the higher-precedence plugin for truly distinct duplicates", () => { const dirA = makeTempDir(); const dirB = makeTempDir(); const manifest = { id: "test-plugin", configSchema: { type: "object" } }; @@ -330,7 +330,42 @@ describe("loadPluginManifestRegistry", () => { }), ]; - expect(countDuplicateWarnings(loadRegistry(candidates))).toBe(1); + const registry = loadRegistry(candidates); + expect(countDuplicateWarnings(registry)).toBe(1); + expect(registry.plugins).toHaveLength(1); + expect(registry.plugins[0]?.origin).toBe("bundled"); + expectRegistryDiagnosticContains( + registry, + "global plugin will be overridden by bundled plugin", + ); + }); + + it("lets config-loaded plugins replace bundled duplicates", () => { + const bundledDir = makeTempDir(); + const configDir = makeTempDir(); + const manifest = { id: "config-shadow", configSchema: { type: "object" } }; + writeManifest(bundledDir, manifest); + writeManifest(configDir, manifest); + + const registry = loadRegistry([ + createPluginCandidate({ + idHint: "config-shadow", + rootDir: bundledDir, + origin: "bundled", + }), + createPluginCandidate({ + idHint: "config-shadow", + rootDir: configDir, + origin: "config", + }), + ]); + + expect(countDuplicateWarnings(registry)).toBe(1); + expect(registry.plugins).toHaveLength(1); + expect(registry.plugins[0]?.origin).toBe("config"); + const warning = registry.diagnostics.find((diag) => diag.pluginId === "config-shadow"); + expect(warning?.source).toBe(path.join(bundledDir, "index.ts")); + expect(warning?.message).toContain(path.join(configDir, "index.ts")); }); it("reports explicit installed globals as the effective duplicate winner", () => { @@ -371,6 +406,8 @@ describe("loadPluginManifestRegistry", () => { diag.message.includes("bundled plugin will be overridden by global plugin"), ), ).toBe(true); + expect(registry.plugins).toHaveLength(1); + expect(registry.plugins[0]?.origin).toBe("global"); }); it("preserves provider auth env metadata from plugin manifests", () => { @@ -787,6 +824,8 @@ describe("loadPluginManifestRegistry", () => { ] as const)("$name", ({ registry: buildRegistry, expectedMessage }) => { const registry = buildRegistry(); expectRegistryDiagnosticContains(registry, expectedMessage); + expect(registry.plugins).toHaveLength(1); + expect(registry.plugins[0]?.origin).toBe("bundled"); }); it("suppresses duplicate warning when candidates share the same physical directory via symlink", () => { diff --git a/src/plugins/manifest-registry.ts b/src/plugins/manifest-registry.ts index ccd7a089b97..7e5e187804e 100644 --- a/src/plugins/manifest-registry.ts +++ b/src/plugins/manifest-registry.ts @@ -581,6 +581,20 @@ export function loadPluginManifestRegistry( : manifestRes.manifestPath; })(); + const record = isBundleRecord + ? buildBundleRecord({ + manifest: manifest as Parameters[0]["manifest"], + candidate, + manifestPath: manifestRes.manifestPath, + }) + : buildRecord({ + manifest: manifest as PluginManifest, + candidate, + manifestPath: manifestRes.manifestPath, + schemaCacheKey, + configSchema, + }); + const existing = seenIds.get(manifest.id); if (existing) { // Check whether both candidates point to the same physical directory @@ -599,62 +613,42 @@ export function loadPluginManifestRegistry( // Prefer higher-precedence origins even if candidates are passed in // an unexpected order (config > workspace > global > bundled). if (PLUGIN_ORIGIN_RANK[candidate.origin] < PLUGIN_ORIGIN_RANK[existing.candidate.origin]) { - records[existing.recordIndex] = isBundleRecord - ? buildBundleRecord({ - manifest: manifest as Parameters[0]["manifest"], - candidate, - manifestPath: manifestRes.manifestPath, - }) - : buildRecord({ - manifest: manifest as PluginManifest, - candidate, - manifestPath: manifestRes.manifestPath, - schemaCacheKey, - configSchema, - }); + records[existing.recordIndex] = record; seenIds.set(manifest.id, { candidate, recordIndex: existing.recordIndex }); } continue; } + + const candidateRank = resolveDuplicatePrecedenceRank({ + pluginId: manifest.id, + candidate, + config, + env, + }); + const existingRank = resolveDuplicatePrecedenceRank({ + pluginId: manifest.id, + candidate: existing.candidate, + config, + env, + }); + const candidateWins = candidateRank < existingRank; + const winnerCandidate = candidateWins ? candidate : existing.candidate; + const overriddenCandidate = candidateWins ? existing.candidate : candidate; + if (candidateWins) { + records[existing.recordIndex] = record; + seenIds.set(manifest.id, { candidate, recordIndex: existing.recordIndex }); + } diagnostics.push({ level: "warn", pluginId: manifest.id, - source: candidate.source, - message: - resolveDuplicatePrecedenceRank({ - pluginId: manifest.id, - candidate, - config, - env, - }) < - resolveDuplicatePrecedenceRank({ - pluginId: manifest.id, - candidate: existing.candidate, - config, - env, - }) - ? `duplicate plugin id detected; ${existing.candidate.origin} plugin will be overridden by ${candidate.origin} plugin (${candidate.source})` - : `duplicate plugin id detected; ${candidate.origin} plugin will be overridden by ${existing.candidate.origin} plugin (${candidate.source})`, + source: overriddenCandidate.source, + message: `duplicate plugin id detected; ${overriddenCandidate.origin} plugin will be overridden by ${winnerCandidate.origin} plugin (${winnerCandidate.source})`, }); - } else { - seenIds.set(manifest.id, { candidate, recordIndex: records.length }); + continue; } - records.push( - isBundleRecord - ? buildBundleRecord({ - manifest: manifest as Parameters[0]["manifest"], - candidate, - manifestPath: manifestRes.manifestPath, - }) - : buildRecord({ - manifest: manifest as PluginManifest, - candidate, - manifestPath: manifestRes.manifestPath, - schemaCacheKey, - configSchema, - }), - ); + seenIds.set(manifest.id, { candidate, recordIndex: records.length }); + records.push(record); } const registry = { plugins: records, diagnostics };