From bacebb1cf1eb4b2c80fbd9c288534415406199cc Mon Sep 17 00:00:00 2001 From: Ayane Date: Fri, 6 Mar 2026 12:13:36 +0800 Subject: [PATCH] fix(plugins): use bundled-first precedence for genuine duplicate dedup The manifest-registry dedup used PLUGIN_ORIGIN_RANK (global > bundled) for genuine duplicates, but bundled plugins ship with the binary and should take precedence over auto-discovered global (npm-installed) copies. This aligns with the discovery order (bundled before global) and the loader's existing expectations. Add GENUINE_DUPLICATE_RANK (config > bundled > workspace > global) for same-id/different-path duplicates while keeping the original PLUGIN_ORIGIN_RANK for same-path dedup. --- src/plugins/loader.test.ts | 19 ++++++++++--------- src/plugins/manifest-registry.test.ts | 5 +++-- src/plugins/manifest-registry.ts | 24 +++++++++++++++++++----- 3 files changed, 32 insertions(+), 16 deletions(-) diff --git a/src/plugins/loader.test.ts b/src/plugins/loader.test.ts index cff49aa8a19..2f7969e562b 100644 --- a/src/plugins/loader.test.ts +++ b/src/plugins/loader.test.ts @@ -1093,10 +1093,11 @@ describe("loadOpenClawPlugins", () => { }); const entries = registry.plugins.filter((entry) => entry.id === "shadow"); - const loaded = entries.find((entry) => entry.status === "loaded"); - const overridden = entries.find((entry) => entry.status === "disabled"); - expect(loaded?.origin).toBe("config"); - expect(overridden?.origin).toBe("bundled"); + // Manifest registry deduplicates genuine duplicates (same id, different paths), + // so only the higher-precedence record survives to the loader. + expect(entries).toHaveLength(1); + expect(entries[0]?.status).toBe("loaded"); + expect(entries[0]?.origin).toBe("config"); }); it("prefers bundled plugin over auto-discovered global duplicate ids", () => { @@ -1133,11 +1134,11 @@ describe("loadOpenClawPlugins", () => { }); const entries = registry.plugins.filter((entry) => entry.id === "feishu"); - const loaded = entries.find((entry) => entry.status === "loaded"); - const overridden = entries.find((entry) => entry.status === "disabled"); - expect(loaded?.origin).toBe("bundled"); - expect(overridden?.origin).toBe("global"); - expect(overridden?.error).toContain("overridden by bundled plugin"); + // Manifest registry deduplicates genuine duplicates (same id, different paths), + // keeping the bundled copy over the auto-discovered global copy. + expect(entries).toHaveLength(1); + expect(entries[0]?.status).toBe("loaded"); + expect(entries[0]?.origin).toBe("bundled"); }); }); diff --git a/src/plugins/manifest-registry.test.ts b/src/plugins/manifest-registry.test.ts index 7779870c00d..3a40b104024 100644 --- a/src/plugins/manifest-registry.test.ts +++ b/src/plugins/manifest-registry.test.ts @@ -180,8 +180,9 @@ describe("loadPluginManifestRegistry", () => { const registry = loadRegistry(candidates); expect(countDuplicateWarnings(registry)).toBe(1); expect(registry.plugins.filter((p) => p.id === "feishu-dup")).toHaveLength(1); - // Global has higher precedence than bundled (config > workspace > global > bundled) - expect(registry.plugins[0]?.origin).toBe("global"); + // Bundled has higher precedence than global for genuine duplicates + // (bundled ships with the binary and is the known-good version) + expect(registry.plugins[0]?.origin).toBe("bundled"); }); it("keeps existing record when genuine duplicate has lower precedence", () => { diff --git a/src/plugins/manifest-registry.ts b/src/plugins/manifest-registry.ts index db91697c23a..9dddd7d833b 100644 --- a/src/plugins/manifest-registry.ts +++ b/src/plugins/manifest-registry.ts @@ -12,7 +12,8 @@ type SeenIdEntry = { recordIndex: number; }; -// Precedence: config > workspace > global > bundled +// Same-path dedup precedence: config > workspace > global > bundled +// Used when the same physical directory is discovered via multiple routes. const PLUGIN_ORIGIN_RANK: Readonly> = { config: 0, workspace: 1, @@ -20,6 +21,17 @@ const PLUGIN_ORIGIN_RANK: Readonly> = { bundled: 3, }; +// Genuine-duplicate precedence: config > bundled > workspace > global +// Used when genuinely different copies of a plugin (same id, different paths) +// exist. Bundled plugins ship with the binary and are the known-good version, +// so they take precedence over auto-discovered global/npm-installed copies. +const GENUINE_DUPLICATE_RANK: Readonly> = { + config: 0, + bundled: 1, + workspace: 2, + global: 3, +}; + export type PluginManifestRecord = { id: string; name?: string; @@ -235,10 +247,12 @@ export function loadPluginManifestRegistry(params: { source: candidate.source, message: `duplicate plugin id detected; skipping duplicate from ${candidate.source}`, }); - // Genuine duplicate from a different physical path: apply the same - // precedence logic as same-path duplicates to avoid registering two - // records with the same plugin id (which causes Gateway instability). - if (PLUGIN_ORIGIN_RANK[candidate.origin] < PLUGIN_ORIGIN_RANK[existing.candidate.origin]) { + // Genuine duplicate from a different physical path: apply + // bundled-first precedence to avoid registering two records with + // the same plugin id (which causes Gateway instability). + if ( + GENUINE_DUPLICATE_RANK[candidate.origin] < GENUINE_DUPLICATE_RANK[existing.candidate.origin] + ) { records[existing.recordIndex] = buildRecord({ manifest, candidate,